diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index 185105b260a..c70804e829e 100644 --- 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 cd44835cb57..386f3f5858d 100644 --- 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 1aee5a55302..5cd91734e4e 100644 --- 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);