From 7304779bc54f0d05a85cd4b9edfa8fe77eb6385b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 11:57:01 -0600 Subject: [PATCH 01/10] Add a test. --- Lib/test/test_types.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index fbca198aab5180..409a9908846102 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -10,6 +10,7 @@ import pickle import locale import sys +import textwrap import types import unittest.mock import weakref @@ -2345,5 +2346,40 @@ def ex(a, /, b, *, c): ) +class SubinterpreterTests(unittest.TestCase): + + @classmethod + def setUpClass(cls): + global interpreters + try: + from test.support import interpreters + except ModuleNotFoundError: + raise unittest.Skip('subinterpreters required') + import test.support.interpreters.channels + + @cpython_only + def test_slot_wrappers(self): + rch, sch = interpreters.channels.create() + + # For now it's sufficient to check int.__str__. + # See https://github.com/python/cpython/issues/117482 + # and https://github.com/python/cpython/pull/117660. + script = textwrap.dedent(''' + text = repr(int.__str__) + sch.send_nowait(text) + ''') + + exec(script) + expected = rch.recv() + + interp = interpreters.create() + interp.exec('from test.support import interpreters') + interp.prepare_main(sch=sch) + interp.exec(script) + results = rch.recv() + + self.assertEqual(results, expected) + + if __name__ == '__main__': unittest.main() From 7d14ac36da15231ef3b7725fe2f4ba3401195b83 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 13:50:37 -0600 Subject: [PATCH 02/10] Copy the original type struct into global state. --- Include/internal/pycore_typeobject.h | 1 + Objects/typeobject.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 32bd19d968b917..df6bfef715dd34 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -33,6 +33,7 @@ struct _types_runtime_state { struct { struct { PyTypeObject *type; + PyTypeObject def; int64_t interp_count; } types[_Py_MAX_MANAGED_STATIC_TYPES]; } managed_static; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 587632cecfba9d..ed056349dbc516 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -314,6 +314,16 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, } } +static PyTypeObject * +managed_static_type_get_def(PyTypeObject *self, int isbuiltin) +{ + size_t index = managed_static_type_index_get(self); + size_t full_index = isbuiltin + ? index + : index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES; + return &_PyRuntime.types.managed_static.types[full_index].def; +} + // Also see _PyStaticType_InitBuiltin() and _PyStaticType_FiniBuiltin(). /* end static builtin helpers */ @@ -8469,6 +8479,11 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, managed_static_type_state_init(interp, self, isbuiltin, initial); + PyTypeObject *def = managed_static_type_get_def(self, isbuiltin); + if (initial) { + memcpy(def, self, sizeof(PyTypeObject)); + } + int res; BEGIN_TYPE_LOCK(); res = type_ready(self, initial); From 5d5000d2fdf3291e49f8907076e2b8f00bcddc92 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 13:51:21 -0600 Subject: [PATCH 03/10] Check the original type struct in add_operators(). --- Objects/typeobject.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ed056349dbc516..c4716b28d89071 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -7860,7 +7860,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base) return 0; } -static int add_operators(PyTypeObject *); +static int add_operators(PyTypeObject *, PyTypeObject *); static int add_tp_new_wrapper(PyTypeObject *type); #define COLLECTION_FLAGS (Py_TPFLAGS_SEQUENCE | Py_TPFLAGS_MAPPING) @@ -8025,10 +8025,10 @@ type_dict_set_doc(PyTypeObject *type) static int -type_ready_fill_dict(PyTypeObject *type) +type_ready_fill_dict(PyTypeObject *type, PyTypeObject *def) { /* Add type-specific descriptors to tp_dict */ - if (add_operators(type) < 0) { + if (add_operators(type, def) < 0) { return -1; } if (type_add_methods(type) < 0) { @@ -8347,7 +8347,7 @@ type_ready_post_checks(PyTypeObject *type) static int -type_ready(PyTypeObject *type, int initial) +type_ready(PyTypeObject *type, PyTypeObject *def, int initial) { ASSERT_TYPE_LOCK_HELD(); @@ -8386,7 +8386,7 @@ type_ready(PyTypeObject *type, int initial) if (type_ready_set_new(type, initial) < 0) { goto error; } - if (type_ready_fill_dict(type) < 0) { + if (type_ready_fill_dict(type, def) < 0) { goto error; } if (initial) { @@ -8443,7 +8443,7 @@ PyType_Ready(PyTypeObject *type) int res; BEGIN_TYPE_LOCK(); if (!(type->tp_flags & Py_TPFLAGS_READY)) { - res = type_ready(type, 1); + res = type_ready(type, NULL, 1); } else { res = 0; assert(_PyType_CheckConsistency(type)); @@ -8486,7 +8486,7 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, int res; BEGIN_TYPE_LOCK(); - res = type_ready(self, initial); + res = type_ready(self, def, initial); END_TYPE_LOCK(); if (res < 0) { _PyStaticType_ClearWeakRefs(interp, self); @@ -11079,17 +11079,22 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, infinite recursion here.) */ static int -add_operators(PyTypeObject *type) +add_operators(PyTypeObject *type, PyTypeObject *def) { PyObject *dict = lookup_tp_dict(type); pytype_slotdef *p; PyObject *descr; void **ptr; + assert(def == NULL || (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + if (def == NULL) { + def = type; + } + for (p = slotdefs; p->name; p++) { if (p->wrapper == NULL) continue; - ptr = slotptr(type, p->offset); + ptr = slotptr(def, p->offset); if (!ptr || !*ptr) continue; int r = PyDict_Contains(dict, p->name_strobj); From b869b74b378ab3948b6acdb9293c412b74b8a36a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 14:24:49 -0600 Subject: [PATCH 04/10] Reset the static type during finalization. --- Objects/typeobject.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index c4716b28d89071..3558d4c599dad8 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -269,7 +269,7 @@ managed_static_type_state_init(PyInterpreterState *interp, PyTypeObject *self, /* Reset the type's per-interpreter state. This basically undoes what managed_static_type_state_init() did. */ -static void +static PyTypeObject * managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, int isbuiltin, int final) { @@ -277,6 +277,7 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, size_t full_index = isbuiltin ? index : index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES; + PyTypeObject *def = &_PyRuntime.types.managed_static.types[full_index].def; managed_static_type_state *state = isbuiltin ? &(interp->types.builtins.initialized[index]) @@ -312,6 +313,8 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, } PyMutex_Unlock(&interp->types.mutex); } + + return def; } static PyTypeObject * @@ -5849,8 +5852,13 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type, } _PyStaticType_ClearWeakRefs(interp, type); - managed_static_type_state_clear(interp, type, isbuiltin, final); - /* We leave _Py_TPFLAGS_STATIC_BUILTIN set on tp_flags. */ + PyTypeObject *def = + managed_static_type_state_clear(interp, type, isbuiltin, final); + /* For now we exclude extension module types. */ + if (final && isbuiltin) { + /* Restore the static type to it's (mostly) original values. */ + memcpy(type, def, sizeof(PyTypeObject)); + } } void From 29421a5d6120e1d0e1d251de3d5eb1a616c65a3b Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 15:44:01 -0600 Subject: [PATCH 05/10] Add a NEWS entry. --- .../2024-07-10-15-43-54.gh-issue-117482.5WYaXR.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-07-10-15-43-54.gh-issue-117482.5WYaXR.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-07-10-15-43-54.gh-issue-117482.5WYaXR.rst b/Misc/NEWS.d/next/Core and Builtins/2024-07-10-15-43-54.gh-issue-117482.5WYaXR.rst new file mode 100644 index 00000000000000..ec1e7327b77f19 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-07-10-15-43-54.gh-issue-117482.5WYaXR.rst @@ -0,0 +1,2 @@ +Unexpected slot wrappers are no longer created for builtin static types in +subinterpreters. From 6268b717d056455a5acff6a5c72055cfa25330d9 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 16:09:59 -0600 Subject: [PATCH 06/10] Do not preserve the index at finalization. --- Objects/typeobject.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 3558d4c599dad8..cdbb0e5ad85ea8 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8490,6 +8490,9 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, PyTypeObject *def = managed_static_type_get_def(self, isbuiltin); if (initial) { memcpy(def, self, sizeof(PyTypeObject)); + /* For now we do not worry about preserving the index + at finalization. */ + managed_static_type_index_clear(def); } int res; From d303c68c253e0abe31b326cc92d511c7dffcde7a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 16:50:40 -0600 Subject: [PATCH 07/10] Preserve type->ob_type. --- Objects/typeobject.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index cdbb0e5ad85ea8..63038f7742d40f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8503,6 +8503,11 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, _PyStaticType_ClearWeakRefs(interp, self); managed_static_type_state_clear(interp, self, isbuiltin, initial); } + + if (initial) { + Py_SET_TYPE(def, Py_TYPE(self)); + } + return res; } From bad2d591b73e110ea1e2f9f8e0f5e0829d8f1eda Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 17:19:45 -0600 Subject: [PATCH 08/10] Preserve tp_dealloc. --- Objects/typeobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 63038f7742d40f..dee6e9927e4a21 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -5857,7 +5857,9 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type, /* For now we exclude extension module types. */ if (final && isbuiltin) { /* Restore the static type to it's (mostly) original values. */ + destructor dealloc = type->tp_dealloc; memcpy(type, def, sizeof(PyTypeObject)); + type->tp_dealloc = dealloc; } } From 1a4be497b103a221cabf0c10187db8fdaa6f8f0a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 10 Jul 2024 17:31:00 -0600 Subject: [PATCH 09/10] Fix WASI. --- Lib/test/test_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_types.py b/Lib/test/test_types.py index 409a9908846102..38a98828085e2f 100644 --- a/Lib/test/test_types.py +++ b/Lib/test/test_types.py @@ -2354,7 +2354,7 @@ def setUpClass(cls): try: from test.support import interpreters except ModuleNotFoundError: - raise unittest.Skip('subinterpreters required') + raise unittest.SkipTest('subinterpreters required') import test.support.interpreters.channels @cpython_only From ebda6371293dbe18649948118cc2e5fa5c926e9a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 11 Jul 2024 13:50:50 -0600 Subject: [PATCH 10/10] Drop the finalization code. --- Objects/typeobject.c | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index dee6e9927e4a21..7d01b680605a38 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -269,7 +269,7 @@ managed_static_type_state_init(PyInterpreterState *interp, PyTypeObject *self, /* Reset the type's per-interpreter state. This basically undoes what managed_static_type_state_init() did. */ -static PyTypeObject * +static void managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, int isbuiltin, int final) { @@ -277,7 +277,6 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, size_t full_index = isbuiltin ? index : index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES; - PyTypeObject *def = &_PyRuntime.types.managed_static.types[full_index].def; managed_static_type_state *state = isbuiltin ? &(interp->types.builtins.initialized[index]) @@ -313,8 +312,6 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, } PyMutex_Unlock(&interp->types.mutex); } - - return def; } static PyTypeObject * @@ -5852,15 +5849,7 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type, } _PyStaticType_ClearWeakRefs(interp, type); - PyTypeObject *def = - managed_static_type_state_clear(interp, type, isbuiltin, final); - /* For now we exclude extension module types. */ - if (final && isbuiltin) { - /* Restore the static type to it's (mostly) original values. */ - destructor dealloc = type->tp_dealloc; - memcpy(type, def, sizeof(PyTypeObject)); - type->tp_dealloc = dealloc; - } + managed_static_type_state_clear(interp, type, isbuiltin, final); } void @@ -8492,9 +8481,6 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, PyTypeObject *def = managed_static_type_get_def(self, isbuiltin); if (initial) { memcpy(def, self, sizeof(PyTypeObject)); - /* For now we do not worry about preserving the index - at finalization. */ - managed_static_type_index_clear(def); } int res; @@ -8506,10 +8492,6 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, managed_static_type_state_clear(interp, self, isbuiltin, initial); } - if (initial) { - Py_SET_TYPE(def, Py_TYPE(self)); - } - return res; }