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.

1 files changed, 16 insertions, 3 deletions

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 @@ 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