Backport of fix for SF bug 840829, memory corruption in some cases of

weakref callbacks.

This is a critical bugfix.  It's already been fixed on the trunk (2.4
development).  I don't intend to backport it to the 22 line, but if a
2.2.4 ever gets released, this should be in it.

diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index 35ab77fd9bc51af116e299047454aba3d5fdf4b7..9149318081f2cc18311f019f32a89d0cd5965cc3 100644 (file)
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -298,6 +298,26 @@ class ReferencesTestCase(TestBase):
         else:
             self.fail("exception not properly restored")
 
+    def test_sf_bug_840829(self):
+        # "weakref callbacks and gc corrupt memory"
+        # subtype_dealloc erroneously exposed a new-style instance
+        # already in the process of getting deallocated to gc,
+        # causing double-deallocation if the instance had a weakref
+        # callback that triggered gc.
+        # If the bug exists, there probably won't be an obvious symptom
+        # in a release build.  In a debug build, a segfault will occur
+        # when the second attempt to remove the instance from the "list
+        # of all objects" occurs.
+
+        import gc
+
+        class C(object):
+            pass
+
+        c = C()
+        wr = weakref.ref(c, lambda ignore: gc.collect())
+        del c
+
 
 class Object:
     def __init__(self, arg):

diff --git a/Misc/NEWS b/Misc/NEWS
index 5bb76a7c8b3a8310be701a87c0bb98f0ccac6aa8..50314afbc7c1c8223ce3c2f2aa026f33b2e040b9 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,12 @@ What's New in Python 2.3.3c1?
 Core and builtins
 -----------------
 
+- Critical bugfix, for SF bug 840829:  if cyclic garbage collection
+  happened to occur during a weakref callback for a new-style class
+  instance, subtle memory corruption could result (in a release build;
+  in a debug build, a segfault occurred reliably very soon after).
+  This has been repaired.
+
 - Patch #820195: object.__contains__() now returns True or False instead
   of 1 or 0.
 
@@ -36,7 +42,7 @@ Extension modules
 - Bug #814613: INET_ADDRSTRLEN fix needed for all compilers on SGI
 
 - The sre module (which underlies the re module) now uses a non-recursive
-  algorithm for matching. A bunch of ugly REs that used to fail with a 
+  algorithm for matching. A bunch of ugly REs that used to fail with a
   RuntimeError will now work. See Bug #757624
 
 Library
@@ -155,7 +161,7 @@ Library
 - Bug #709491: Reset __starttext_tag in sgmllib.
 
 - The bsddb module and dbhash module now support the iterator and
-  mapping protocols. 
+  mapping protocols.
 
 - Bug #711632: Reset all state members in HTMLParser.reset.
 

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 4ca23508bf53870beb413b0c921cdcd93b965707..dbd2bc8db8e5ca85e966552a01fcc17da13fba71 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -639,7 +639,11 @@ subtype_dealloc(PyObject *self)
        ++_PyTrash_delete_nesting;
        Py_TRASHCAN_SAFE_BEGIN(self);
        --_PyTrash_delete_nesting;
-       _PyObject_GC_TRACK(self); /* We'll untrack for real later */
+       /* 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.
+        */
 
        /* Find the nearest base with a different tp_dealloc */
        base = type;
@@ -654,6 +658,7 @@ 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) {