gh-105927: _abc and _thread use PyWeakref_GetRef() (#105961)

vstinner · web-flow · commit fb1e691e4bc7 · 2023-06-21T15:44:25.000+02:00
Hold a strong reference on the object, rather than using a borrowed reference:
replace PyWeakref_GET_OBJECT() with PyWeakref_GetRef() and
_PyWeakref_GET_REF().

Remove assert(PyWeakref_CheckRef(localweakref)) since it's already
tested by _PyWeakref_GET_REF().
diff --git a/Modules/_abc.c b/Modules/_abc.c
@@ -8,6 +8,7 @@
 #include "pycore_object.h"        // _PyType_GetSubclasses()
 #include "pycore_runtime.h"       // _Py_ID()
 #include "pycore_typeobject.h"    // _PyType_GetMRO()
+#include "pycore_weakref.h"       // _PyWeakref_GET_REF()
 #include "clinic/_abc.c.h"
 
 /*[clinic input]
@@ -150,12 +151,10 @@ _in_weak_set(PyObject *set, PyObject *obj)
 static PyObject *
 _destroy(PyObject *setweakref, PyObject *objweakref)
 {
-    PyObject *set;
-    set = PyWeakref_GET_OBJECT(setweakref);
-    if (set == Py_None) {
+    PyObject *set = _PyWeakref_GET_REF(setweakref);
+    if (set == NULL) {
         Py_RETURN_NONE;
     }
-    Py_INCREF(set);
     if (PySet_Discard(set, objweakref) < 0) {
         Py_DECREF(set);
         return NULL;
@@ -843,16 +842,16 @@ subclasscheck_check_registry(_abc_data *impl, PyObject *subclass,
     assert(i == registry_size);
 
     for (i = 0; i < registry_size; i++) {
-        PyObject *rkey = PyWeakref_GetObject(copy[i]);
-        if (rkey == NULL) {
+        PyObject *rkey;
+        if (PyWeakref_GetRef(copy[i], &rkey) < 0) {
             // Someone inject non-weakref type in the registry.
             ret = -1;
             break;
         }
-        if (rkey == Py_None) {
+
+        if (rkey == NULL) {
             continue;
         }
-        Py_INCREF(rkey);
         int r = PyObject_IsSubclass(subclass, rkey);
         Py_DECREF(rkey);
         if (r < 0) {
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
@@ -7,6 +7,7 @@
 #include "pycore_moduleobject.h"  // _PyModule_GetState()
 #include "pycore_pylifecycle.h"
 #include "pycore_pystate.h"       // _PyThreadState_SetCurrent()
+#include "pycore_weakref.h"       // _PyWeakref_GET_REF()
 #include <stddef.h>               // offsetof()
 #include "structmember.h"         // PyMemberDef
 
@@ -1024,25 +1025,23 @@ local_getattro(localobject *self, PyObject *name)
 static PyObject *
 _localdummy_destroyed(PyObject *localweakref, PyObject *dummyweakref)
 {
-    assert(PyWeakref_CheckRef(localweakref));
-    PyObject *obj = PyWeakref_GET_OBJECT(localweakref);
-    if (obj == Py_None) {
+    localobject *self = (localobject *)_PyWeakref_GET_REF(localweakref);
+    if (self == NULL) {
         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);
     if (self->dummies != NULL) {
         PyObject *ldict;
         ldict = PyDict_GetItemWithError(self->dummies, dummyweakref);
         if (ldict != NULL) {
             PyDict_DelItem(self->dummies, dummyweakref);
         }
         if (PyErr_Occurred())
-            PyErr_WriteUnraisable(obj);
+            PyErr_WriteUnraisable((PyObject*)self);
     }
-    Py_DECREF(obj);
+    Py_DECREF(self);
     Py_RETURN_NONE;
 }
 
@@ -1314,24 +1313,25 @@ This function is meant for internal and specialized purposes only.\n\
 In most applications `threading.enumerate()` should be used instead.");
 
 static void
-release_sentinel(void *wr_raw)
+release_sentinel(void *weakref_raw)
 {
-    PyObject *wr = _PyObject_CAST(wr_raw);
+    PyObject *weakref = _PyObject_CAST(weakref_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);
-    lockobject *lock;
-    if (obj != Py_None) {
-        lock = (lockobject *) obj;
+    lockobject *lock = (lockobject *)_PyWeakref_GET_REF(weakref);
+    if (lock != NULL) {
         if (lock->locked) {
             PyThread_release_lock(lock->lock_lock);
             lock->locked = 0;
         }
+        Py_DECREF(lock);
     }
+
     /* Deallocating a weakref with a NULL callback only calls
        PyObject_GC_Del(), which can't call any Python code. */
-    Py_DECREF(wr);
+    Py_DECREF(weakref);
 }
 
 static PyObject *
