Issue 3110: Crash with weakref subclass,

seen after a "import multiprocessing.reduction"

An instance of a weakref subclass can have attributes.
If such a weakref holds the only strong reference to the object,
deleting the weakref will delete the object. In this case,
the callback must not be called, because the ref object is being deleted!

diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index 185105b260aed475280156d2a98aa3d09d534a5d..c70804e829e38dad7cc7f2fc1eb166fe3618d4b8 100644 (file)
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -666,7 +666,7 @@ class ReferencesTestCase(TestBase):
         w = Target()
 
 
-class SubclassableWeakrefTestCase(unittest.TestCase):
+class SubclassableWeakrefTestCase(TestBase):
 
     def test_subclass_refs(self):
         class MyRef(weakref.ref):
@@ -730,6 +730,44 @@ class SubclassableWeakrefTestCase(unittest.TestCase):
         self.assertEqual(r.meth(), "abcdef")
         self.failIf(hasattr(r, "__dict__"))
 
+    def test_subclass_refs_with_cycle(self):
+        # Bug #3110
+        # An instance of a weakref subclass can have attributes.
+        # If such a weakref holds the only strong reference to the object,
+        # deleting the weakref will delete the object. In this case,
+        # the callback must not be called, because the ref object is
+        # being deleted.
+        class MyRef(weakref.ref):
+            pass
+
+        # Use a local callback, for "regrtest -R::"
+        # to detect refcounting problems
+        def callback(w):
+            self.cbcalled += 1
+
+        o = C()
+        r1 = MyRef(o, callback)
+        r1.o = o
+        del o
+
+        del r1 # Used to crash here
+
+        self.assertEqual(self.cbcalled, 0)
+
+        # Same test, with two weakrefs to the same object
+        # (since code paths are different)
+        o = C()
+        r1 = MyRef(o, callback)
+        r2 = MyRef(o, callback)
+        r1.r = r2
+        r2.o = o
+        del o
+        del r2
+
+        del r1 # Used to crash here
+
+        self.assertEqual(self.cbcalled, 0)
+
 
 class Object:
     def __init__(self, arg):
@@ -1171,6 +1209,7 @@ def test_main():
         MappingTestCase,
         WeakValueDictionaryTestCase,
         WeakKeyDictionaryTestCase,
+        SubclassableWeakrefTestCase,
         )
     test_support.run_doctest(sys.modules[__name__])
 

diff --git a/Misc/NEWS b/Misc/NEWS
index cd44835cb57f95e0a52cec656da2285417ba4631..386f3f5858d7798c47ef1569a502a7b458389a63 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.6 beta 1?
 Core and Builtins
 -----------------
 
+- Issue #3100: Corrected a crash on deallocation of a subclassed weakref which
+  holds the last (strong) reference to its referent.
+
 - Add future_builtins.ascii().
 
 - Several set methods now accept multiple arguments:  update(), union(),

diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
index 1aee5a55302f74f46247c101826f08ae1e72c6ba..5cd91734e4e76821ddf611ac510ca50a49126213 100644 (file)
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -907,7 +907,8 @@ PyObject_ClearWeakRefs(PyObject *object)
             current->wr_callback = NULL;
             clear_weakref(current);
             if (callback != NULL) {
-                handle_callback(current, callback);
+                if (current->ob_refcnt > 0)
+                    handle_callback(current, callback);
                 Py_DECREF(callback);
             }
         }
@@ -925,9 +926,15 @@ PyObject_ClearWeakRefs(PyObject *object)
             for (i = 0; i < count; ++i) {
                 PyWeakReference *next = current->wr_next;
 
-                Py_INCREF(current);
-                PyTuple_SET_ITEM(tuple, i * 2, (PyObject *) current);
-                PyTuple_SET_ITEM(tuple, i * 2 + 1, current->wr_callback);
+                if (current->ob_refcnt > 0)
+                {
+                    Py_INCREF(current);
+                    PyTuple_SET_ITEM(tuple, i * 2, (PyObject *) current);
+                    PyTuple_SET_ITEM(tuple, i * 2 + 1, current->wr_callback);
+                }
+                else {
+                    Py_DECREF(current->wr_callback);
+                }
                 current->wr_callback = NULL;
                 clear_weakref(current);
                 current = next;
@@ -935,6 +942,7 @@ PyObject_ClearWeakRefs(PyObject *object)
             for (i = 0; i < count; ++i) {
                 PyObject *callback = PyTuple_GET_ITEM(tuple, i * 2 + 1);
 
+                /* The tuple may have slots left to NULL */
                 if (callback != NULL) {
                     PyObject *item = PyTuple_GET_ITEM(tuple, i * 2);
                     handle_callback((PyWeakReference *)item, callback);