diff options
-rw-r--r-- | Include/weakrefobject.h | 12 | ||||
-rw-r--r-- | Lib/test/test_weakref.py | 21 | ||||
-rw-r--r-- | Misc/NEWS | 4 | ||||
-rw-r--r-- | Objects/weakrefobject.c | 5 |
4 files changed, 38 insertions, 4 deletions
diff --git a/Include/weakrefobject.h b/Include/weakrefobject.h index 7258387..537e7eb 100644 --- a/Include/weakrefobject.h +++ b/Include/weakrefobject.h @@ -70,7 +70,17 @@ PyAPI_FUNC(Py_ssize_t) _PyWeakref_GetWeakrefCount(PyWeakReference *head); PyAPI_FUNC(void) _PyWeakref_ClearRef(PyWeakReference *self); #endif -#define PyWeakref_GET_OBJECT(ref) (((PyWeakReference *)(ref))->wr_object) +/* Explanation for the Py_REFCNT() check: when a weakref's target is part + of a long chain of deallocations which triggers the trashcan mechanism, + clearing the weakrefs can be delayed long after the target's refcount + has dropped to zero. In the meantime, code accessing the weakref will + be able to "see" the target object even though it is supposed to be + unreachable. See issue #16602. */ + +#define PyWeakref_GET_OBJECT(ref) \ + (Py_REFCNT(((PyWeakReference *)(ref))->wr_object) > 0 \ + ? ((PyWeakReference *)(ref))->wr_object \ + : Py_None) #ifdef __cplusplus diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index dc6bf77..cdd26c7 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -781,6 +781,27 @@ class ReferencesTestCase(TestBase): self.assertEqual(hash(a), hash(42)) self.assertRaises(TypeError, hash, b) + def test_trashcan_16602(self): + # Issue #16602: when a weakref's target was part of a long + # deallocation chain, the trashcan mechanism could delay clearing + # of the weakref and make the target object visible from outside + # code even though its refcount had dropped to 0. A crash ensued. + class C: + def __init__(self, parent): + if not parent: + return + wself = weakref.ref(self) + def cb(wparent): + o = wself() + self.wparent = weakref.ref(parent, cb) + + d = weakref.WeakKeyDictionary() + root = c = C(None) + for n in range(100): + d[c] = c = C(c) + del root + gc.collect() + class SubclassableWeakrefTestCase(TestBase): @@ -10,6 +10,10 @@ What's New in Python 3.4.0 Alpha 1? Core and Builtins ----------------- +- Issue #16602: When a weakref's target was part of a long deallocation + chain, the object could remain reachable through its weakref even though + its refcount had dropped to zero. + - Issue #16495: Remove extraneous NULL encoding check from bytes_decode(). - Issue #16619: Create NameConstant AST class to represent None, True, and False diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index d3a4dd5..b49dcee 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -52,9 +52,8 @@ clear_weakref(PyWeakReference *self) { PyObject *callback = self->wr_callback; - if (PyWeakref_GET_OBJECT(self) != Py_None) { - PyWeakReference **list = GET_WEAKREFS_LISTPTR( - PyWeakref_GET_OBJECT(self)); + if (self->wr_object != Py_None) { + PyWeakReference **list = GET_WEAKREFS_LISTPTR(self->wr_object); if (*list == self) /* If 'self' is the end of the list (and thus self->wr_next == NULL) |