From e012b8baed796615cc92c2785361e7a12a601f96 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Sat, 9 Jul 2022 11:10:41 -0600 Subject: [PATCH 1/7] Store tp_weaklist on the interpreter state for static builtin types. --- Include/cpython/object.h | 2 +- Include/internal/pycore_object.h | 6 ++++++ Include/internal/pycore_typeobject.h | 5 +++++ Objects/typeobject.c | 5 +++++ 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 1ca1a576fb2329..026803320a11b2 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -219,7 +219,7 @@ struct _typeobject { PyObject *tp_mro; /* method resolution order */ PyObject *tp_cache; PyObject *tp_subclasses; - PyObject *tp_weaklist; + PyObject *tp_weaklist; /* not used for static builtin types */ destructor tp_del; /* Type attribute cache version tag. Added in version 2.6 */ diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index c7490799d816af..198486c9451814 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -220,6 +220,12 @@ extern void _Py_PrintReferenceAddresses(FILE *); static inline PyObject ** _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) { + if (PyType_Check(op) && + ((PyTypeObject *)op)->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + static_builtin_state *state = _PyStaticType_GetState( + (PyTypeObject *)op); + return &state->tp_weaklist; + } Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; return (PyObject **)((char *)op + offset); } diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index dc1c02ba412809..abea8727a1cbdc 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -45,6 +45,11 @@ struct type_cache { typedef struct { PyTypeObject *type; + /* We never clean up weakrefs for static builtin types since + they will effectively never get triggered. However, there + are also some diagnostic uses for the list of weakrefs, + so we still keep it. */ + PyObject *tp_weaklist; } static_builtin_state; struct types_state { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9eb2390118d097..bd69c32cc71dbd 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -127,6 +127,8 @@ static_builtin_state_init(PyTypeObject *self) static_builtin_state *state = static_builtin_state_get(interp, self); state->type = self; + /* state->tp_weaklist is left NULL until insert_head() or insert_after() + (in weakrefobject.c) sets it. */ } static void @@ -138,6 +140,7 @@ static_builtin_state_clear(PyTypeObject *self) static_builtin_state *state = static_builtin_state_get(interp, self); state->type = NULL; + /* state->tp_weaklist should already have been cleared out. */ static_builtin_index_clear(self); assert(interp->types.num_builtins_initialized > 0); @@ -502,6 +505,8 @@ static PyMemberDef type_members[] = { {"__basicsize__", T_PYSSIZET, offsetof(PyTypeObject,tp_basicsize),READONLY}, {"__itemsize__", T_PYSSIZET, offsetof(PyTypeObject, tp_itemsize), READONLY}, {"__flags__", T_ULONG, offsetof(PyTypeObject, tp_flags), READONLY}, + /* Note that this value is misleading for static builtin types, + since the memory at this offset will always be NULL. */ {"__weakrefoffset__", T_PYSSIZET, offsetof(PyTypeObject, tp_weaklistoffset), READONLY}, {"__base__", T_OBJECT, offsetof(PyTypeObject, tp_base), READONLY}, From 8116dde0dafed1b486fdea12081519d5bdaffa75 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Tue, 26 Jul 2022 16:08:07 -0600 Subject: [PATCH 2/7] Factor out _PyStaticType_GET_WEAKREFS_LISTPTR(). --- Include/internal/pycore_object.h | 2 +- Include/internal/pycore_typeobject.h | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 198486c9451814..9f061d8b08772c 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -224,7 +224,7 @@ _PyObject_GET_WEAKREFS_LISTPTR(PyObject *op) ((PyTypeObject *)op)->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { static_builtin_state *state = _PyStaticType_GetState( (PyTypeObject *)op); - return &state->tp_weaklist; + return _PyStaticType_GET_WEAKREFS_LISTPTR(state); } Py_ssize_t offset = Py_TYPE(op)->tp_weaklistoffset; return (PyObject **)((char *)op + offset); diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index abea8727a1cbdc..a6c6304d51179b 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -52,6 +52,13 @@ typedef struct { PyObject *tp_weaklist; } static_builtin_state; +static inline PyObject ** +_PyStaticType_GET_WEAKREFS_LISTPTR(static_builtin_state *state) +{ + assert(state != NULL); + return &state->tp_weaklist; +} + struct types_state { struct type_cache type_cache; size_t num_builtins_initialized; From f7d27feb02156d6238e629333f2b1bd64008e146 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Tue, 26 Jul 2022 16:19:15 -0600 Subject: [PATCH 3/7] Add _PyStaticType_ClearWeakRefs(). --- Include/internal/pycore_typeobject.h | 1 + Objects/typeobject.c | 3 ++- Objects/weakrefobject.c | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index a6c6304d51179b..4f9d6b1c4d4ba2 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -70,6 +70,7 @@ extern PyStatus _PyTypes_InitSlotDefs(void); extern int _PyStaticType_InitBuiltin(PyTypeObject *type); extern static_builtin_state * _PyStaticType_GetState(PyTypeObject *); +extern void _PyStaticType_ClearWeakRefs(PyTypeObject *type); extern void _PyStaticType_Dealloc(PyTypeObject *type); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index bd69c32cc71dbd..e4adf1c4e12f8c 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -140,7 +140,7 @@ static_builtin_state_clear(PyTypeObject *self) static_builtin_state *state = static_builtin_state_get(interp, self); state->type = NULL; - /* state->tp_weaklist should already have been cleared out. */ + assert(state->tp_weaklist == NULL); // It was already cleared out. static_builtin_index_clear(self); assert(interp->types.num_builtins_initialized > 0); @@ -4358,6 +4358,7 @@ _PyStaticType_Dealloc(PyTypeObject *type) type->tp_flags &= ~Py_TPFLAGS_READY; if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + _PyStaticType_ClearWeakRefs(type); static_builtin_state_clear(type); /* We leave _Py_TPFLAGS_STATIC_BUILTIN set on tp_flags. */ } diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index 2b4361e63ca413..e15c8f4fabbb9a 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -1018,3 +1018,19 @@ PyObject_ClearWeakRefs(PyObject *object) PyErr_Restore(err_type, err_value, err_tb); } } + +/* This function is called by _PyStaticType_Dealloc() to clear weak references. + * + * This is called at the end of runtime finalization, so we can just + * wipe out the type's weaklist. We don't bother with callbacks + * or anything else. + */ +void +_PyStaticType_ClearWeakRefs(PyTypeObject *type) +{ + static_builtin_state *state = _PyStaticType_GetState(type); + PyObject **list = _PyStaticType_GET_WEAKREFS_LISTPTR(state); + while (*list != NULL) { + clear_weakref((PyWeakReference *)*list); + } +} From 8a5b5bcb81ec4c06fa3cccd135a0f60c5041a250 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Wed, 27 Jul 2022 14:46:11 -0600 Subject: [PATCH 4/7] Add a comment about how _PyStaticType_ClearWeakRefs() loops. --- Objects/weakrefobject.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index e15c8f4fabbb9a..17e1e9cdc814fa 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -1031,6 +1031,9 @@ _PyStaticType_ClearWeakRefs(PyTypeObject *type) static_builtin_state *state = _PyStaticType_GetState(type); PyObject **list = _PyStaticType_GET_WEAKREFS_LISTPTR(state); while (*list != NULL) { + /* Note that clear_weakref() pops the first ref off the type's + weaklist before clearing its wr_object and wr_callback. + That is how we're able to loop over the list. */ clear_weakref((PyWeakReference *)*list); } } From 69f9be87d6cac95c4c34690332fb7eadf46cd664 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Thu, 28 Jul 2022 18:10:48 -0600 Subject: [PATCH 5/7] Document the change. --- Doc/c-api/typeobj.rst | 7 +++++++ Doc/whatsnew/3.12.rst | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index a331e9c1885092..7514801f2d4d59 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -1942,6 +1942,13 @@ and :c:type:`PyType_Type` effectively act as defaults.) Weak reference list head, for weak references to this type object. Not inherited. Internal use only. + .. versionchanged:: 3.12 + + Internals detail: For the static builtin types this is always ``NULL``, + even if weakrefs are added. Instead, the weakrefs for each are stored + on ``PyInterpreterState``. Use the public C-API or the internal + ``_PyObject_GET_WEAKREFS_LISTPTR()`` macro to avoid the distinction. + **Inheritance:** This field is not inherited. diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 03020559362b1d..40f721ae2765cb 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -413,6 +413,13 @@ Porting to Python 3.12 ``Py_UNICODE*`` based format (e.g. ``u``, ``Z``) anymore. Please migrate to other formats for Unicode like ``s``, ``z``, ``es``, and ``U``. +* ``tp_weaklist`` for ``PyType_Type`` is always ``NULL`` for all static + builtin types. This is an internal-only field on ``PyTypeObject`` + but we're pointing out the change in case someone happens to be + accessing the field directly anyway. To avoid breakage, consider + using the existing public C-API instead, or, if necessary, the + (internal-only) ``_PyObject_GET_WEAKREFS_LISTPTR()`` macro. + Deprecated ---------- From d63e1662442a030cce336c8aa3675d3978e90ad9 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Thu, 28 Jul 2022 18:22:48 -0600 Subject: [PATCH 6/7] Update Doc/whatsnew/3.12.rst --- Doc/whatsnew/3.12.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 40f721ae2765cb..67cf32b99bb56e 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -413,8 +413,8 @@ Porting to Python 3.12 ``Py_UNICODE*`` based format (e.g. ``u``, ``Z``) anymore. Please migrate to other formats for Unicode like ``s``, ``z``, ``es``, and ``U``. -* ``tp_weaklist`` for ``PyType_Type`` is always ``NULL`` for all static - builtin types. This is an internal-only field on ``PyTypeObject`` +* ``tp_weaklist`` for all static builtin types is always ``NULL``. + This is an internal-only field on ``PyTypeObject`` but we're pointing out the change in case someone happens to be accessing the field directly anyway. To avoid breakage, consider using the existing public C-API instead, or, if necessary, the From 7af915fd39ee0fb6343404132081f761ade1d504 Mon Sep 17 00:00:00 2001 From: Eric Snow <ericsnowcurrently@gmail.com> Date: Thu, 28 Jul 2022 18:31:02 -0600 Subject: [PATCH 7/7] Fix a typo. --- Doc/whatsnew/3.12.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 67cf32b99bb56e..0c53bc0c1111d7 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -414,7 +414,7 @@ Porting to Python 3.12 to other formats for Unicode like ``s``, ``z``, ``es``, and ``U``. * ``tp_weaklist`` for all static builtin types is always ``NULL``. - This is an internal-only field on ``PyTypeObject`` + This is an internal-only field on ``PyTypeObject`` but we're pointing out the change in case someone happens to be accessing the field directly anyway. To avoid breakage, consider using the existing public C-API instead, or, if necessary, the