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!

Backport of r34309

diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index c669109f08b221f676610f70adbc2a848b21ff36..e7b6ed23062fdcfbf0a41aae313d97e429ad15f6 100644 (file)
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -645,7 +645,7 @@ class ReferencesTestCase(TestBase):
         w = Target()
 
 
-class SubclassableWeakrefTestCase(unittest.TestCase):
+class SubclassableWeakrefTestCase(TestBase):
 
     def test_subclass_refs(self):
         class MyRef(weakref.ref):
@@ -709,6 +709,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):
@@ -1150,6 +1188,7 @@ def test_main():
         MappingTestCase,
         WeakValueDictionaryTestCase,
         WeakKeyDictionaryTestCase,
+        SubclassableWeakrefTestCase,
         )
     test_support.run_doctest(sys.modules[__name__])
 

diff --git a/Misc/NEWS b/Misc/NEWS
index cdc3f9c748dd0075d03c46458090e02aa62083a0..27c12c1425aefc5254569f4ac290c8757fb91837 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 2.5.3?
 Core and builtins
 -----------------
 
+- Issue #3100: Corrected a crash on deallocation of a subclassed weakref which
+  holds the last (strong) reference to its referent.
+
 - Issue #1686386: Tuple's tp_repr did not take into account the possibility of
   having a self-referential tuple, which is possible from C code.  Nor did
   object's tp_str consider that a type's tp_str could do something that could

diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
index a404f29ade2b0bdeb00b4f3abaa32384806b3437..2cad79346476fd94853d04dc151c014246165231 100644 (file)
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -900,7 +900,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);
             }
         }
@@ -918,9 +919,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;
@@ -928,6 +935,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);