From 8de770dc77ae1b24b6ba63657cd4c463c0ff663e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Jun 2023 08:57:42 +0200 Subject: [PATCH 1/5] gh-105927: Add PyWeakref_GetRef() function * Add tests on PyWeakref_NewRef(), PyWeakref_GetObject(), PyWeakref_GET_OBJECT() and PyWeakref_GetRef(). * PyWeakref_GetObject() now raises a TypeError if the argument is not a weak reference, instead of SystemError. --- Doc/c-api/weakref.rst | 10 +++ Doc/data/refcounts.dat | 4 ++ Doc/data/stable_abi.dat | 1 + Doc/whatsnew/3.13.rst | 4 ++ Include/weakrefobject.h | 1 + Lib/test/test_stable_abi_ctypes.py | 1 + Lib/test/test_weakref.py | 4 ++ ...-06-20-08-59-05.gh-issue-105927.DfGeEA.rst | 3 + Misc/stable_abi.toml | 2 + Modules/_testcapimodule.c | 68 +++++++++++++++++++ Objects/weakrefobject.c | 24 ++++++- PC/python3dll.c | 1 + 12 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/C API/2023-06-20-08-59-05.gh-issue-105927.DfGeEA.rst diff --git a/Doc/c-api/weakref.rst b/Doc/c-api/weakref.rst index f27ec4411b4a26..cb7b26e17bae35 100644 --- a/Doc/c-api/weakref.rst +++ b/Doc/c-api/weakref.rst @@ -51,6 +51,16 @@ as much as it can. ``None``, or ``NULL``, this will return ``NULL`` and raise :exc:`TypeError`. +.. c:function:: int PyWeakref_GetRef(PyObject *ref, PyObject **pobj) + + Get the referenced object from a weak reference, *ref*, into ``*pobj``. + Return 0 on success. Raise an exception and return -1 on error. + + If the referent is no longer live, set ``*pobj`` to ``NULL`` and return 0. + + .. versionadded:: 3.13 + + .. c:function:: PyObject* PyWeakref_GetObject(PyObject *ref) Return the referenced object from a weak reference, *ref*. If the referent is diff --git a/Doc/data/refcounts.dat b/Doc/data/refcounts.dat index d707cc34c98c97..bbd0bc6d92f3ff 100644 --- a/Doc/data/refcounts.dat +++ b/Doc/data/refcounts.dat @@ -2810,6 +2810,10 @@ PyWeakref_GET_OBJECT:PyObject*:ref:0: PyWeakref_GetObject:PyObject*::0: PyWeakref_GetObject:PyObject*:ref:0: +PyWeakref_GetRef:int::: +PyWeakref_GetRef:PyObject*:ref:0: +PyWeakref_GetRef:PyObject**:pobj:+1: + PyWeakref_NewProxy:PyObject*::+1: PyWeakref_NewProxy:PyObject*:ob:0: PyWeakref_NewProxy:PyObject*:callback:0: diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index a3fde01cf67f3c..7fb002cd80369b 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -781,6 +781,7 @@ function,PyVectorcall_Call,3.12,, function,PyVectorcall_NARGS,3.12,, type,PyWeakReference,3.2,,opaque function,PyWeakref_GetObject,3.2,, +function,PyWeakref_GetRef,3.13,, function,PyWeakref_NewProxy,3.2,, function,PyWeakref_NewRef,3.2,, var,PyWrapperDescr_Type,3.2,, diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index bbe02a9e85b42c..6cf2bd2426370c 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -431,6 +431,10 @@ New Features of a :term:`borrowed reference`. (Contributed by Victor Stinner in :gh:`105922`.) +* Add :c:func:`PyWeakref_GetRef` function: similar to + :c:func:`PyWeakref_GetObject` but returns a :term:`strong reference`, or + ``NULL`` if the referent is no longer live. + (Contributed by Victor Stinner in :gh:`105927`.) Porting to Python 3.13 ---------------------- diff --git a/Include/weakrefobject.h b/Include/weakrefobject.h index 8e1fa1b9286a77..2c69f9e4564ab3 100644 --- a/Include/weakrefobject.h +++ b/Include/weakrefobject.h @@ -28,6 +28,7 @@ PyAPI_FUNC(PyObject *) PyWeakref_NewRef(PyObject *ob, PyAPI_FUNC(PyObject *) PyWeakref_NewProxy(PyObject *ob, PyObject *callback); PyAPI_FUNC(PyObject *) PyWeakref_GetObject(PyObject *ref); +PyAPI_FUNC(int) PyWeakref_GetRef(PyObject *ref, PyObject **pobj); #ifndef Py_LIMITED_API diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index c26dff5aaf370b..038c978e7bbd02 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -794,6 +794,7 @@ def test_windows_feature_macros(self): "PyVectorcall_Call", "PyVectorcall_NARGS", "PyWeakref_GetObject", + "PyWeakref_GetRef", "PyWeakref_NewProxy", "PyWeakref_NewRef", "PyWrapperDescr_Type", diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index 1bc1d05f7daba9..f9fea0162634a2 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -967,6 +967,10 @@ def __del__(self): pass del x support.gc_collect() + @support.cpython_only + def test_capi(self): + import _testcapi + _testcapi.check_weakref_capi(C) class SubclassableWeakrefTestCase(TestBase): diff --git a/Misc/NEWS.d/next/C API/2023-06-20-08-59-05.gh-issue-105927.DfGeEA.rst b/Misc/NEWS.d/next/C API/2023-06-20-08-59-05.gh-issue-105927.DfGeEA.rst new file mode 100644 index 00000000000000..afa40c8ef5d686 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-06-20-08-59-05.gh-issue-105927.DfGeEA.rst @@ -0,0 +1,3 @@ +Add :c:func:`PyWeakref_GetRef` function: similar to +:c:func:`PyWeakref_GetObject` but returns a :term:`strong reference`, or +``NULL`` if the referent is no longer live. Patch by Victor Stinner. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 7025ed4e66b698..bc7259f11816f3 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2430,3 +2430,5 @@ added = '3.12' [function.PyImport_AddModuleRef] added = '3.13' +[function.PyWeakref_GetRef] + added = '3.13' diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9d4517c8f68b09..c6d1cd1ae7824a 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3372,6 +3372,73 @@ check_pyimport_addmodule(PyObject *self, PyObject *args) } +static PyObject * +check_weakref_capi(PyObject *self, PyObject *factory) +{ + // obj = factory() + PyObject *obj = PyObject_CallNoArgs(factory); + if (obj == NULL) { + return NULL; + } + Py_ssize_t refcnt = Py_REFCNT(obj); + assert(refcnt == 1); + + // test PyWeakref_GetRef() + PyObject *weakref = PyWeakref_NewRef(obj, NULL); + if (weakref == NULL) { + Py_DECREF(obj); + return NULL; + } + assert(PyWeakref_Check(weakref)); + assert(PyWeakref_CheckRefExact(weakref)); + assert(PyWeakref_CheckRefExact(weakref)); + assert(Py_REFCNT(obj) == refcnt); + + // test PyWeakref_GetRef() + PyObject *ref1; + assert(PyWeakref_GetRef(weakref, &ref1) == 0); + assert(ref1 == obj); + assert(Py_REFCNT(obj) == (refcnt + 1)); + Py_DECREF(ref1); + + // test PyWeakref_GetObject() + PyObject *ref2 = PyWeakref_GetObject(weakref); + assert(ref2 == obj); + assert(Py_REFCNT(obj) == refcnt); + + // test PyWeakref_GET_OBJECT() + PyObject *ref3 = PyWeakref_GET_OBJECT(weakref); + assert(ref3 == obj); + assert(Py_REFCNT(obj) == refcnt); + + // delete the object + assert(refcnt == 1); + Py_DECREF(obj); + assert(PyWeakref_GET_OBJECT(weakref) == Py_None); + PyObject *ref4; + assert(PyWeakref_GetRef(weakref, &ref4) == 0); + assert(ref4 == NULL); + + // None is not a weak reference object + PyObject *invalid_weakref = Py_None; + assert(!PyWeakref_Check(invalid_weakref)); + assert(!PyWeakref_CheckRefExact(invalid_weakref)); + assert(!PyWeakref_CheckRefExact(invalid_weakref)); + + assert(!PyErr_Occurred()); + PyObject *ref5 = factory; // marker to check that value was set + assert(PyWeakref_GetRef(invalid_weakref, &ref5) == -1); + assert(PyErr_ExceptionMatches(PyExc_TypeError)); + PyErr_Clear(); + + assert(PyWeakref_GetObject(invalid_weakref) == NULL); + assert(PyErr_ExceptionMatches(PyExc_TypeError)); + PyErr_Clear(); + + Py_RETURN_NONE; +} + + static PyMethodDef TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, {"test_config", test_config, METH_NOARGS}, @@ -3516,6 +3583,7 @@ static PyMethodDef TestMethods[] = { {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS}, + {"check_weakref_capi", check_weakref_capi, METH_O}, {NULL, NULL} /* sentinel */ }; diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index 9995a7756570ca..6126f6d38d5147 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -894,13 +894,35 @@ PyWeakref_NewProxy(PyObject *ob, PyObject *callback) } +int +PyWeakref_GetRef(PyObject *ref, PyObject **pobj) +{ + if (ref == NULL) { + *pobj = NULL; + PyErr_BadInternalCall(); + return -1; + } + if (!PyWeakref_Check(ref)) { + *pobj = NULL; + PyErr_SetString(PyExc_TypeError, "expected a weakref"); + return -1; + } + *pobj = _PyWeakref_GET_REF(ref); + return 0; +} + + PyObject * PyWeakref_GetObject(PyObject *ref) { - if (ref == NULL || !PyWeakref_Check(ref)) { + if (ref == NULL) { PyErr_BadInternalCall(); return NULL; } + if (!PyWeakref_Check(ref)) { + PyErr_SetString(PyExc_TypeError, "expected a weakref"); + return NULL; + } return PyWeakref_GET_OBJECT(ref); } diff --git a/PC/python3dll.c b/PC/python3dll.c index fea19b77185dd5..65bdf326ffbc7f 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -735,6 +735,7 @@ EXPORT_FUNC(PyUnicodeTranslateError_SetStart) EXPORT_FUNC(PyVectorcall_Call) EXPORT_FUNC(PyVectorcall_NARGS) EXPORT_FUNC(PyWeakref_GetObject) +EXPORT_FUNC(PyWeakref_GetRef) EXPORT_FUNC(PyWeakref_NewProxy) EXPORT_FUNC(PyWeakref_NewRef) EXPORT_FUNC(PyWrapper_New) From 59bb298fa97d77071b3449d58f0e38b9fc8bd914 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Jun 2023 22:27:43 +0200 Subject: [PATCH 2/5] Address Erlend's review * Change Sphinx formatting * Don't change PyWeakref_GetObject() exception --- Doc/c-api/weakref.rst | 4 ++-- Modules/_testcapimodule.c | 2 +- Objects/weakrefobject.c | 6 +----- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Doc/c-api/weakref.rst b/Doc/c-api/weakref.rst index cb7b26e17bae35..0ef521874c8fe6 100644 --- a/Doc/c-api/weakref.rst +++ b/Doc/c-api/weakref.rst @@ -53,10 +53,10 @@ as much as it can. .. c:function:: int PyWeakref_GetRef(PyObject *ref, PyObject **pobj) - Get the referenced object from a weak reference, *ref*, into ``*pobj``. + Get the referenced object from a weak reference, *ref*, into *\*pobj*. Return 0 on success. Raise an exception and return -1 on error. - If the referent is no longer live, set ``*pobj`` to ``NULL`` and return 0. + If the referent is no longer live, set *\*pobj* to ``NULL`` and return 0. .. versionadded:: 3.13 diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index c6d1cd1ae7824a..0a6a6a980c95b1 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3432,7 +3432,7 @@ check_weakref_capi(PyObject *self, PyObject *factory) PyErr_Clear(); assert(PyWeakref_GetObject(invalid_weakref) == NULL); - assert(PyErr_ExceptionMatches(PyExc_TypeError)); + assert(PyErr_ExceptionMatches(PyExc_SystemError)); PyErr_Clear(); Py_RETURN_NONE; diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index 6126f6d38d5147..49342d0658de6b 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -915,14 +915,10 @@ PyWeakref_GetRef(PyObject *ref, PyObject **pobj) PyObject * PyWeakref_GetObject(PyObject *ref) { - if (ref == NULL) { + if (ref == NULL || !PyWeakref_Check(ref)) { PyErr_BadInternalCall(); return NULL; } - if (!PyWeakref_Check(ref)) { - PyErr_SetString(PyExc_TypeError, "expected a weakref"); - return NULL; - } return PyWeakref_GET_OBJECT(ref); } From ee5d23a02ae20c1e3b9fe1ce20bac081e5a0a4ad Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Jun 2023 22:45:54 +0200 Subject: [PATCH 3/5] Fix comment --- Modules/_testcapimodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 0a6a6a980c95b1..82e61b219fe070 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3383,7 +3383,7 @@ check_weakref_capi(PyObject *self, PyObject *factory) Py_ssize_t refcnt = Py_REFCNT(obj); assert(refcnt == 1); - // test PyWeakref_GetRef() + // test PyWeakref_NewRef() PyObject *weakref = PyWeakref_NewRef(obj, NULL); if (weakref == NULL) { Py_DECREF(obj); From 4589377993d50d86cc72a47be62d223b958b9da4 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Jun 2023 23:09:42 +0200 Subject: [PATCH 4/5] Enhance tests --- Lib/test/test_weakref.py | 4 ---- Modules/_testcapimodule.c | 41 +++++++++++++++++++++++++-------------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index f9fea0162634a2..1bc1d05f7daba9 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -967,10 +967,6 @@ def __del__(self): pass del x support.gc_collect() - @support.cpython_only - def test_capi(self): - import _testcapi - _testcapi.check_weakref_capi(C) class SubclassableWeakrefTestCase(TestBase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 82e61b219fe070..8b63f9c90a385d 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3373,17 +3373,23 @@ check_pyimport_addmodule(PyObject *self, PyObject *args) static PyObject * -check_weakref_capi(PyObject *self, PyObject *factory) +test_weakref_capi(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) { - // obj = factory() - PyObject *obj = PyObject_CallNoArgs(factory); + // Create a new heap type, create an instance of this type, and delete the + // type. This object supports weak references. + PyObject *new_type = PyObject_CallFunction((PyObject*)&PyType_Type, + "s(){}", "TypeName"); + if (new_type == NULL) { + return NULL; + } + PyObject *obj = PyObject_CallNoArgs(new_type); + Py_DECREF(new_type); if (obj == NULL) { return NULL; } Py_ssize_t refcnt = Py_REFCNT(obj); - assert(refcnt == 1); - // test PyWeakref_NewRef() + // test PyWeakref_NewRef(), reference is alive PyObject *weakref = PyWeakref_NewRef(obj, NULL); if (weakref == NULL) { Py_DECREF(obj); @@ -3394,28 +3400,30 @@ check_weakref_capi(PyObject *self, PyObject *factory) assert(PyWeakref_CheckRefExact(weakref)); assert(Py_REFCNT(obj) == refcnt); - // test PyWeakref_GetRef() + // test PyWeakref_GetRef(), reference is alive PyObject *ref1; assert(PyWeakref_GetRef(weakref, &ref1) == 0); assert(ref1 == obj); assert(Py_REFCNT(obj) == (refcnt + 1)); Py_DECREF(ref1); - // test PyWeakref_GetObject() + // test PyWeakref_GetObject(), reference is alive PyObject *ref2 = PyWeakref_GetObject(weakref); assert(ref2 == obj); - assert(Py_REFCNT(obj) == refcnt); - // test PyWeakref_GET_OBJECT() + // test PyWeakref_GET_OBJECT(), reference is alive PyObject *ref3 = PyWeakref_GET_OBJECT(weakref); assert(ref3 == obj); - assert(Py_REFCNT(obj) == refcnt); - // delete the object - assert(refcnt == 1); + // delete the referenced object + assert(Py_REFCNT(obj) == 1); Py_DECREF(obj); + + // test PyWeakref_GET_OBJECT(), reference is dead assert(PyWeakref_GET_OBJECT(weakref) == Py_None); - PyObject *ref4; + + // test PyWeakref_GetRef(), reference is dead + PyObject *ref4 = Py_True; // marker to check that value was set assert(PyWeakref_GetRef(weakref, &ref4) == 0); assert(ref4 == NULL); @@ -3425,12 +3433,15 @@ check_weakref_capi(PyObject *self, PyObject *factory) assert(!PyWeakref_CheckRefExact(invalid_weakref)); assert(!PyWeakref_CheckRefExact(invalid_weakref)); + // test PyWeakref_GetRef(), invalid type assert(!PyErr_Occurred()); - PyObject *ref5 = factory; // marker to check that value was set + PyObject *ref5 = Py_True; // marker to check that value was set assert(PyWeakref_GetRef(invalid_weakref, &ref5) == -1); assert(PyErr_ExceptionMatches(PyExc_TypeError)); PyErr_Clear(); + assert(ref5 == NULL); + // test PyWeakref_GetObject(), invalid type assert(PyWeakref_GetObject(invalid_weakref) == NULL); assert(PyErr_ExceptionMatches(PyExc_SystemError)); PyErr_Clear(); @@ -3583,7 +3594,7 @@ static PyMethodDef TestMethods[] = { {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS}, - {"check_weakref_capi", check_weakref_capi, METH_O}, + {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; From 2de389766342ee18907130debf63a0336b0bbb45 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 20 Jun 2023 23:30:45 +0200 Subject: [PATCH 5/5] Fix Sphinx warnings Enhance also the doc: specific strong/borrowed reference --- Doc/c-api/weakref.rst | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/Doc/c-api/weakref.rst b/Doc/c-api/weakref.rst index 0ef521874c8fe6..44f4dce9ea0238 100644 --- a/Doc/c-api/weakref.rst +++ b/Doc/c-api/weakref.rst @@ -11,20 +11,20 @@ simple reference object, and the second acts as a proxy for the original object as much as it can. -.. c:function:: int PyWeakref_Check(ob) +.. c:function:: int PyWeakref_Check(PyObject *ob) - Return true if *ob* is either a reference or proxy object. This function + Return non-zero if *ob* is either a reference or proxy object. This function always succeeds. -.. c:function:: int PyWeakref_CheckRef(ob) +.. c:function:: int PyWeakref_CheckRef(PyObject *ob) - Return true if *ob* is a reference object. This function always succeeds. + Return non-zero if *ob* is a reference object. This function always succeeds. -.. c:function:: int PyWeakref_CheckProxy(ob) +.. c:function:: int PyWeakref_CheckProxy(PyObject *ob) - Return true if *ob* is a proxy object. This function always succeeds. + Return non-zero if *ob* is a proxy object. This function always succeeds. .. c:function:: PyObject* PyWeakref_NewRef(PyObject *ob, PyObject *callback) @@ -53,7 +53,8 @@ as much as it can. .. c:function:: int PyWeakref_GetRef(PyObject *ref, PyObject **pobj) - Get the referenced object from a weak reference, *ref*, into *\*pobj*. + Get a :term:`strong reference` to the referenced object from a weak + reference, *ref*, into *\*pobj*. Return 0 on success. Raise an exception and return -1 on error. If the referent is no longer live, set *\*pobj* to ``NULL`` and return 0. @@ -63,8 +64,8 @@ as much as it can. .. c:function:: PyObject* PyWeakref_GetObject(PyObject *ref) - Return the referenced object from a weak reference, *ref*. If the referent is - no longer live, returns :const:`Py_None`. + Return a :term:`borrowed reference` to the referenced object from a weak + reference, *ref*. If the referent is no longer live, returns ``Py_None``. .. note::