gh-105927: Add _PyWeakref_GET_REF() internal function (#105929)

Add new pycore_weakref.h internal header file.

diff --git a/Include/internal/pycore_weakref.h b/Include/internal/pycore_weakref.h
new file mode 100644 (file)
index 0000000..4e2b979
--- /dev/null
+++ b/Include/internal/pycore_weakref.h
@@ -0,0 +1,40 @@
+#ifndef Py_INTERNAL_WEAKREF_H
+#define Py_INTERNAL_WEAKREF_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef Py_BUILD_CORE
+#  error "this header requires Py_BUILD_CORE define"
+#endif
+
+static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj) {
+    assert(PyWeakref_Check(ref_obj));
+    PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
+    PyObject *obj = ref->wr_object;
+
+    if (obj == Py_None) {
+        // clear_weakref() was called
+        return NULL;
+    }
+
+    // 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.
+    Py_ssize_t refcnt = Py_REFCNT(obj);
+    if (refcnt == 0) {
+        return NULL;
+    }
+
+    assert(refcnt > 0);
+    return Py_NewRef(obj);
+}
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* !Py_INTERNAL_WEAKREF_H */
+

diff --git a/Makefile.pre.in b/Makefile.pre.in
index 3a404b0d16403f4ae471ecc7492f92543961c282..e9a8d8ffb71fd2e7247d95b336a826a5a115b6e9 100644 (file)
--- a/Makefile.pre.in
+++ b/Makefile.pre.in
@@ -1801,6 +1801,7 @@ PYTHON_HEADERS= \
                $(srcdir)/Include/internal/pycore_unicodeobject.h \
                $(srcdir)/Include/internal/pycore_unicodeobject_generated.h \
                $(srcdir)/Include/internal/pycore_warnings.h \
+               $(srcdir)/Include/internal/pycore_weakref.h \
                $(DTRACE_HEADERS) \
                @PLATFORM_HEADERS@ \
                \

diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c
index 4e18fce9a35ecb1989ca2fa38d020add930fc936..9995a7756570cace284d453675f55b8da566952c 100644 (file)
--- a/Objects/weakrefobject.c
+++ b/Objects/weakrefobject.c
@@ -1,5 +1,6 @@
 #include "Python.h"
 #include "pycore_object.h"        // _PyObject_GET_WEAKREFS_LISTPTR()
+#include "pycore_weakref.h"       // _PyWeakref_GET_REF()
 #include "structmember.h"         // PyMemberDef
 
 
