Commit 3255e134 authored by Amaury Forgeot d'Arc's avatar Amaury Forgeot d'Arc

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
parent 75ee9eb9
...@@ -645,7 +645,7 @@ class ReferencesTestCase(TestBase): ...@@ -645,7 +645,7 @@ class ReferencesTestCase(TestBase):
w = Target() w = Target()
class SubclassableWeakrefTestCase(unittest.TestCase): class SubclassableWeakrefTestCase(TestBase):
def test_subclass_refs(self): def test_subclass_refs(self):
class MyRef(weakref.ref): class MyRef(weakref.ref):
...@@ -709,6 +709,44 @@ class SubclassableWeakrefTestCase(unittest.TestCase): ...@@ -709,6 +709,44 @@ class SubclassableWeakrefTestCase(unittest.TestCase):
self.assertEqual(r.meth(), "abcdef") self.assertEqual(r.meth(), "abcdef")
self.failIf(hasattr(r, "__dict__")) 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: class Object:
def __init__(self, arg): def __init__(self, arg):
...@@ -1150,6 +1188,7 @@ def test_main(): ...@@ -1150,6 +1188,7 @@ def test_main():
MappingTestCase, MappingTestCase,
WeakValueDictionaryTestCase, WeakValueDictionaryTestCase,
WeakKeyDictionaryTestCase, WeakKeyDictionaryTestCase,
SubclassableWeakrefTestCase,
) )
test_support.run_doctest(sys.modules[__name__]) test_support.run_doctest(sys.modules[__name__])
......
...@@ -12,6 +12,9 @@ What's New in Python 2.5.3? ...@@ -12,6 +12,9 @@ What's New in Python 2.5.3?
Core and builtins 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 - 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 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 object's tp_str consider that a type's tp_str could do something that could
......
...@@ -900,7 +900,8 @@ PyObject_ClearWeakRefs(PyObject *object) ...@@ -900,7 +900,8 @@ PyObject_ClearWeakRefs(PyObject *object)
current->wr_callback = NULL; current->wr_callback = NULL;
clear_weakref(current); clear_weakref(current);
if (callback != NULL) { if (callback != NULL) {
handle_callback(current, callback); if (current->ob_refcnt > 0)
handle_callback(current, callback);
Py_DECREF(callback); Py_DECREF(callback);
} }
} }
...@@ -918,9 +919,15 @@ PyObject_ClearWeakRefs(PyObject *object) ...@@ -918,9 +919,15 @@ PyObject_ClearWeakRefs(PyObject *object)
for (i = 0; i < count; ++i) { for (i = 0; i < count; ++i) {
PyWeakReference *next = current->wr_next; PyWeakReference *next = current->wr_next;
Py_INCREF(current); if (current->ob_refcnt > 0)
PyTuple_SET_ITEM(tuple, i * 2, (PyObject *) current); {
PyTuple_SET_ITEM(tuple, i * 2 + 1, current->wr_callback); 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; current->wr_callback = NULL;
clear_weakref(current); clear_weakref(current);
current = next; current = next;
...@@ -928,6 +935,7 @@ PyObject_ClearWeakRefs(PyObject *object) ...@@ -928,6 +935,7 @@ PyObject_ClearWeakRefs(PyObject *object)
for (i = 0; i < count; ++i) { for (i = 0; i < count; ++i) {
PyObject *callback = PyTuple_GET_ITEM(tuple, i * 2 + 1); PyObject *callback = PyTuple_GET_ITEM(tuple, i * 2 + 1);
/* The tuple may have slots left to NULL */
if (callback != NULL) { if (callback != NULL) {
PyObject *item = PyTuple_GET_ITEM(tuple, i * 2); PyObject *item = PyTuple_GET_ITEM(tuple, i * 2);
handle_callback((PyWeakReference *)item, callback); handle_callback((PyWeakReference *)item, callback);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment