subtype_dealloc(): A more complete fix for critical bug 840829 +

expanded the test case with a piece that needs the more-complete fix.

I'll backport this to 2.3 maint.
This commit is contained in:
Tim Peters 2003-11-13 21:59:32 +00:00
parent 981a918575
commit f7f9e9966b
2 changed files with 38 additions and 6 deletions

View File

@ -318,6 +318,25 @@ class ReferencesTestCase(TestBase):
wr = weakref.ref(c, lambda ignore: gc.collect())
del c
# There endeth the first part. It gets worse.
del wr
c1 = C()
c1.i = C()
wr = weakref.ref(c1.i, lambda ignore: gc.collect())
c2 = C()
c2.c1 = c1
del c1 # still alive because c2 points to it
# Now when subtype_dealloc gets called on c2, it's not enough just
# that c2 is immune from gc while the weakref callbacks associated
# with c2 execute (there are none in this 2nd half of the test, btw).
# subtype_dealloc goes on to call the base classes' deallocs too,
# so any gc triggered by weakref callbacks associated with anything
# torn down by a base class dealloc can also trigger double
# deallocation of c2.
del c2
class Object:
def __init__(self, arg):

View File

@ -639,10 +639,10 @@ subtype_dealloc(PyObject *self)
++_PyTrash_delete_nesting;
Py_TRASHCAN_SAFE_BEGIN(self);
--_PyTrash_delete_nesting;
/* DO NOT restore GC tracking at this point. The weakref callback
* (if any) may trigger GC, and if self is tracked at that point,
* it will look like trash to GC and GC will try to delete it
* again. Double-deallocation is a subtle disaster.
/* DO NOT restore GC tracking at this point. weakref callbacks
* (if any, and whether directly here or indirectly in something we
* call) may trigger GC, and if self is tracked at that point, it
* will look like trash to GC and GC will try to delete self again.
*/
/* Find the nearest base with a different tp_dealloc */
@ -658,13 +658,15 @@ subtype_dealloc(PyObject *self)
if (type->tp_weaklistoffset && !base->tp_weaklistoffset)
PyObject_ClearWeakRefs(self);
_PyObject_GC_TRACK(self); /* We'll untrack for real later */
/* Maybe call finalizer; exit early if resurrected */
if (type->tp_del) {
_PyObject_GC_TRACK(self);
type->tp_del(self);
if (self->ob_refcnt > 0)
goto endlabel;
goto endlabel; /* resurrected */
else
_PyObject_GC_UNTRACK(self);
}
/* Clear slots up to the nearest base with a different tp_dealloc */
@ -689,6 +691,7 @@ subtype_dealloc(PyObject *self)
}
/* Finalize GC if the base doesn't do GC and we do */
_PyObject_GC_TRACK(self);
if (!PyType_IS_GC(base))
_PyObject_GC_UNTRACK(self);
@ -730,6 +733,16 @@ subtype_dealloc(PyObject *self)
trashcan begin
GC track
Q. Why did the last question say "immediately GC-track again"?
It's nowhere near immediately.
A. Because the code *used* to re-track immediately. Bad Idea.
self has a refcount of 0, and if gc ever gets its hands on it
(which can happen if any weakref callback gets invoked), it
looks like trash to gc too, and gc also tries to delete self
then. But we're already deleting self. Double dealloction is
a subtle disaster.
Q. Why the bizarre (net-zero) manipulation of
_PyTrash_delete_nesting around the trashcan macros?