weakref: make weakrefs thread-safe without the GIL

Also reduce GC frequency in "collect_in_thread" to improve test_weakref
speed when running without the GIL.

Include/cpython/weakrefobject.h

@@ -2,55 +2,59 @@
 #  error "this header file must not be included directly"
 #endif
 
-/* PyWeakReference is the base struct for the Python ReferenceType, ProxyType,
- * and CallableProxyType.
- */
-struct _PyWeakReference {
+struct _PyWeakrefBase {
     PyObject_HEAD
 
+    /* If wr_object is weakly referenced, wr_object has a doubly-linked NULL-
+     * terminated list of weak references to it.  These are the list pointers.
+     * If wr_object goes away, wr_object is set to Py_None, and these pointers
+     * have no meaning then.
+     */
+    struct _PyWeakrefBase *wr_prev;
+    struct _PyWeakrefBase *wr_next;
+};
+
+struct _PyWeakrefControl {
+    struct _PyWeakrefBase base;
+
+    /* Protectes the weakref linked-list and wr_object from
+     * concurrent accesses. */
+    _PyMutex mutex;
+
     /* The object to which this is a weak reference, or Py_None if none.
      * Note that this is a stealth reference:  wr_object's refcount is
      * not incremented to reflect this pointer.
      */
     PyObject *wr_object;
+};
+
+/* PyWeakReference is the base struct for the Python ReferenceType, ProxyType,
+ * and CallableProxyType.
+ */
+struct _PyWeakReference {
+    struct _PyWeakrefBase base;
+
+    /* Pointer to weakref control block */
+    struct _PyWeakrefControl *wr_parent;
 
     /* A callable to invoke when wr_object dies, or NULL if none. */
     PyObject *wr_callback;
 
+    vectorcallfunc vectorcall;
+
     /* A cache for wr_object's hash code.  As usual for hashes, this is -1
      * if the hash code isn't known yet.
      */
     Py_hash_t hash;
-
-    /* If wr_object is weakly referenced, wr_object has a doubly-linked NULL-
-     * terminated list of weak references to it.  These are the list pointers.
-     * If wr_object goes away, wr_object is set to Py_None, and these pointers
-     * have no meaning then.
-     */
-    PyWeakReference *wr_prev;
-    PyWeakReference *wr_next;
-    vectorcallfunc vectorcall;
 };
 
-PyAPI_FUNC(Py_ssize_t) _PyWeakref_GetWeakrefCount(PyWeakReference *head);
+typedef struct _PyWeakrefControl PyWeakrefControl;
+typedef struct _PyWeakrefBase PyWeakrefBase;
+
+PyAPI_FUNC(void) _PyWeakref_DetachRef(PyWeakReference *self);
+
+PyAPI_FUNC(Py_ssize_t) _PyWeakref_GetWeakrefCount(struct _PyWeakrefControl *ctrl);
 
 PyAPI_FUNC(void) _PyWeakref_ClearRef(PyWeakReference *self);
 
-static inline PyObject* PyWeakref_GET_OBJECT(PyObject *ref_obj) {
-    PyWeakReference *ref;
-    PyObject *obj;
-    assert(PyWeakref_Check(ref_obj));
-    ref = _Py_CAST(PyWeakReference*, ref_obj);
-    obj = 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 gh-60806.
-    if (Py_IS_REFERENCED(obj)) {
-        return obj;
-    }
-    return Py_None;
-}
-#define PyWeakref_GET_OBJECT(ref) PyWeakref_GET_OBJECT(_PyObject_CAST(ref))
+#define PyWeakref_GET_OBJECT(ref) PyWeakref_GetObject(_PyObject_CAST(ref))

Include/internal/pycore_object.h

@@ -380,21 +380,21 @@ extern void _Py_PrintReferenceAddresses(FILE *);
  * nor should it be used to add, remove, or swap any refs in the list.
  * That is the sole responsibility of the code in weakrefobject.c.
  */
-static inline PyObject **
-_PyObject_GET_WEAKREFS_LISTPTR(PyObject *op)
+static inline PyWeakrefControl **
+_PyObject_GET_WEAKREFS_CONTROLPTR(PyObject *op)
 {
     if (PyType_Check(op) &&
             ((PyTypeObject *)op)->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) {
         static_builtin_state *state = _PyStaticType_GetState(
                                                         (PyTypeObject *)op);
         return _PyStaticType_GET_WEAKREFS_LISTPTR(state);
     }
-    // Essentially _PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET():
+    // Essentially _PyObject_GET_WEAKREFS_CONTROLPTR_FROM_OFFSET():
     Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset;
-    return (PyObject **)((char *)op + offset);
+    return (PyWeakrefControl **)((char *)op + offset);
 }
 
-/* This is a special case of _PyObject_GET_WEAKREFS_LISTPTR().
+/* This is a special case of _PyObject_GET_WEAKREFS_CONTROLPTR().
  * Only the most fundamental lookup path is used.
  * Consequently, static types should not be used.
  *
@@ -404,15 +404,21 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op)
  * are only finalized at the end of runtime finalization.
  *
  * If the weaklist for static types is actually needed then use
- * _PyObject_GET_WEAKREFS_LISTPTR().
+ * _PyObject_GET_WEAKREFS_CONTROLPTR().
  */
-static inline PyWeakReference **
-_PyObject_GET_WEAKREFS_LISTPTR_FROM_OFFSET(PyObject *op)
+static inline PyWeakrefControl **
+_PyObject_GET_WEAKREFS_CONTROLPTR_FROM_OFFSET(PyObject *op)
 {
     assert(!PyType_Check(op) ||
             ((PyTypeObject *)op)->tp_flags & Py_TPFLAGS_HEAPTYPE);
     Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset;
-    return (PyWeakReference **)((char *)op + offset);
+    return (PyWeakrefControl **)((char *)op + offset);
+}
+
+static inline PyWeakrefControl *
+_PyObject_GET_WEAKREF_CONTROL(PyObject *op)
+{
+    return _Py_atomic_load_ptr(_PyObject_GET_WEAKREFS_CONTROLPTR(op));
 }
 
 

Include/internal/pycore_typeobject.h

@@ -52,10 +52,10 @@ typedef struct {
        they will effectively never get triggered.  However, there
        are also some diagnostic uses for the list of weakrefs,
        so we still keep it. */
-    PyObject *tp_weaklist;
+    PyWeakrefControl *tp_weaklist;
 } static_builtin_state;
 
-static inline PyObject **
+static inline PyWeakrefControl **
 _PyStaticType_GET_WEAKREFS_LISTPTR(static_builtin_state *state)
 {
     assert(state != NULL);

Include/object.h

@@ -347,6 +347,9 @@ PyAPI_FUNC(int) PyObject_IsTrue(PyObject *);
 PyAPI_FUNC(int) PyObject_Not(PyObject *);
 PyAPI_FUNC(int) PyCallable_Check(PyObject *);
 PyAPI_FUNC(void) PyObject_ClearWeakRefs(PyObject *);
+#ifndef Py_LIMITED_API
+PyAPI_FUNC(void) _PyObject_ClearWeakRefsFromDealloc(PyObject *);
+#endif
 
 /* PyObject_Dir(obj) acts like Python builtins.dir(obj), returning a
    list of strings.  PyObject_Dir(NULL) is like builtins.dir(),

Include/weakrefobject.h

@@ -28,14 +28,18 @@ PyAPI_FUNC(PyObject *) PyWeakref_NewRef(PyObject *ob,
 PyAPI_FUNC(PyObject *) PyWeakref_NewProxy(PyObject *ob,
                                           PyObject *callback);
 PyAPI_FUNC(PyObject *) PyWeakref_GetObject(PyObject *ref);
-
+#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030B0000
+PyAPI_FUNC(PyObject *) PyWeakref_FetchObject(PyObject *ref);
+#endif
+#define PyWeakref_LockObject PyWeakref_FetchObject
 
 #ifndef Py_LIMITED_API
 #  define Py_CPYTHON_WEAKREFOBJECT_H
 #  include "cpython/weakrefobject.h"
 #  undef Py_CPYTHON_WEAKREFOBJECT_H
 #endif
 
+
 #ifdef __cplusplus
 }
 #endif

Lib/test/test_capi/test_misc.py

@@ -634,7 +634,7 @@ def test_heaptype_with_weakref(self):
         inst = _testcapi.HeapCTypeWithWeakref()
         ref = weakref.ref(inst)
         self.assertEqual(ref(), inst)
-        self.assertEqual(inst.weakreflist, ref)
+        self.assertEqual(type(inst.weakreflist).__name__, "weakref_control")
 
     def test_heaptype_with_managed_weakref(self):
         inst = _testcapi.HeapCTypeWithManagedWeakref()

Lib/test/test_sys.py

@@ -1582,11 +1582,11 @@ class newstyleclass(object): pass
         # TODO: add check that forces layout of unicodefields
         # weakref
         import weakref
-        check(weakref.ref(int), size('2Pn3P'))
+        check(weakref.ref(int), size('4Pn'))
         # weakproxy
         # XXX
         # weakcallableproxy
-        check(weakref.proxy(int), size('2Pn3P'))
+        check(weakref.proxy(int), size('4Pn'))
 
     def check_slots(self, obj, base, extra):
         expected = sys.getsizeof(base) + struct.calcsize(extra)

Lib/test/test_weakref.py

@@ -79,7 +79,7 @@ def callback(self, ref):
 
 
 @contextlib.contextmanager
-def collect_in_thread(period=0.0001):
+def collect_in_thread(period=0.005):
     """
     Ensure GC collections happen in a different thread, at a high frequency.
     """
@@ -990,9 +990,9 @@ class MyRef(weakref.ref):
         self.assertEqual(weakref.getweakrefcount(o), 3)
         refs = weakref.getweakrefs(o)
         self.assertEqual(len(refs), 3)
-        self.assertIs(r2, refs[0])
-        self.assertIn(r1, refs[1:])
-        self.assertIn(r3, refs[1:])
+        self.assertIn(r2, refs)
+        self.assertIn(r1, refs)
+        self.assertIn(r3, refs)
 
     def test_subclass_refs_dont_conflate_callbacks(self):
         class MyRef(weakref.ref):

Misc/stable_abi.toml

@@ -1577,6 +1577,8 @@
     added = '3.2'
 [function.PyWeakref_GetObject]
     added = '3.2'
+[function.PyWeakref_FetchObject]
+    added = '3.12'
 [function.PyWeakref_NewProxy]
     added = '3.2'
 [function.PyWeakref_NewRef]

Modules/_abc.c

@@ -150,11 +150,10 @@ static PyObject *
 _destroy(PyObject *setweakref, PyObject *objweakref)
 {
     PyObject *set;
-    set = PyWeakref_GET_OBJECT(setweakref);
+    set = PyWeakref_FetchObject(setweakref);
     if (set == Py_None) {
         Py_RETURN_NONE;
     }
-    Py_INCREF(set);
     if (PySet_Discard(set, objweakref) < 0) {
         Py_DECREF(set);
         return NULL;

Modules/_ctypes/_ctypes.c

@@ -244,17 +244,19 @@ PyDict_SetItemProxy(PyObject *dict, PyObject *key, PyObject *item)
     return result;
 }
 
-PyObject *
+static PyObject *
 PyDict_GetItemProxy(PyObject *dict, PyObject *key)
 {
     PyObject *result;
     PyObject *item = PyDict_GetItemWithError(dict, key);
 
     if (item == NULL)
         return NULL;
-    if (!PyWeakref_CheckProxy(item))
+    if (!PyWeakref_CheckProxy(item)) {
+        Py_INCREF(item);
         return item;
-    result = PyWeakref_GET_OBJECT(item);
+    }
+    result = PyWeakref_FetchObject(item);
     if (result == Py_None)
         return NULL;
     return result;
@@ -4818,7 +4820,6 @@ PyCArrayType_from_ctype(PyObject *itemtype, Py_ssize_t length)
         return NULL;
     result = PyDict_GetItemProxy(cache, key);
     if (result) {
-        Py_INCREF(result);
         Py_DECREF(key);
         return result;
     }

Modules/_threadmodule.c

@@ -1091,14 +1091,14 @@ static PyObject *
 _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
 {
     assert(PyWeakref_CheckRef(localweakref));
-    PyObject *obj = PyWeakref_GET_OBJECT(localweakref);
+    PyObject *obj = PyWeakref_FetchObject(localweakref);
     if (obj == Py_None) {
         Py_RETURN_NONE;
     }
 
     /* If the thread-local object is still alive and not being cleared,
        remove the corresponding local dict */
-    localobject *self = (localobject *)Py_NewRef(obj);
+    localobject *self = (localobject *)obj;
     if (self->dummies != NULL) {
         PyObject *ldict;
         ldict = PyDict_GetItemWithError(self->dummies, dummyweakref);
@@ -1387,14 +1387,16 @@ release_sentinel(void *wr_raw)
     /* Tricky: this function is called when the current thread state
        is being deleted.  Therefore, only simple C code can safely
        execute here. */
-    PyObject *obj = PyWeakref_GET_OBJECT(wr);
+    // FIXME(sgross): this isn't simple C code
+    PyObject *obj = PyWeakref_FetchObject(wr);
     lockobject *lock;
     if (obj != Py_None) {
         lock = (lockobject *) obj;
         if (lock->locked) {
             PyThread_release_lock(lock->lock_lock);
             lock->locked = 0;
         }
+        Py_DECREF(obj);
     }
     /* Deallocating a weakref with a NULL callback only calls
        PyObject_GC_Del(), which can't call any Python code. */