From: Amaury Forgeot d'Arc <amauryfa@gmail.com>
Date: Mon, 16 Jun 2008 19:50:09 +0000 (+0000)
Subject: Merged revisions 64309 via svnmerge from
X-Git-Tag: v3.0b1~29
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c856c7a2f05f059a7f53dc2238f1e271cb1caaa1;p=python

Merged revisions 64309 via svnmerge from
svn+ssh://pythondev@svn.python.org/python/trunk

........
  r64309 | amaury.forgeotdarc | 2008-06-16 21:12:42 +0200 (lun., 16 juin 2008) | 8 lines

  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 a056c23377..1788ac54d7 100644
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -661,7 +661,7 @@ class ReferencesTestCase(TestBase):
         w = Target()
 
 
-class SubclassableWeakrefTestCase(unittest.TestCase):
+class SubclassableWeakrefTestCase(TestBase):
 
     def test_subclass_refs(self):
         class MyRef(weakref.ref):
@@ -725,6 +725,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,
         )
     support.run_doctest(sys.modules[__name__])
 
diff --git a/Misc/NEWS b/Misc/NEWS
index 4408b79a36..07c5eddf3e 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's new in Python 3.0b1?
 Core and Builtins
 -----------------
 
+- Issue #3100: Corrected a crash on deallocation of a subclassed weakref which
+  holds the last (strong) reference to its referent.
+
 - Issue #2630: implement PEP 3138. repr() now returns printable
   Unicode characters unescaped, to get an ASCII-only representation
   of an object use ascii().
diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
index 040e93810d..302cdc9edf 100644
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -884,7 +884,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);
             }
         }
@@ -902,9 +903,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;
@@ -912,6 +919,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);