diff options
author | Fred Drake <fdrake@acm.org> | 2004-02-04 23:14:14 +0000 |
---|---|---|
committer | Fred Drake <fdrake@acm.org> | 2004-02-04 23:14:14 +0000 |
commit | bc875f5a36b744c1a7263c35befaf31208a416b1 (patch) | |
tree | 9b362880eeb66497bd813f43f2806eef2d2bb7f0 | |
parent | 21ae4f983e3625cc16807c68952c6cd9fe44bb8b (diff) | |
download | cpython-git-bc875f5a36b744c1a7263c35befaf31208a416b1.tar.gz |
Allocating a new weakref object can cause existing weakref objects for
the same object to be collected by the cyclic GC support if they are
only referenced by a cycle. If the weakref being collected was one of
the weakrefs without callbacks, some local variables for the
constructor became invalid and have to be re-computed.
The test caused a segfault under a debug build without the fix applied.
-rw-r--r-- | Lib/test/test_weakref.py | 32 | ||||
-rw-r--r-- | Objects/weakrefobject.c | 19 |
2 files changed, 48 insertions, 3 deletions
diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index f732d7ba81..00cd7ae04f 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -1,3 +1,4 @@ +import gc import sys import unittest import UserList @@ -591,6 +592,37 @@ class ReferencesTestCase(TestBase): gc.collect() self.assertEqual(alist, []) + def test_gc_during_ref_creation(self): + self.check_gc_during_creation(weakref.ref) + + def test_gc_during_proxy_creation(self): + self.check_gc_during_creation(weakref.proxy) + + def check_gc_during_creation(self, makeref): + thresholds = gc.get_threshold() + gc.set_threshold(1, 1, 1) + gc.collect() + class A: + pass + + def callback(*args): + pass + + referenced = A() + + a = A() + a.a = a + a.wr = makeref(referenced) + + try: + # now make sure the object and the ref get labeled as + # cyclic trash: + a = A() + a.wrc = weakref.ref(referenced, callback) + + finally: + gc.set_threshold(*thresholds) + class Object: def __init__(self, arg): self.arg = arg diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index cf0316a50f..575a928f75 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -630,16 +630,23 @@ PyWeakref_NewRef(PyObject *ob, PyObject *callback) /* return existing weak reference if it exists */ result = ref; if (result != NULL) - Py_XINCREF(result); + Py_INCREF(result); else { + /* Note: new_weakref() can trigger cyclic GC, so the weakref + list on ob can be mutated. This means that the ref and + proxy pointers we got back earlier may have been collected, + so we need to compute these values again before we use + them. */ result = new_weakref(ob, callback); if (result != NULL) { if (callback == NULL) { insert_head(result, list); } else { - PyWeakReference *prev = (proxy == NULL) ? ref : proxy; + PyWeakReference *prev; + get_basic_refs(*list, &ref, &proxy); + prev = (proxy == NULL) ? ref : proxy; if (prev == NULL) insert_head(result, list); else @@ -672,8 +679,13 @@ PyWeakref_NewProxy(PyObject *ob, PyObject *callback) /* attempt to return an existing weak reference if it exists */ result = proxy; if (result != NULL) - Py_XINCREF(result); + Py_INCREF(result); else { + /* Note: new_weakref() can trigger cyclic GC, so the weakref + list on ob can be mutated. This means that the ref and + proxy pointers we got back earlier may have been collected, + so we need to compute these values again before we use + them. */ result = new_weakref(ob, callback); if (result != NULL) { PyWeakReference *prev; @@ -682,6 +694,7 @@ PyWeakref_NewProxy(PyObject *ob, PyObject *callback) result->ob_type = &_PyWeakref_CallableProxyType; else result->ob_type = &_PyWeakref_ProxyType; + get_basic_refs(*list, &ref, &proxy); if (callback == NULL) prev = ref; else |