From 92b414351f005017bff3ac5ced9209b41e5ddcec Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 14 Aug 2026 20:51:44 -0700 Subject: [PATCH] gh-151377: Fix races updating type slots and subclasses (GH-155370) (cherry picked from commit 07624ef11b924b39da97e978536f34f740e39575) Co-authored-by: Neil Schemenauer --- ...-06-12-17-15-53.gh-issue-151377.bQSQUk.rst | 2 + Objects/typeobject.c | 136 +++++++++++++----- 2 files changed, 100 insertions(+), 38 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-06-12-17-15-53.gh-issue-151377.bQSQUk.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-06-12-17-15-53.gh-issue-151377.bQSQUk.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-06-12-17-15-53.gh-issue-151377.bQSQUk.rst new file mode 100644 index 000000000000000..1e4fae5834ee108 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-06-12-17-15-53.gh-issue-151377.bQSQUk.rst @@ -0,0 +1,2 @@ +Fix races in free-threaded builds when updating type slots for newly created +classes and when removing entries from a base type's subclasses dictionary. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 8108e8e92731dad..49d1dda0d4c2cf9 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -772,17 +772,14 @@ _PyType_HasSubclasses(PyTypeObject *self) return 1; } -PyObject* -_PyType_GetSubclasses(PyTypeObject *self) +static int +get_subclasses_unlocked(PyTypeObject *self, PyObject *list) { - PyObject *list = PyList_New(0); - if (list == NULL) { - return NULL; - } + ASSERT_TYPE_LOCK_HELD(); PyObject *subclasses = lookup_tp_subclasses(self); // borrowed ref if (subclasses == NULL) { - return list; + return 0; } assert(PyDict_CheckExact(subclasses)); // The loop cannot modify tp_subclasses, there is no need @@ -796,12 +793,33 @@ _PyType_GetSubclasses(PyTypeObject *self) continue; } - if (PyList_Append(list, _PyObject_CAST(subclass)) < 0) { - Py_DECREF(list); - Py_DECREF(subclass); - return NULL; - } + int res = PyList_Append(list, _PyObject_CAST(subclass)); Py_DECREF(subclass); + if (res < 0) { + return -1; + } + } + return 0; +} + +PyObject* +_PyType_GetSubclasses(PyTypeObject *self) +{ + PyObject *list = PyList_New(0); + if (list == NULL) { + return NULL; + } + + // The type lock protects tp_subclasses from being mutated while we + // iterate over it (e.g. by add_subclass() or by remove_subclass() when a + // subclass is deallocated). + int res; + BEGIN_TYPE_LOCK(); + res = get_subclasses_unlocked(self, list); + END_TYPE_LOCK(); + + if (res < 0) { + Py_CLEAR(list); } return list; } @@ -3944,6 +3962,8 @@ static PyObject *object_new(PyTypeObject *, PyObject *, PyObject *); static int object_init(PyObject *, PyObject *, PyObject *); static int update_slot(PyTypeObject *, PyObject *, slot_update_t *update); static void fixup_slot_dispatchers(PyTypeObject *); +static int type_ready(PyTypeObject *, int, int); +static int type_ready_publish(PyTypeObject *, int); static int type_new_set_names(PyTypeObject *); static int type_new_init_subclass(PyTypeObject *, PyObject *); static bool has_slotdef(PyObject *); @@ -4950,13 +4970,10 @@ type_new_impl(type_new_ctx *ctx) } /* Initialize the rest */ - if (PyType_Ready(type) < 0) { + if (type_ready_publish(type, 1) < 0) { goto error; } - // Put the proper slots in place - fixup_slot_dispatchers(type); - if (!_PyDict_HasOnlyStringKeys(type->tp_dict)) { if (PyErr_WarnFormat( PyExc_RuntimeWarning, @@ -4977,10 +4994,6 @@ type_new_impl(type_new_ctx *ctx) } assert(_PyType_CheckConsistency(type)); -#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4 - // After this point, other threads can potentally use this type. - ((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG; -#endif return (PyObject *)type; @@ -5721,8 +5734,7 @@ type_from_slots_or_spec( * After this call we should generally only touch up what's * accessible to Python code, like __dict__. */ - - if (PyType_Ready(type) < 0) { + if (type_ready_publish(type, 0) < 0) { goto finally; } @@ -5782,10 +5794,6 @@ type_from_slots_or_spec( } assert(_PyType_CheckConsistency(type)); -#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4 - // After this point, other threads can potentally use this type. - ((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG; -#endif finally: if (PyErr_Occurred()) { @@ -6851,7 +6859,9 @@ type_dealloc_common(PyTypeObject *type) PyObject *bases = lookup_tp_bases(type); if (bases != NULL) { PyObject *exc = PyErr_GetRaisedException(); + BEGIN_TYPE_LOCK(); remove_all_subclasses(type, bases); + END_TYPE_LOCK(); PyErr_SetRaisedException(exc); } } @@ -9525,7 +9535,7 @@ type_ready_post_checks(PyTypeObject *type) static int -type_ready(PyTypeObject *type, int initial) +type_ready(PyTypeObject *type, int initial, int add_subclasses) { ASSERT_TYPE_LOCK_HELD(); @@ -9578,8 +9588,10 @@ type_ready(PyTypeObject *type, int initial) if (type_ready_set_hash(type) < 0) { goto error; } - if (type_ready_add_subclasses(type) < 0) { - goto error; + if (add_subclasses) { + if (type_ready_add_subclasses(type) < 0) { + goto error; + } } if (initial) { if (type_ready_managed_dict(type) < 0) { @@ -9590,11 +9602,13 @@ type_ready(PyTypeObject *type, int initial) } } - /* All done -- set the ready flag */ - if (initial) { - type_add_flags(type, Py_TPFLAGS_READY); - } else { - assert(type->tp_flags & Py_TPFLAGS_READY); + if (add_subclasses) { + /* All done -- set the ready flag */ + if (initial) { + type_add_flags(type, Py_TPFLAGS_READY); + } else { + assert(type->tp_flags & Py_TPFLAGS_READY); + } } stop_readying(type); @@ -9607,6 +9621,46 @@ type_ready(PyTypeObject *type, int initial) return -1; } +static int +type_ready_publish(PyTypeObject *type, int fix_slots) +{ + int res; + BEGIN_TYPE_LOCK(); + res = type_ready(type, 1, 0); + if (res == 0) { + assert(!(type->tp_flags & Py_TPFLAGS_READY)); + assert(!is_readying(type)); + + if (fix_slots) { + // Put the proper slots in place and only then publish the type as + // a subclass of its bases. Since the type is not reachable by + // other threads before it is published, the slots can be updated + // without stopping the world. This step is skipped for + // type_from_slots_or_spec(). + fixup_slot_dispatchers(type); + } + + // Set the ready flag before revealing the type since type_add_flags() + // may only be used on types that are not yet revealed. + type_add_flags(type, Py_TPFLAGS_READY); + +#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4 + // Mark the type as revealed while still holding the type lock. + // Threads can only find the type through the subclasses of its bases, + // which is done below with the lock held. So, they cannot see the + // type before the flag is set. + ((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG; +#endif + + res = type_ready_add_subclasses(type); + if (res == 0) { + assert(_PyType_CheckConsistency(type)); + } + } + END_TYPE_LOCK(); + return res; +} + int PyType_Ready(PyTypeObject *type) { @@ -9626,7 +9680,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, 1, 1); } else { res = 0; assert(_PyType_CheckConsistency(type)); @@ -9667,7 +9721,7 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self, int res; BEGIN_TYPE_LOCK(); - res = type_ready(self, initial); + res = type_ready(self, initial, 1); END_TYPE_LOCK(); if (res < 0) { _PyStaticType_ClearWeakRefs(interp, self); @@ -12126,13 +12180,19 @@ update_slot(PyTypeObject *type, PyObject *name, slot_update_t *queued_updates) /* Store the proper functions in the slot dispatches at class (type) definition time, based upon which operations the class overrides in its - dict. */ + dict. The type must not be revealed to other threads yet, so that the + slots can be updated directly rather than with the world stopped. */ static void fixup_slot_dispatchers(PyTypeObject *type) { + ASSERT_TYPE_LOCK_HELD(); + ASSERT_WORLD_STOPPED_OR_NEW_TYPE(type); assert(!PyErr_Occurred()); for (pytype_slotdef *p = slotdefs; p->name; ) { - update_one_slot(type, p, &p, NULL); + int rv = update_one_slot(type, p, &p, NULL); + // always returns 0 if queued_updates == NULL + assert (rv == 0); + (void)rv; } }