[Python-checkins] cpython (3.2): Issue #16602: When a weakref's target was part of a long deallocation chain,

Sat Dec 8 21:20:29 CET 2012

http://hg.python.org/cpython/rev/0748c22b37e5
changeset: 80759:0748c22b37e5
branch: 3.2
parent: 80755:fed7306f26ce
user: Antoine Pitrou <solipsis at pitrou.net>
date: Sat Dec 08 21:15:26 2012 +0100
summary:
 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.
files:
 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
--- a/Include/weakrefobject.h
+++ b/Include/weakrefobject.h
@@ -70,7 +70,17 @@
 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
--- a/Lib/test/test_weakref.py
+++ b/Lib/test/test_weakref.py
@@ -776,6 +776,27 @@
 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):
 
diff --git a/Misc/ACKS b/Misc/ACKS
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1067,6 +1067,7 @@
 Frank J. Tobin
 R Lindsay Todd
 Bennett Todd
+Eugene Toder
 Erik Tollerud
 Matias Torchinsky
 Sandro Tosi
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,10 @@
 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 #16416: On Mac OS X, operating system data are now always
 encoded/decoded to/from UTF-8/surrogateescape, instead of the locale encoding
 (which may be ASCII if no locale environment variable is set), to avoid
diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -52,9 +52,8 @@
 {
 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)
-- 
Repository URL: http://hg.python.org/cpython