@@ -147,12 +148,11 @@ weakref_hash(PyWeakReference *self)
 {
     if (self->hash != -1)
         return self->hash;
-    PyObject* obj = PyWeakref_GET_OBJECT(self);
-    if (obj == Py_None) {
+    PyObject* obj = _PyWeakref_GET_REF((PyObject*)self);
+    if (obj == NULL) {
         PyErr_SetString(PyExc_TypeError, "weak object has gone away");
         return -1;
     }
-    Py_INCREF(obj);
     self->hash = PyObject_Hash(obj);
     Py_DECREF(obj);
     return self->hash;
@@ -162,15 +162,13 @@ weakref_hash(PyWeakReference *self)
 static PyObject *
 weakref_repr(PyObject *self)
 {
-    PyObject *name, *repr;
-    PyObject* obj = PyWeakref_GET_OBJECT(self);
-
-    if (obj == Py_None) {
+    PyObject* obj = _PyWeakref_GET_REF(self);
+    if (obj == NULL) {
         return PyUnicode_FromFormat("<weakref at %p; dead>", self);
     }
 
-    Py_INCREF(obj);
-    name = _PyObject_LookupSpecial(obj, &_Py_ID(__name__));
+    PyObject *name = _PyObject_LookupSpecial(obj, &_Py_ID(__name__));
+    PyObject *repr;
     if (name == NULL || !PyUnicode_Check(name)) {
         repr = PyUnicode_FromFormat(
             "<weakref at %p; to '%s' at %p>",
@@ -191,16 +189,18 @@ weakref_repr(PyObject *self)
    gone away, they are equal if they are identical. */
 
 static PyObject *
-weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op)
+weakref_richcompare(PyObject* self, PyObject* other, int op)
 {
     if ((op != Py_EQ && op != Py_NE) ||
         !PyWeakref_Check(self) ||
         !PyWeakref_Check(other)) {
         Py_RETURN_NOTIMPLEMENTED;
     }
-    PyObject* obj = PyWeakref_GET_OBJECT(self);
-    PyObject* other_obj = PyWeakref_GET_OBJECT(other);
-    if (obj == Py_None || other_obj == Py_None) {
+    PyObject* obj = _PyWeakref_GET_REF(self);
+    PyObject* other_obj = _PyWeakref_GET_REF(other);
+    if (obj == NULL || other_obj == NULL) {
+        Py_XDECREF(obj);
+        Py_XDECREF(other_obj);
         int res = (self == other);
         if (op == Py_NE)
             res = !res;
@@ -209,8 +209,6 @@ weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op)
         else
             Py_RETURN_FALSE;
     }
-    Py_INCREF(obj);
-    Py_INCREF(other_obj);
     PyObject* res = PyObject_RichCompare(obj, other_obj, op);
     Py_DECREF(obj);
     Py_DECREF(other_obj);
@@ -372,7 +370,7 @@ _PyWeakref_RefType = {
                 Py_TPFLAGS_HAVE_VECTORCALL | Py_TPFLAGS_BASETYPE,
     .tp_traverse = (traverseproc)gc_traverse,
     .tp_clear = (inquiry)gc_clear,
-    .tp_richcompare = (richcmpfunc)weakref_richcompare,
+    .tp_richcompare = weakref_richcompare,
     .tp_methods = weakref_methods,
     .tp_members = weakref_members,
     .tp_init = weakref___init__,
@@ -385,7 +383,7 @@ _PyWeakref_RefType = {
 static bool
 proxy_check_ref(PyObject *obj)
 {
-    if (obj == Py_None) {
+    if (obj == NULL) {
         PyErr_SetString(PyExc_ReferenceError,
                         "weakly-referenced object no longer exists");
         return false;
@@ -400,16 +398,19 @@ proxy_check_ref(PyObject *obj)
  */
 #define UNWRAP(o) \
         if (PyWeakref_CheckProxy(o)) { \
-            o = PyWeakref_GET_OBJECT(o); \
-            if (!proxy_check_ref(o)) \
+            o = _PyWeakref_GET_REF(o); \
+            if (!proxy_check_ref(o)) { \
                 return NULL; \
+            } \
+        } \
+        else { \
+            Py_INCREF(o); \
         }
 
 #define WRAP_UNARY(method, generic) \
     static PyObject * \
     method(PyObject *proxy) { \
         UNWRAP(proxy); \
-        Py_INCREF(proxy); \
         PyObject* res = generic(proxy); \
         Py_DECREF(proxy); \
         return res; \
@@ -420,8 +421,6 @@ proxy_check_ref(PyObject *obj)
     method(PyObject *x, PyObject *y) { \
         UNWRAP(x); \
         UNWRAP(y); \
-        Py_INCREF(x); \
-        Py_INCREF(y); \
         PyObject* res = generic(x, y); \
         Py_DECREF(x); \
         Py_DECREF(y); \
@@ -436,11 +435,9 @@ proxy_check_ref(PyObject *obj)
     method(PyObject *proxy, PyObject *v, PyObject *w) { \
         UNWRAP(proxy); \
         UNWRAP(v); \
-        if (w != NULL) \
+        if (w != NULL) { \
             UNWRAP(w); \
-        Py_INCREF(proxy); \
-        Py_INCREF(v); \
-        Py_XINCREF(w); \
+        } \
         PyObject* res = generic(proxy, v, w); \
         Py_DECREF(proxy); \
         Py_DECREF(v); \
@@ -452,7 +449,6 @@ proxy_check_ref(PyObject *obj)
     static PyObject * \
     method(PyObject *proxy, PyObject *Py_UNUSED(ignored)) { \
             UNWRAP(proxy); \
-            Py_INCREF(proxy); \
             PyObject* res = PyObject_CallMethodNoArgs(proxy, &_Py_ID(SPECIAL)); \
             Py_DECREF(proxy); \
             return res; \
@@ -466,24 +462,24 @@ WRAP_UNARY(proxy_str, PyObject_Str)
 WRAP_TERNARY(proxy_call, PyObject_Call)
 
 static PyObject *
-proxy_repr(PyWeakReference *proxy)
+proxy_repr(PyObject *proxy)
 {
-    return PyUnicode_FromFormat(
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
+    PyObject *repr = PyUnicode_FromFormat(
         "<weakproxy at %p to %s at %p>",
-        proxy,
-        Py_TYPE(PyWeakref_GET_OBJECT(proxy))->tp_name,
-        PyWeakref_GET_OBJECT(proxy));
+        proxy, Py_TYPE(obj)->tp_name, obj);
+    Py_DECREF(obj);
+    return repr;
 }
 
 
 static int
 proxy_setattr(PyObject *proxy, PyObject *name, PyObject *value)
 {
-    PyObject *obj = PyWeakref_GET_OBJECT(proxy);
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(obj)) {
         return -1;
     }
-    Py_INCREF(obj);
     int res = PyObject_SetAttr(obj, name, value);
     Py_DECREF(obj);
     return res;
@@ -494,7 +490,10 @@ proxy_richcompare(PyObject *proxy, PyObject *v, int op)
 {
     UNWRAP(proxy);
     UNWRAP(v);
-    return PyObject_RichCompare(proxy, v, op);
+    PyObject* res = PyObject_RichCompare(proxy, v, op);
+    Py_DECREF(proxy);
+    Py_DECREF(v);
+    return res;
 }
 
 /* number slots */
@@ -536,11 +535,10 @@ WRAP_BINARY(proxy_imatmul, PyNumber_InPlaceMatrixMultiply)
 static int
 proxy_bool(PyObject *proxy)
 {
-    PyObject *o = PyWeakref_GET_OBJECT(proxy);
+    PyObject *o = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(o)) {
         return -1;
     }
-    Py_INCREF(o);
     int res = PyObject_IsTrue(o);
     Py_DECREF(o);
     return res;
@@ -561,11 +559,10 @@ proxy_dealloc(PyWeakReference *self)
 static int
 proxy_contains(PyObject *proxy, PyObject *value)
 {
-    PyObject *obj = PyWeakref_GET_OBJECT(proxy);
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(obj)) {
         return -1;
     }
-    Py_INCREF(obj);
     int res = PySequence_Contains(obj, value);
     Py_DECREF(obj);
     return res;
@@ -576,11 +573,10 @@ proxy_contains(PyObject *proxy, PyObject *value)
 static Py_ssize_t
 proxy_length(PyObject *proxy)
 {
-    PyObject *obj = PyWeakref_GET_OBJECT(proxy);
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(obj)) {
         return -1;
     }
-    Py_INCREF(obj);
     Py_ssize_t res = PyObject_Length(obj);
     Py_DECREF(obj);
     return res;
@@ -591,11 +587,10 @@ WRAP_BINARY(proxy_getitem, PyObject_GetItem)
 static int
 proxy_setitem(PyObject *proxy, PyObject *key, PyObject *value)
 {
-    PyObject *obj = PyWeakref_GET_OBJECT(proxy);
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(obj)) {
         return -1;
     }
-    Py_INCREF(obj);
     int res;
     if (value == NULL) {
         res = PyObject_DelItem(obj, key);
@@ -611,11 +606,10 @@ proxy_setitem(PyObject *proxy, PyObject *key, PyObject *value)
 static PyObject *
 proxy_iter(PyObject *proxy)
 {
-    PyObject *obj = PyWeakref_GET_OBJECT(proxy);
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(obj)) {
         return NULL;
     }
-    Py_INCREF(obj);
     PyObject* res = PyObject_GetIter(obj);
     Py_DECREF(obj);
     return res;
@@ -624,7 +618,7 @@ proxy_iter(PyObject *proxy)
 static PyObject *
 proxy_iternext(PyObject *proxy)
 {
-    PyObject *obj = PyWeakref_GET_OBJECT(proxy);
+    PyObject *obj = _PyWeakref_GET_REF(proxy);
     if (!proxy_check_ref(obj)) {
         return NULL;
     }
@@ -632,9 +626,9 @@ proxy_iternext(PyObject *proxy)
         PyErr_Format(PyExc_TypeError,
             "Weakref proxy referenced a non-iterator '%.200s' object",
             Py_TYPE(obj)->tp_name);
+        Py_DECREF(obj);
         return NULL;
     }
-    Py_INCREF(obj);
     PyObject* res = PyIter_Next(obj);
     Py_DECREF(obj);
     return res;
@@ -721,7 +715,7 @@ _PyWeakref_ProxyType = {
     0,                                  /* tp_getattr */
     0,                                  /* tp_setattr */
     0,                                  /* tp_as_async */
-    (reprfunc)proxy_repr,               /* tp_repr */
+    proxy_repr,                         /* tp_repr */
     &proxy_as_number,                   /* tp_as_number */
     &proxy_as_sequence,                 /* tp_as_sequence */
     &proxy_as_mapping,                  /* tp_as_mapping */
@@ -756,7 +750,7 @@ _PyWeakref_CallableProxyType = {
     0,                                  /* tp_getattr */
     0,                                  /* tp_setattr */
     0,                                  /* tp_as_async */
-    (unaryfunc)proxy_repr,              /* tp_repr */
+    proxy_repr,                         /* tp_repr */
     &proxy_as_number,                   /* tp_as_number */
     &proxy_as_sequence,                 /* tp_as_sequence */
     &proxy_as_mapping,                  /* tp_as_mapping */

diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj
index 72d869efb9db6702bf54018f4e4af2f4eee5aaab..a68452a916aa3cd87bdda122634d246f739c280d 100644 (file)
--- a/PCbuild/pythoncore.vcxproj
+++ b/PCbuild/pythoncore.vcxproj
@@ -275,6 +275,7 @@
     <ClInclude Include="..\Include\internal\pycore_unicodeobject.h" />
     <ClInclude Include="..\Include\internal\pycore_unicodeobject_generated.h" />
     <ClInclude Include="..\Include\internal\pycore_warnings.h" />
+    <ClInclude Include="..\Include\internal\pycore_weakref.h" />
     <ClInclude Include="..\Include\interpreteridobject.h" />
     <ClInclude Include="..\Include\intrcheck.h" />
     <ClInclude Include="..\Include\iterobject.h" />

diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters
index 5d8b7196c14e6a1f53d02c6be7aedf3adb88cbae..bf9b42f4d790e32f3aee5104f16306beb0217136 100644 (file)
--- a/PCbuild/pythoncore.vcxproj.filters
+++ b/PCbuild/pythoncore.vcxproj.filters
@@ -492,6 +492,9 @@
     <ClInclude Include="..\Include\internal\pycore_warnings.h">
       <Filter>Include\internal</Filter>
     </ClInclude>
+    <ClInclude Include="..\Include\internal\pycore_weakref.h">
+      <Filter>Include\internal</Filter>
+    </ClInclude>
     <ClInclude Include="..\Include\internal\pycore_abstract.h">
       <Filter>Include\internal</Filter>
     </ClInclude>