Commit 17c89bd8 authored by Tim Peters's avatar Tim Peters

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.
parent a8ae0ee2
...@@ -58,6 +58,20 @@ init_strings(void) ...@@ -58,6 +58,20 @@ init_strings(void)
return 0; 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*); static void ghostify(cPersistentObject*);
/* Load the state of the object, unghostifying it. Upon success, return 1. /* Load the state of the object, unghostifying it. Upon success, return 1.
...@@ -88,6 +102,11 @@ unghostify(cPersistentObject *self) ...@@ -88,6 +102,11 @@ unghostify(cPersistentObject *self)
} }
self->state = cPersistent_UPTODATE_STATE; self->state = cPersistent_UPTODATE_STATE;
Py_DECREF(r); 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; return 1;
} }
...@@ -134,6 +153,11 @@ ghostify(cPersistentObject *self) ...@@ -134,6 +153,11 @@ ghostify(cPersistentObject *self)
return; 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 the cache is still active, we must unlink the object. */
if (self->ring.r_next) { if (self->ring.r_next) {
/* if we're ghostifying an object, we better have some non-ghosts */ /* if we're ghostifying an object, we better have some non-ghosts */
...@@ -249,7 +273,7 @@ pickle_slotnames(PyTypeObject *cls) ...@@ -249,7 +273,7 @@ pickle_slotnames(PyTypeObject *cls)
return slotnames; return slotnames;
} }
slotnames = PyObject_CallFunctionObjArgs(copy_reg_slotnames, slotnames = PyObject_CallFunctionObjArgs(copy_reg_slotnames,
(PyObject*)cls, NULL); (PyObject*)cls, NULL);
if (slotnames && !(slotnames == Py_None || PyList_Check(slotnames))) { if (slotnames && !(slotnames == Py_None || PyList_Check(slotnames))) {
PyErr_SetString(PyExc_TypeError, PyErr_SetString(PyExc_TypeError,
...@@ -257,7 +281,7 @@ pickle_slotnames(PyTypeObject *cls) ...@@ -257,7 +281,7 @@ pickle_slotnames(PyTypeObject *cls)
Py_DECREF(slotnames); Py_DECREF(slotnames);
return NULL; return NULL;
} }
return slotnames; return slotnames;
} }
...@@ -288,7 +312,7 @@ pickle_copy_dict(PyObject *state) ...@@ -288,7 +312,7 @@ pickle_copy_dict(PyObject *state)
if (PyObject_SetItem(copy, key, value) < 0) if (PyObject_SetItem(copy, key, value) < 0)
goto err; goto err;
} }
return copy; return copy;
err: err:
Py_DECREF(copy); Py_DECREF(copy);
...@@ -366,13 +390,13 @@ pickle___getstate__(PyObject *self) ...@@ -366,13 +390,13 @@ pickle___getstate__(PyObject *self)
} }
} }
if (n) if (n)
state = Py_BuildValue("(NO)", state, slots); state = Py_BuildValue("(NO)", state, slots);
end: end:
Py_XDECREF(slotnames); Py_XDECREF(slotnames);
Py_XDECREF(slots); Py_XDECREF(slots);
return state; return state;
} }
...@@ -381,12 +405,12 @@ pickle_setattrs_from_dict(PyObject *self, PyObject *dict) ...@@ -381,12 +405,12 @@ pickle_setattrs_from_dict(PyObject *self, PyObject *dict)
{ {
PyObject *key, *value; PyObject *key, *value;
int pos = 0; int pos = 0;
if (!PyDict_Check(dict)) { if (!PyDict_Check(dict)) {
PyErr_SetString(PyExc_TypeError, "Expected dictionary"); PyErr_SetString(PyExc_TypeError, "Expected dictionary");
return -1; return -1;
} }
while (PyDict_Next(dict, &pos, &key, &value)) { while (PyDict_Next(dict, &pos, &key, &value)) {
if (PyObject_SetAttr(self, key, value) < 0) if (PyObject_SetAttr(self, key, value) < 0)
return -1; return -1;
...@@ -451,7 +475,7 @@ pickle___setstate__(PyObject *self, PyObject *state) ...@@ -451,7 +475,7 @@ pickle___setstate__(PyObject *self, PyObject *state)
return Py_None; return Py_None;
} }
static char pickle___reduce__doc[] = static char pickle___reduce__doc[] =
"Reduce an object to contituent parts for serialization\n" "Reduce an object to contituent parts for serialization\n"
; ;
...@@ -475,18 +499,18 @@ pickle___reduce__(PyObject *self) ...@@ -475,18 +499,18 @@ pickle___reduce__(PyObject *self)
PyErr_Clear(); PyErr_Clear();
l = 0; l = 0;
} }
args = PyTuple_New(l+1); args = PyTuple_New(l+1);
if (args == NULL) if (args == NULL)
goto end; goto end;
Py_INCREF(self->ob_type); Py_INCREF(self->ob_type);
PyTuple_SET_ITEM(args, 0, (PyObject*)(self->ob_type)); PyTuple_SET_ITEM(args, 0, (PyObject*)(self->ob_type));
for (i = 0; i < l; i++) { for (i = 0; i < l; i++) {
Py_INCREF(PyTuple_GET_ITEM(bargs, i)); Py_INCREF(PyTuple_GET_ITEM(bargs, i));
PyTuple_SET_ITEM(args, i+1, PyTuple_GET_ITEM(bargs, i)); PyTuple_SET_ITEM(args, i+1, PyTuple_GET_ITEM(bargs, i));
} }
state = PyObject_CallMethodObjArgs(self, py___getstate__, NULL); state = PyObject_CallMethodObjArgs(self, py___getstate__, NULL);
if (!state) if (!state)
goto end; goto end;
...@@ -680,7 +704,7 @@ Per__p_getattr(cPersistentObject *self, PyObject *name) ...@@ -680,7 +704,7 @@ Per__p_getattr(cPersistentObject *self, PyObject *name)
} }
else else
result = Py_True; result = Py_True;
Py_INCREF(result); Py_INCREF(result);
Done: Done:
...@@ -688,7 +712,7 @@ Per__p_getattr(cPersistentObject *self, PyObject *name) ...@@ -688,7 +712,7 @@ Per__p_getattr(cPersistentObject *self, PyObject *name)
return result; return result;
} }
/* /*
XXX we should probably not allow assignment of __class__ and __dict__. XXX we should probably not allow assignment of __class__ and __dict__.
*/ */
...@@ -1012,7 +1036,7 @@ static struct PyMethodDef Per_methods[] = { ...@@ -1012,7 +1036,7 @@ static struct PyMethodDef Per_methods[] = {
{"__getstate__", (PyCFunction)Per__getstate__, METH_NOARGS, {"__getstate__", (PyCFunction)Per__getstate__, METH_NOARGS,
pickle___getstate__doc }, pickle___getstate__doc },
{"__setstate__", (PyCFunction)pickle___setstate__, METH_O, {"__setstate__", (PyCFunction)pickle___setstate__, METH_O,
pickle___setstate__doc}, pickle___setstate__doc},
{"__reduce__", (PyCFunction)pickle___reduce__, METH_NOARGS, {"__reduce__", (PyCFunction)pickle___reduce__, METH_NOARGS,
pickle___reduce__doc}, pickle___reduce__doc},
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment