From 7be667dfafa2465df6342d72dca9c1f82dd830d0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 30 May 2023 18:13:35 -0600 Subject: gh-105020: Share tp_bases and tp_mro Between Interpreters For All Static Builtin Types (gh-105115) In gh-103912 we added tp_bases and tp_mro to each PyInterpreterState.types.builtins entry. However, doing so ignored the fact that both PyTypeObject fields are public API, and not documented as internal (as opposed to tp_subclasses). We address that here by reverting back to shared objects, making them immortal in the process. --- Include/internal/pycore_object.h | 15 +++++ Include/internal/pycore_typeobject.h | 6 +- Lib/test/test_capi/test_misc.py | 33 ++++++++++ .../2023-05-30-17-45-32.gh-issue-105115.iRho1K.rst | 3 + Modules/_testcapimodule.c | 23 +++++++ Objects/typeobject.c | 76 +++++++++++++--------- 6 files changed, 122 insertions(+), 34 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-05-30-17-45-32.gh-issue-105115.iRho1K.rst diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 500b3ee..0981d11 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -76,6 +76,21 @@ static inline void _Py_SetImmortal(PyObject *op) } #define _Py_SetImmortal(op) _Py_SetImmortal(_PyObject_CAST(op)) +/* _Py_ClearImmortal() should only be used during runtime finalization. */ +static inline void _Py_ClearImmortal(PyObject *op) +{ + if (op) { + assert(op->ob_refcnt == _Py_IMMORTAL_REFCNT); + op->ob_refcnt = 1; + Py_DECREF(op); + } +} +#define _Py_ClearImmortal(op) \ + do { \ + _Py_ClearImmortal(_PyObject_CAST(op)); \ + op = NULL; \ + } while (0) + static inline void _Py_DECREF_SPECIALIZED(PyObject *op, const destructor destruct) { diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index c7d0000..8f3fbbc 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -46,11 +46,9 @@ typedef struct { PyTypeObject *type; int readying; int ready; - // XXX tp_dict, tp_bases, and tp_mro can probably be statically - // allocated, instead of dynamically and stored on the interpreter. + // XXX tp_dict can probably be statically allocated, + // instead of dynamically and stored on the interpreter. PyObject *tp_dict; - PyObject *tp_bases; - PyObject *tp_mro; PyObject *tp_subclasses; /* We never clean up weakrefs for static builtin types since they will effectively never get triggered. However, there diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index dc3441e..037c811 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1593,6 +1593,39 @@ class SubinterpreterTest(unittest.TestCase): self.assertEqual(main_attr_id, subinterp_attr_id) +class BuiltinStaticTypesTests(unittest.TestCase): + + TYPES = [ + object, + type, + int, + str, + dict, + type(None), + bool, + BaseException, + Exception, + Warning, + DeprecationWarning, # Warning subclass + ] + + def test_tp_bases_is_set(self): + # PyTypeObject.tp_bases is documented as public API. + # See https://github.com/python/cpython/issues/105020. + for typeobj in self.TYPES: + with self.subTest(typeobj): + bases = _testcapi.type_get_tp_bases(typeobj) + self.assertIsNot(bases, None) + + def test_tp_mro_is_set(self): + # PyTypeObject.tp_bases is documented as public API. + # See https://github.com/python/cpython/issues/105020. + for typeobj in self.TYPES: + with self.subTest(typeobj): + mro = _testcapi.type_get_tp_mro(typeobj) + self.assertIsNot(mro, None) + + class TestThreadState(unittest.TestCase): @threading_helper.reap_threads diff --git a/Misc/NEWS.d/next/C API/2023-05-30-17-45-32.gh-issue-105115.iRho1K.rst b/Misc/NEWS.d/next/C API/2023-05-30-17-45-32.gh-issue-105115.iRho1K.rst new file mode 100644 index 0000000..595cc0e --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-05-30-17-45-32.gh-issue-105115.iRho1K.rst @@ -0,0 +1,3 @@ +``PyTypeObject.tp_bases`` (and ``tp_mro``) for builtin static types are now +shared by all interpreters, whereas in 3.12-beta1 they were stored on +``PyInterpreterState``. Also note that now the tuples are immortal objects. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index aff09aa..66c1cba 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2606,6 +2606,27 @@ type_assign_version(PyObject *self, PyObject *type) } +static PyObject * +type_get_tp_bases(PyObject *self, PyObject *type) +{ + PyObject *bases = ((PyTypeObject *)type)->tp_bases; + if (bases == NULL) { + Py_RETURN_NONE; + } + return Py_NewRef(bases); +} + +static PyObject * +type_get_tp_mro(PyObject *self, PyObject *type) +{ + PyObject *mro = ((PyTypeObject *)type)->tp_mro; + if (mro == NULL) { + Py_RETURN_NONE; + } + return Py_NewRef(mro); +} + + // Test PyThreadState C API static PyObject * test_tstate_capi(PyObject *self, PyObject *Py_UNUSED(args)) @@ -3361,6 +3382,8 @@ static PyMethodDef TestMethods[] = { {"test_py_is_funcs", test_py_is_funcs, METH_NOARGS}, {"type_get_version", type_get_version, METH_O, PyDoc_STR("type->tp_version_tag")}, {"type_assign_version", type_assign_version, METH_O, PyDoc_STR("PyUnstable_Type_AssignVersionTag")}, + {"type_get_tp_bases", type_get_tp_bases, METH_O}, + {"type_get_tp_mro", type_get_tp_mro, METH_O}, {"test_tstate_capi", test_tstate_capi, METH_NOARGS, NULL}, {"frame_getlocals", frame_getlocals, METH_O, NULL}, {"frame_getglobals", frame_getglobals, METH_O, NULL}, diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 0a2e1b1..6540d4e 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -268,12 +268,6 @@ clear_tp_dict(PyTypeObject *self) static inline PyObject * lookup_tp_bases(PyTypeObject *self) { - if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - static_builtin_state *state = _PyStaticType_GetState(interp, self); - assert(state != NULL); - return state->tp_bases; - } return self->tp_bases; } @@ -287,12 +281,22 @@ _PyType_GetBases(PyTypeObject *self) static inline void set_tp_bases(PyTypeObject *self, PyObject *bases) { + assert(PyTuple_CheckExact(bases)); if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - static_builtin_state *state = _PyStaticType_GetState(interp, self); - assert(state != NULL); - state->tp_bases = bases; - return; + // XXX tp_bases can probably be statically allocated for each + // static builtin type. + assert(_Py_IsMainInterpreter(_PyInterpreterState_GET())); + assert(self->tp_bases == NULL); + if (PyTuple_GET_SIZE(bases) == 0) { + assert(self->tp_base == NULL); + } + else { + assert(PyTuple_GET_SIZE(bases) == 1); + assert(PyTuple_GET_ITEM(bases, 0) == (PyObject *)self->tp_base); + assert(self->tp_base->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN); + assert(_Py_IsImmortal(self->tp_base)); + } + _Py_SetImmortal(bases); } self->tp_bases = bases; } @@ -301,10 +305,14 @@ static inline void clear_tp_bases(PyTypeObject *self) { if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - static_builtin_state *state = _PyStaticType_GetState(interp, self); - assert(state != NULL); - Py_CLEAR(state->tp_bases); + if (_Py_IsMainInterpreter(_PyInterpreterState_GET())) { + if (self->tp_bases != NULL + && PyTuple_GET_SIZE(self->tp_bases) > 0) + { + assert(_Py_IsImmortal(self->tp_bases)); + _Py_ClearImmortal(self->tp_bases); + } + } return; } Py_CLEAR(self->tp_bases); @@ -314,12 +322,6 @@ clear_tp_bases(PyTypeObject *self) static inline PyObject * lookup_tp_mro(PyTypeObject *self) { - if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - static_builtin_state *state = _PyStaticType_GetState(interp, self); - assert(state != NULL); - return state->tp_mro; - } return self->tp_mro; } @@ -333,12 +335,14 @@ _PyType_GetMRO(PyTypeObject *self) static inline void set_tp_mro(PyTypeObject *self, PyObject *mro) { + assert(PyTuple_CheckExact(mro)); if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - static_builtin_state *state = _PyStaticType_GetState(interp, self); - assert(state != NULL); - state->tp_mro = mro; - return; + // XXX tp_mro can probably be statically allocated for each + // static builtin type. + assert(_Py_IsMainInterpreter(_PyInterpreterState_GET())); + assert(self->tp_mro == NULL); + /* Other checks are done via set_tp_bases. */ + _Py_SetImmortal(mro); } self->tp_mro = mro; } @@ -347,10 +351,14 @@ static inline void clear_tp_mro(PyTypeObject *self) { if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { - PyInterpreterState *interp = _PyInterpreterState_GET(); - static_builtin_state *state = _PyStaticType_GetState(interp, self); - assert(state != NULL); - Py_CLEAR(state->tp_mro); + if (_Py_IsMainInterpreter(_PyInterpreterState_GET())) { + if (self->tp_mro != NULL + && PyTuple_GET_SIZE(self->tp_mro) > 0) + { + assert(_Py_IsImmortal(self->tp_mro)); + _Py_ClearImmortal(self->tp_mro); + } + } return; } Py_CLEAR(self->tp_mro); @@ -7153,6 +7161,14 @@ type_ready_preheader(PyTypeObject *type) static int type_ready_mro(PyTypeObject *type) { + if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { + if (!_Py_IsMainInterpreter(_PyInterpreterState_GET())) { + assert(lookup_tp_mro(type) != NULL); + return 0; + } + assert(lookup_tp_mro(type) == NULL); + } + /* Calculate method resolution order */ if (mro_internal(type, NULL) < 0) { return -1; -- cgit v0.12