Skip to content

Commit

Permalink
[3.13] gh-117482: Fix Builtin Types Slot Wrappers (gh-121630)
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, AKA gh-121602)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
  • Loading branch information
miss-islington and ericsnowcurrently authored Jul 11, 2024
1 parent 38c4028 commit c6dbfbb
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 c6dbfbb

Please sign in to comment.