gh-105927: Avoid calling PyWeakref_GET_OBJECT() (#105997)

vstinner · web-flow · commit 46a3190fcf85 · 2023-06-22T22:31:31.000+02:00
* Replace PyWeakref_GET_OBJECT() with _PyWeakref_GET_REF().
* _sqlite/blob.c now holds a strong reference to the blob object
  while calling close_blob().
* _xidregistry_find_type() now holds a strong reference to registered
  while using it.
diff --git a/Modules/_sqlite/blob.c b/Modules/_sqlite/blob.c
@@ -1,5 +1,10 @@
+#ifndef Py_BUILD_CORE_BUILTIN
+#  define Py_BUILD_CORE_MODULE 1
+#endif
+
 #include "blob.h"
 #include "util.h"
+#include "pycore_weakref.h"       // _PyWeakref_GET_REF()
 
 #define clinic_state() (pysqlite_get_state_by_type(Py_TYPE(self)))
 #include "clinic/blob.c.h"
@@ -97,10 +102,12 @@ pysqlite_close_all_blobs(pysqlite_Connection *self)
 {
     for (int i = 0; i < PyList_GET_SIZE(self->blobs); i++) {
         PyObject *weakref = PyList_GET_ITEM(self->blobs, i);
-        PyObject *blob = PyWeakref_GetObject(weakref);
-        if (!Py_IsNone(blob)) {
-            close_blob((pysqlite_Blob *)blob);
+        PyObject *blob = _PyWeakref_GET_REF(weakref);
+        if (blob == NULL) {
+            continue;
         }
+        close_blob((pysqlite_Blob *)blob);
+        Py_DECREF(blob);
     }
 }
 
diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
@@ -140,7 +140,11 @@ weakref_vectorcall(PyObject *self, PyObject *const *args,
     if (!_PyArg_CheckPositional("weakref", nargs, 0, 0)) {
         return NULL;
     }
-    return Py_NewRef(PyWeakref_GET_OBJECT(self));
+    PyObject *obj = _PyWeakref_GET_REF(self);
+    if (obj == NULL) {
+        Py_RETURN_NONE;
+    }
+    return obj;
 }
 
 static Py_hash_t
diff --git a/Python/pystate.c b/Python/pystate.c
@@ -13,7 +13,8 @@
 #include "pycore_pymem.h"         // _PyMem_SetDefaultAllocator()
 #include "pycore_pystate.h"
 #include "pycore_runtime_init.h"  // _PyRuntimeState_INIT
-#include "pycore_sysmodule.h"
+#include "pycore_sysmodule.h"     // _PySys_Audit()
+#include "pycore_weakref.h"       // _PyWeakref_GET_REF()
 
 /* --------------------------------------------------------------------------
 CAUTION
@@ -2589,16 +2590,18 @@ _xidregistry_find_type(struct _xidregistry *xidregistry, PyTypeObject *cls)
 {
     struct _xidregitem *cur = xidregistry->head;
     while (cur != NULL) {
-        PyObject *registered = PyWeakref_GetObject(cur->cls);
-        if (registered == Py_None) {
+        PyObject *registered = _PyWeakref_GET_REF(cur->cls);
+        if (registered == NULL) {
             // The weakly ref'ed object was freed.
             cur = _xidregistry_remove_entry(xidregistry, cur);
         }
         else {
             assert(PyType_Check(registered));
             if (registered == (PyObject *)cls) {
+                Py_DECREF(registered);
                 return cur;
             }
+            Py_DECREF(registered);
             cur = cur->next;
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,10 @@`
	`1`	`+#ifndef Py_BUILD_CORE_BUILTIN`
	`2`	`+# define Py_BUILD_CORE_MODULE 1`
	`3`	`+#endif`
	`4`	`+`
`1`	`5`	`#include "blob.h"`
`2`	`6`	`#include "util.h"`
	`7`	`+#include "pycore_weakref.h" // _PyWeakref_GET_REF()`
`3`	`8`
`4`	`9`	`#define clinic_state() (pysqlite_get_state_by_type(Py_TYPE(self)))`
`5`	`10`	`#include "clinic/blob.c.h"`
`@@ -97,10 +102,12 @@ pysqlite_close_all_blobs(pysqlite_Connection *self)`
`97`	`102`	`{`
`98`	`103`	`for (int i = 0; i < PyList_GET_SIZE(self->blobs); i++) {`
`99`	`104`	`PyObject *weakref = PyList_GET_ITEM(self->blobs, i);`
`100`		`- PyObject *blob = PyWeakref_GetObject(weakref);`
`101`		`- if (!Py_IsNone(blob)) {`
`102`		`- close_blob((pysqlite_Blob *)blob);`
	`105`	`+ PyObject *blob = _PyWeakref_GET_REF(weakref);`
	`106`	`+ if (blob == NULL) {`
	`107`	`+ continue;`
`103`	`108`	`}`
	`109`	`+ close_blob((pysqlite_Blob *)blob);`
	`110`	`+ Py_DECREF(blob);`
`104`	`111`	`}`
`105`	`112`	`}`
`106`	`113`
Original file line number	Diff line number	Diff line change
`@@ -140,7 +140,11 @@ weakref_vectorcall(PyObject self, PyObject const *args,`
`140`	`140`	`if (!_PyArg_CheckPositional("weakref", nargs, 0, 0)) {`
`141`	`141`	`return NULL;`
`142`	`142`	`}`
`143`		`- return Py_NewRef(PyWeakref_GET_OBJECT(self));`
	`143`	`+ PyObject *obj = _PyWeakref_GET_REF(self);`
	`144`	`+ if (obj == NULL) {`
	`145`	`+ Py_RETURN_NONE;`
	`146`	`+ }`
	`147`	`+ return obj;`
`144`	`148`	`}`
`145`	`149`
`146`	`150`	`static Py_hash_t`
Original file line number	Diff line number	Diff line change
`@@ -13,7 +13,8 @@`
`13`	`13`	`#include "pycore_pymem.h" // _PyMem_SetDefaultAllocator()`
`14`	`14`	`#include "pycore_pystate.h"`
`15`	`15`	`#include "pycore_runtime_init.h" // _PyRuntimeState_INIT`
`16`		`-#include "pycore_sysmodule.h"`
	`16`	`+#include "pycore_sysmodule.h" // _PySys_Audit()`
	`17`	`+#include "pycore_weakref.h" // _PyWeakref_GET_REF()`
`17`	`18`
`18`	`19`	`/* --------------------------------------------------------------------------`
`19`	`20`	`CAUTION`
`@@ -2589,16 +2590,18 @@ _xidregistry_find_type(struct _xidregistry xidregistry, PyTypeObject cls)`
`2589`	`2590`	`{`
`2590`	`2591`	`struct _xidregitem *cur = xidregistry->head;`
`2591`	`2592`	`while (cur != NULL) {`
`2592`		`- PyObject *registered = PyWeakref_GetObject(cur->cls);`
`2593`		`- if (registered == Py_None) {`
	`2593`	`+ PyObject *registered = _PyWeakref_GET_REF(cur->cls);`
	`2594`	`+ if (registered == NULL) {`
`2594`	`2595`	`// The weakly ref'ed object was freed.`
`2595`	`2596`	`cur = _xidregistry_remove_entry(xidregistry, cur);`
`2596`	`2597`	`}`
`2597`	`2598`	`else {`
`2598`	`2599`	`assert(PyType_Check(registered));`
`2599`	`2600`	`if (registered == (PyObject *)cls) {`
	`2601`	`+ Py_DECREF(registered);`
`2600`	`2602`	`return cur;`
`2601`	`2603`	`}`
	`2604`	`+ Py_DECREF(registered);`
`2602`	`2605`	`cur = cur->next;`
`2603`	`2606`	`}`
`2604`	`2607`	`}`