Skip to content

Commit

Permalink
pythongh-117482: Fix Builtin Types Slot Wrappers (pythongh-121602)
Browse files Browse the repository at this point in the history
When builtin static types are initialized for a subinterpreter, various "tp" slots have already been inherited (for the main interpreter).  This was interfering with the logic in add_operators() (in Objects/typeobject.c), causing a wrapper to get created when it shouldn't.  This change fixes that by preserving the original data from the static type struct and checking that.
(cherry picked from commit 5250a03)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
  • Loading branch information
ericsnowcurrently authored and miss-islington committed Jul 11, 2024
1 parent 38c4028 commit 4b5e638
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 10 deletions.
1 change: 1 addition & 0 deletions Include/internal/pycore_typeobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions Lib/test/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import pickle
import locale
import sys
import textwrap
import types
import unittest.mock
import weakref
Expand Down Expand Up @@ -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.SkipTest('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()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Unexpected slot wrappers are no longer created for builtin static types in
subinterpreters.
40 changes: 30 additions & 10 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -5668,7 +5678,6 @@ 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. */
}

void
Expand Down Expand Up @@ -7671,7 +7680,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)
Expand Down Expand Up @@ -7836,10 +7845,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) {
Expand Down Expand Up @@ -8158,7 +8167,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();

Expand Down Expand Up @@ -8197,7 +8206,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) {
Expand Down Expand Up @@ -8254,7 +8263,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));
Expand Down Expand Up @@ -8290,14 +8299,20 @@ 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);
res = type_ready(self, def, initial);
END_TYPE_LOCK();
if (res < 0) {
_PyStaticType_ClearWeakRefs(interp, self);
managed_static_type_state_clear(interp, self, isbuiltin, initial);
}

return res;
}

Expand Down Expand Up @@ -10885,17 +10900,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);
Expand Down

0 comments on commit 4b5e638

Please sign in to comment.