diff --git a/Doc/c-api/weakref.rst b/Doc/c-api/weakref.rst index 98ebe711adaeb46..df71c134b7d4713 100644 --- a/Doc/c-api/weakref.rst +++ b/Doc/c-api/weakref.rst @@ -66,5 +66,5 @@ as much as it can. .. c:function:: PyObject* PyWeakref_GET_OBJECT(PyObject *ref) - Similar to :c:func:`PyWeakref_GetObject`, but implemented as a macro that does no - error checking. + Similar to :c:func:`PyWeakref_GetObject`, but implemented as a static inline + function which only check errors with assertions. diff --git a/Include/cpython/weakrefobject.h b/Include/cpython/weakrefobject.h index 9efcc412df9bed4..10ec0f71370b0db 100644 --- a/Include/cpython/weakrefobject.h +++ b/Include/cpython/weakrefobject.h @@ -35,13 +35,28 @@ PyAPI_FUNC(Py_ssize_t) _PyWeakref_GetWeakrefCount(PyWeakReference *head); PyAPI_FUNC(void) _PyWeakref_ClearRef(PyWeakReference *self); -/* 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 #16602. */ -#define PyWeakref_GET_OBJECT(ref) \ - (Py_REFCNT(((PyWeakReference *)(ref))->wr_object) > 0 \ - ? ((PyWeakReference *)(ref))->wr_object \ - : Py_None) + +static inline PyObject* _PyWeakref_GET_OBJECT(PyWeakReference *ref) +{ + PyObject *obj = ref->wr_object; + assert(obj != 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 #16602. */ + if (Py_REFCNT(obj) > 0) { + return obj; + } + else { + return Py_None; + } +} + +static inline PyObject* PyWeakref_GET_OBJECT(PyWeakReference *ref) +{ + assert(PyWeakref_CheckRef(ref)); + return _PyWeakref_GET_OBJECT(ref); +} +#define PyWeakref_GET_OBJECT(ref) PyWeakref_GET_OBJECT((PyWeakReference*)(ref)) diff --git a/Modules/_abc.c b/Modules/_abc.c index b7465c379dddf42..3b11ccdee746b5e 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -155,8 +155,7 @@ _in_weak_set(PyObject *set, PyObject *obj) static PyObject * _destroy(PyObject *setweakref, PyObject *objweakref) { - PyObject *set; - set = PyWeakref_GET_OBJECT(setweakref); + PyObject *set = PyWeakref_GET_OBJECT(setweakref); if (set == Py_None) { Py_RETURN_NONE; } @@ -502,7 +501,6 @@ set_collection_flag_recursive(PyTypeObject *child, unsigned long flag) assert(PyDict_CheckExact(grandchildren)); Py_ssize_t i = 0; while (PyDict_Next(grandchildren, &i, NULL, &grandchildren)) { - assert(PyWeakref_CheckRef(grandchildren)); PyObject *grandchild = PyWeakref_GET_OBJECT(grandchildren); if (PyType_Check(grandchild)) { set_collection_flag_recursive((PyTypeObject *)grandchild, flag); diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 96078c7726d597a..0a64b9784ff250d 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -246,7 +246,7 @@ PyDict_SetItemProxy(PyObject *dict, PyObject *key, PyObject *item) return result; } -PyObject * +static PyObject * PyDict_GetItemProxy(PyObject *dict, PyObject *key) { PyObject *result; @@ -256,7 +256,7 @@ PyDict_GetItemProxy(PyObject *dict, PyObject *key) return NULL; if (!PyWeakref_CheckProxy(item)) return item; - result = PyWeakref_GET_OBJECT(item); + result = _PyWeakref_GET_OBJECT((PyWeakReference*)item); if (result == Py_None) return NULL; return result; diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 9e6e462b59e06c2..d1fba8b0fdf6a28 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1031,7 +1031,6 @@ 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) { Py_RETURN_NONE; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index c46c3d80edbaafd..d9f56685b39e0dd 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -339,7 +339,6 @@ PyType_Modified(PyTypeObject *type) assert(PyDict_CheckExact(raw)); i = 0; while (PyDict_Next(raw, &i, NULL, &ref)) { - assert(PyWeakref_CheckRef(ref)); ref = PyWeakref_GET_OBJECT(ref); if (ref != Py_None) { PyType_Modified(_PyType_CAST(ref)); @@ -4146,7 +4145,6 @@ type___subclasses___impl(PyTypeObject *self) assert(PyDict_CheckExact(raw)); i = 0; while (PyDict_Next(raw, &i, NULL, &ref)) { - assert(PyWeakref_CheckRef(ref)); ref = PyWeakref_GET_OBJECT(ref); if (ref != Py_None) { if (PyList_Append(list, ref) < 0) { @@ -8647,9 +8645,7 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *name, assert(PyDict_CheckExact(subclasses)); i = 0; while (PyDict_Next(subclasses, &i, NULL, &ref)) { - assert(PyWeakref_CheckRef(ref)); PyObject *obj = PyWeakref_GET_OBJECT(ref); - assert(obj != NULL); if (obj == Py_None) { continue; } diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index 892276897522133..8f1c1905583cd2c 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -131,22 +131,22 @@ static PyObject * weakref_call(PyWeakReference *self, PyObject *args, PyObject *kw) { static char *kwlist[] = {NULL}; - - if (PyArg_ParseTupleAndKeywords(args, kw, ":__call__", kwlist)) { - PyObject *object = PyWeakref_GET_OBJECT(self); - Py_INCREF(object); - return (object); + if (!PyArg_ParseTupleAndKeywords(args, kw, ":__call__", kwlist)) { + return NULL; } - return NULL; + + return Py_NewRef(PyWeakref_GET_OBJECT(self)); } static Py_hash_t weakref_hash(PyWeakReference *self) { - if (self->hash != -1) + if (self->hash != -1) { return self->hash; - PyObject* obj = PyWeakref_GET_OBJECT(self); + } + + PyObject *obj = PyWeakref_GET_OBJECT(self); if (obj == Py_None) { PyErr_SetString(PyExc_TypeError, "weak object has gone away"); return -1; @@ -163,7 +163,7 @@ weakref_repr(PyWeakReference *self) { PyObject *name, *repr; _Py_IDENTIFIER(__name__); - PyObject* obj = PyWeakref_GET_OBJECT(self); + PyObject *obj = PyWeakref_GET_OBJECT(self); if (obj == Py_None) { return PyUnicode_FromFormat("", self); @@ -177,17 +177,12 @@ weakref_repr(PyWeakReference *self) if (name == NULL || !PyUnicode_Check(name)) { repr = PyUnicode_FromFormat( "", - self, - Py_TYPE(PyWeakref_GET_OBJECT(self))->tp_name, - obj); + self, Py_TYPE(obj)->tp_name, obj); } else { repr = PyUnicode_FromFormat( "", - self, - Py_TYPE(PyWeakref_GET_OBJECT(self))->tp_name, - obj, - name); + self, Py_TYPE(obj)->tp_name, obj, name); } Py_DECREF(obj); Py_XDECREF(name); @@ -206,8 +201,9 @@ weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op) !PyWeakref_Check(other)) { Py_RETURN_NOTIMPLEMENTED; } - if (PyWeakref_GET_OBJECT(self) == Py_None - || PyWeakref_GET_OBJECT(other) == Py_None) { + PyObject* obj = PyWeakref_GET_OBJECT(self); + PyObject* other_obj = PyWeakref_GET_OBJECT(other); + if (obj == Py_None || other_obj == Py_None) { int res = (self == other); if (op == Py_NE) res = !res; @@ -216,8 +212,6 @@ weakref_richcompare(PyWeakReference* self, PyWeakReference* other, int op) else Py_RETURN_FALSE; } - PyObject* obj = PyWeakref_GET_OBJECT(self); - PyObject* other_obj = PyWeakref_GET_OBJECT(other); Py_INCREF(obj); Py_INCREF(other_obj); PyObject* res = PyObject_RichCompare(obj, other_obj, op); @@ -413,10 +407,18 @@ _PyWeakref_RefType = { }; +static inline PyObject* +proxy_get_object(PyWeakReference *ref) +{ + assert(PyWeakref_CheckProxy(ref)); + return _PyWeakref_GET_OBJECT(ref); +} + + static int proxy_checkref(PyWeakReference *proxy) { - if (PyWeakref_GET_OBJECT(proxy) == Py_None) { + if (proxy_get_object(proxy) == Py_None) { PyErr_SetString(PyExc_ReferenceError, "weakly-referenced object no longer exists"); return 0; @@ -431,9 +433,10 @@ proxy_checkref(PyWeakReference *proxy) */ #define UNWRAP(o) \ if (PyWeakref_CheckProxy(o)) { \ - if (!proxy_checkref((PyWeakReference *)o)) \ + PyWeakReference *_proxy = (PyWeakReference *)o; \ + if (!proxy_checkref(_proxy)) \ return NULL; \ - o = PyWeakref_GET_OBJECT(o); \ + o = proxy_get_object(_proxy); \ } #define WRAP_UNARY(method, generic) \ @@ -500,20 +503,20 @@ WRAP_TERNARY(proxy_call, PyObject_Call) static PyObject * proxy_repr(PyWeakReference *proxy) { + PyObject *obj = proxy_get_object(proxy); return PyUnicode_FromFormat( "", - proxy, - Py_TYPE(PyWeakref_GET_OBJECT(proxy))->tp_name, - PyWeakref_GET_OBJECT(proxy)); + proxy, Py_TYPE(obj)->tp_name, obj); } static int proxy_setattr(PyWeakReference *proxy, PyObject *name, PyObject *value) { - if (!proxy_checkref(proxy)) + if (!proxy_checkref(proxy)) { return -1; - PyObject *obj = PyWeakref_GET_OBJECT(proxy); + } + PyObject *obj = proxy_get_object(proxy); Py_INCREF(obj); int res = PyObject_SetAttr(obj, name, value); Py_DECREF(obj); @@ -567,7 +570,7 @@ WRAP_BINARY(proxy_imatmul, PyNumber_InPlaceMatrixMultiply) static int proxy_bool(PyWeakReference *proxy) { - PyObject *o = PyWeakref_GET_OBJECT(proxy); + PyObject *o = proxy_get_object(proxy); if (!proxy_checkref(proxy)) { return -1; } @@ -594,8 +597,7 @@ proxy_contains(PyWeakReference *proxy, PyObject *value) if (!proxy_checkref(proxy)) return -1; - PyObject *obj = PyWeakref_GET_OBJECT(proxy); - Py_INCREF(obj); + PyObject *obj = Py_NewRef(proxy_get_object(proxy)); int res = PySequence_Contains(obj, value); Py_DECREF(obj); return res; @@ -609,8 +611,7 @@ proxy_length(PyWeakReference *proxy) if (!proxy_checkref(proxy)) return -1; - PyObject *obj = PyWeakref_GET_OBJECT(proxy); - Py_INCREF(obj); + PyObject *obj = Py_NewRef(proxy_get_object(proxy)); Py_ssize_t res = PyObject_Length(obj); Py_DECREF(obj); return res; @@ -624,8 +625,7 @@ proxy_setitem(PyWeakReference *proxy, PyObject *key, PyObject *value) if (!proxy_checkref(proxy)) return -1; - PyObject *obj = PyWeakref_GET_OBJECT(proxy); - Py_INCREF(obj); + PyObject *obj = Py_NewRef(proxy_get_object(proxy)); int res; if (value == NULL) { res = PyObject_DelItem(obj, key); @@ -643,8 +643,7 @@ proxy_iter(PyWeakReference *proxy) { if (!proxy_checkref(proxy)) return NULL; - PyObject *obj = PyWeakref_GET_OBJECT(proxy); - Py_INCREF(obj); + PyObject *obj = Py_NewRef(proxy_get_object(proxy)); PyObject* res = PyObject_GetIter(obj); Py_DECREF(obj); return res; @@ -656,7 +655,7 @@ proxy_iternext(PyWeakReference *proxy) if (!proxy_checkref(proxy)) return NULL; - PyObject *obj = PyWeakref_GET_OBJECT(proxy); + PyObject *obj = proxy_get_object(proxy); if (!PyIter_Check(obj)) { PyErr_Format(PyExc_TypeError, "Weakref proxy referenced a non-iterator '%.200s' object",