From d38c990bb73f47a6293e5f783131b709e7ef0982 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sat, 8 Dec 2012 21:15:26 +0100 Subject: 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. Thanks to Eugene Toder for diagnosing and reporting the issue. --- Include/weakrefobject.h | 12 +++++++++++- Lib/test/test_weakref.py | 21 +++++++++++++++++++++ Misc/ACKS | 1 + Misc/NEWS | 4 ++++ Objects/weakrefobject.c | 5 ++--- 5 files changed, 39 insertions(+), 4 deletions(-) diff --git a/Include/weakrefobject.h b/Include/weakrefobject.h index f15c9d9c..e46aecf 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 b7c72bd..3d86cb7 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 0a7c8bd..5e4c03a 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 0c01e8b..9934212 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 7b943b0..99bf42b 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) -- cgit v0.12