[Zope-Checkins] SVN: Zope/trunk/lib/python/persistent/cPersistence.c Extreme sanction for collector #1350.

[Tim Peters]

Log message for revision 27788:
  Extreme sanction for collector #1350.
  
  In ghostify() and unghostify(), trigger a fatal error if the
  object is insane.  This prevents a segfault (or, worse, arbitrary
  memory corruption) later.
  
  The test suite isn't bothered by this, and neither is bringing
  up a Zope and playing around with it.  The only known cause
  appears to be threading problems related to Transience.py,
  partly explained in issue #1350.  It should be impossible for
  these fatal errors to trigger via thread-correct use of ZODB.
  
  I don't expect to keep these fatal errors in the code; indeed,
  I'm checking this in only in Zope's *copy* of ZODB.  The intent
  is to help whoever can make time for 1350 know whether that
  problem still exists, until that problem goes away.  Unfortunately,
  it's not even possible to raise an exception from ghostify()
  (it's a void routine that "can't fail"), so it takes an extreme
  measure to catch the problem as soon as it's visible.
  


Changed:
  U   Zope/trunk/lib/python/persistent/cPersistence.c


-=-
Modified: Zope/trunk/lib/python/persistent/cPersistence.c
===================================================================
--- Zope/trunk/lib/python/persistent/cPersistence.c	2004-10-07 19:30:55 UTC (rev 27787)
+++ Zope/trunk/lib/python/persistent/cPersistence.c	2004-10-08 05:21:51 UTC (rev 27788)
@@ -58,6 +58,20 @@
     return 0;
 }
 
+static void
+fatal(cPersistentObject *self, const char *caller, const char *detail)
+{
+	char buf[1000];
+	PyOS_snprintf(buf, sizeof(buf),
+	    "cPersistence.c %s(): object at %p with type %.200s\n"
+	    "%s.\n"
+	    "The only known cause is multiple threads trying to ghost and\n"
+	    "unghost the object simultaneously.\n"
+	    "That's not legal, but ZODB can't stop it.\n"
+	    "See Collector #1350.\n",
+	    caller, self, self->ob_type->tp_name, detail);
+	Py_FatalError(buf);
+}
 static void ghostify(cPersistentObject*);
 
 /* Load the state of the object, unghostifying it.  Upon success, return 1.
@@ -88,6 +102,11 @@
         }
         self->state = cPersistent_UPTODATE_STATE;
         Py_DECREF(r);
+        if (self->cache && self->ring.r_next == NULL) {
+        	fatal(self, "unghostify",
+		      "is not in the cache despite that we just "
+		      "unghostified it");
+	}
     }
     return 1;
 }
@@ -134,6 +153,11 @@
         return;
     }
 
+    if (! self->ring.r_next) {
+	fatal(self, "ghostify", "claims to be in a cache but isn't");
+    }
+
+    /* XXX The next comment is nonsense. */
     /* If the cache is still active, we must unlink the object. */
     if (self->ring.r_next) {
 	/* if we're ghostifying an object, we better have some non-ghosts */
@@ -249,7 +273,7 @@
 	return slotnames;
     }
 
-    slotnames = PyObject_CallFunctionObjArgs(copy_reg_slotnames, 
+    slotnames = PyObject_CallFunctionObjArgs(copy_reg_slotnames,
 					     (PyObject*)cls, NULL);
     if (slotnames && !(slotnames == Py_None || PyList_Check(slotnames))) {
 	PyErr_SetString(PyExc_TypeError,
@@ -257,7 +281,7 @@
 	Py_DECREF(slotnames);
 	return NULL;
     }
-  
+
     return slotnames;
 }
 
@@ -288,7 +312,7 @@
 	if (PyObject_SetItem(copy, key, value) < 0)
 	    goto err;
     }
-  
+
     return copy;
  err:
     Py_DECREF(copy);
@@ -366,13 +390,13 @@
         }
     }
 
-    if (n) 
+    if (n)
 	state = Py_BuildValue("(NO)", state, slots);
 
  end:
     Py_XDECREF(slotnames);
     Py_XDECREF(slots);
-  
+
     return state;
 }
 
@@ -381,12 +405,12 @@
 {
     PyObject *key, *value;
     int pos = 0;
-  
+
     if (!PyDict_Check(dict)) {
 	PyErr_SetString(PyExc_TypeError, "Expected dictionary");
 	return -1;
     }
-  
+
     while (PyDict_Next(dict, &pos, &key, &value)) {
 	if (PyObject_SetAttr(self, key, value) < 0)
 	    return -1;
@@ -451,7 +475,7 @@
     return Py_None;
 }
 
-static char pickle___reduce__doc[] = 
+static char pickle___reduce__doc[] =
 "Reduce an object to contituent parts for serialization\n"
 ;
 
@@ -475,18 +499,18 @@
 	PyErr_Clear();
 	l = 0;
     }
-  
+
     args = PyTuple_New(l+1);
     if (args == NULL)
 	goto end;
-  
+
     Py_INCREF(self->ob_type);
     PyTuple_SET_ITEM(args, 0, (PyObject*)(self->ob_type));
     for (i = 0; i < l; i++) {
 	Py_INCREF(PyTuple_GET_ITEM(bargs, i));
 	PyTuple_SET_ITEM(args, i+1, PyTuple_GET_ITEM(bargs, i));
     }
-  
+
     state = PyObject_CallMethodObjArgs(self, py___getstate__, NULL);
     if (!state)
 	goto end;
@@ -680,7 +704,7 @@
     }
     else
 	result = Py_True;
-      
+
     Py_INCREF(result);
 
   Done:
@@ -688,7 +712,7 @@
     return result;
 }
 
-/* 
+/*
    XXX we should probably not allow assignment of __class__ and __dict__.
 */
 
@@ -1012,7 +1036,7 @@
   {"__getstate__", (PyCFunction)Per__getstate__, METH_NOARGS,
    pickle___getstate__doc },
   {"__setstate__", (PyCFunction)pickle___setstate__, METH_O,
-   pickle___setstate__doc},                                          
+   pickle___setstate__doc},
   {"__reduce__", (PyCFunction)pickle___reduce__, METH_NOARGS,
    pickle___reduce__doc},