From 7ed085111a83f0c4b6fd5fa7df5605c0337222e0 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Jun 2023 00:16:28 +0200 Subject: [PATCH] gh-105927: Add _PyWeakref_GET_REF() internal function Add new pycore_weakref.h internal header file. --- Include/internal/pycore_weakref.h | 40 ++++++++++++++ Makefile.pre.in | 1 + Objects/weakrefobject.c | 89 ++++++++++++++---------------- PCbuild/pythoncore.vcxproj | 1 + PCbuild/pythoncore.vcxproj.filters | 3 + 5 files changed, 85 insertions(+), 49 deletions(-) create mode 100644 Include/internal/pycore_weakref.h diff --git a/Include/internal/pycore_weakref.h b/Include/internal/pycore_weakref.h new file mode 100644 index 00000000000000..4e2b979c91928b --- /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 3a404b0d16403f..e9a8d8ffb71fd2 100644 --- 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 4e18fce9a35ecb..7ffe050597aa39 100644 --- 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("", 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( "", @@ -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,16 @@ 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; \ + } \ } #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 +418,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 +432,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 +446,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 +459,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( "", - 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 +487,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 +532,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 +556,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 +570,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 +584,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 +603,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 +615,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 +623,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 +712,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 +747,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 72d869efb9db67..a68452a916aa3c 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -275,6 +275,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index 5d8b7196c14e6a..bf9b42f4d790e3 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -492,6 +492,9 @@ Include\internal + + Include\internal + Include\internal