diff --git a/Include/weakrefobject.h b/Include/weakrefobject.h index f15c9d9c128..e46aecf3858 100644 --- a/Include/weakrefobject.h +++ b/Include/weakrefobject.h @@ -66,7 +66,17 @@ PyAPI_FUNC(Py_ssize_t) _PyWeakref_GetWeakrefCount(PyWeakReference *head); PyAPI_FUNC(void) _PyWeakref_ClearRef(PyWeakReference *self); -#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 b7c72bd5e1b..3d86cb7eeb5 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -774,6 +774,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(object): + 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): diff --git a/Misc/ACKS b/Misc/ACKS index 0a7c8bd2e21..5e4c03a04e8 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -990,6 +990,7 @@ Jim Tittsler Frank J. Tobin R Lindsay Todd Bennett Todd +Eugene Toder Matias Torchinsky Sandro Tosi Richard Townsend diff --git a/Misc/NEWS b/Misc/NEWS index 0c01e8b6ab8..9934212f6c0 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -9,6 +9,10 @@ What's New in Python 2.7.4 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 #9011: Fix hacky AST code that modified the CST when compiling a negated numeric literal. diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index 7b943b0f004..99bf42bfece 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)