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.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index bdbabf4..032fa18 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -639,10 +639,10 @@
++_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 @@
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 @@
}
/* 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 @@
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?