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.
diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
index f732d7b..00cd7ae 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 @@
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 cf0316a..575a928 100644
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -630,16 +630,23 @@
/* 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 @@
/* 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 @@
result->ob_type = &_PyWeakref_CallableProxyType;
else
result->ob_type = &_PyWeakref_ProxyType;
+ get_basic_refs(*list, &ref, &proxy);
if (callback == NULL)
prev = ref;
else