From 672c9a16272733ec2c6d050ed78a58b0b91efbb1 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 13:09:58 -0700 Subject: [PATCH 1/8] gh-158195: Do not hold the type lock during stop-the-world In the free-threaded build, type slot, MRO and __bases__ updates are prepared under TYPE_LOCK and published with the world stopped. The lock was "pinned" so that it stayed held while the thread waited for the stop-the-world mutex. That deadlocks against a thread that owns the stop-the-world mutex and needs TYPE_LOCK to re-attach, which happens when Python code (a custom mro(), a str subclass __eq__ of a class dict key, or an automatic GC) runs under the type lock and stops the world. Remove the pinning and publish optimistically instead. A type publish generation counter is incremented by every mutation that a prepared update might depend on. The preparing thread snapshots it, prepares, stops the world and compares. If the counter changed, nothing is published and the update is prepared again. --- Include/internal/pycore_interp_structs.h | 6 + Lib/test/test_free_threading/test_type.py | 232 +++++++++ ...10-20-10-00-00.gh-issue-158195.UnpinTL.rst | 7 + Objects/typeobject.c | 482 +++++++++++------- 4 files changed, 540 insertions(+), 187 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-10-00-00.gh-issue-158195.UnpinTL.rst diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index a9ad61f458944ab..2236b158803bae3 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -588,6 +588,12 @@ struct types_state { where all those lower numbers are used for core static types. */ unsigned int next_version_tag; +#ifdef Py_GIL_DISABLED + /* Incremented on changes that can invalidate a type update that is + prepared under the type lock and published with the world stopped. */ + uint64_t publish_gen; +#endif + /* Every static builtin type is initialized for each interpreter during its own initialization, including for the main interpreter during global runtime initialization. This is done by calling diff --git a/Lib/test/test_free_threading/test_type.py b/Lib/test/test_free_threading/test_type.py index a55c4815a038376..3717eb6b8202874 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -1,3 +1,4 @@ +import gc import threading import unittest @@ -19,6 +20,27 @@ class A: attr = 1 +def world_stopper_threads(done): + """Threads that stop the world until 'done' is set. + + The stopper keeps the stop-the-world mutex contended, so that threads + updating types block on it and release the type lock. The collector + makes the stop-the-world pauses long, so that other threads have time + to update types while the lock is released. + """ + def stopper(): + def g(): + pass + while not done.is_set(): + g.__qualname__ = "g" # stops the world + + def collector(): + while not done.is_set(): + gc.collect() + + return [Thread(target=stopper), Thread(target=collector)] + + @threading_helper.requires_working_threading() class TestType(TestCase): def test_attr_cache(self): @@ -401,6 +423,216 @@ class B(A): with threading_helper.start_threads(threads): pass + def check_stop_the_world_under_type_lock(self, setter): + # gh-158195: Python code that runs with the type lock held and stops + # the world could deadlock with a thread that held the type lock + # pinned while it waited for the stop-the-world mutex (__bases__ + # assignment). The first thread released the type lock while it was + # blocked on the stop-the-world mutex, was handed the mutex, and then + # waited for the type lock while owning the mutex. + done = threading.Event() + + def run_setter(): + barrier.wait() + setter() + + def basesetter(): + class A: + pass + class B: + pass + class C(A): + pass + barrier.wait() + while not done.is_set(): + C.__bases__ = (B,) + C.__bases__ = (A,) + + def stopper(): + def g(): + pass + barrier.wait() + while not done.is_set(): + g.__qualname__ = "g" # stops the world + + # The stopper thread keeps the stop-the-world mutex contended, which + # makes the setters block on it. + setters = [Thread(target=run_setter) for _ in range(2)] + others = [Thread(target=basesetter), Thread(target=stopper)] + barrier = threading.Barrier(len(setters) + len(others)) + with threading_helper.start_threads(setters + others): + for t in setters: + t.join() + done.set() + + def test_stop_the_world_in_mro_deadlock(self): + # type_ready() calls a metaclass mro() with the type lock held. + N = 50 + def f(): + pass + + class Meta(type): + def mro(cls): + f.__qualname__ = "f" # stops the world + return type.mro(cls) + + def setter(): + for _ in range(N): + Meta("K", (), {}) + + self.check_stop_the_world_under_type_lock(setter) + + def test_stop_the_world_in_setattr_key_eq_deadlock(self): + # type_setattro() compares the attribute name with a str subclass key + # of the type's __dict__ (calling its __eq__) with the type lock held. + N = 50 + def f(): + pass + + class S(str): + __hash__ = str.__hash__ + def __eq__(self, other): + f.__qualname__ = "f" # stops the world + return str.__eq__(self, other) + + C = type("C", (), {S("x"): 0}) + + def setter(): + for i in range(N): + C.x = i + + self.check_stop_the_world_under_type_lock(setter) + + def test_concurrent_slot_update_consistency(self): + # Several threads assign and delete the same special method of a + # class with subclasses while other threads stop the world. The + # type lock is released while a thread waits for the stop-the-world, + # so the slot updates it prepared can go stale and must be redone. + # At the end, the type slots must agree with the class dicts. + N = 300 + done = threading.Event() + + class Base: + pass + + subclasses = [type(f"Sub{i}", (Base,), {}) for i in range(5)] + funcs = [lambda self, i=i: f"repr{i}" for i in range(4)] + + def setter(i): + barrier.wait() + for _ in range(N): + Base.__repr__ = funcs[i] + try: + del Base.__repr__ + except AttributeError: + pass + + setters = [Thread(target=setter, args=(i,)) for i in range(len(funcs))] + others = world_stopper_threads(done) + barrier = threading.Barrier(len(setters)) + with threading_helper.start_threads(setters + others): + for t in setters: + t.join() + done.set() + + for cls in [Base] + subclasses: + if "__repr__" in Base.__dict__: + expected = Base.__dict__["__repr__"](cls()) + self.assertEqual(repr(cls()), expected) + else: + # object.__repr__ is used + self.assertStartsWith(repr(cls()), "<") + + def test_slot_update_subclass_freed(self): + # Subclasses are created and freed while another thread updates a + # slot that is inherited by them. The slot update queue must keep + # the subclasses alive until the update is applied, since the type + # lock is released while waiting for the stop-the-world. + N = 300 + done = threading.Event() + + class Base: + pass + + def setter(): + func = lambda self: "x" + barrier.wait() + for _ in range(N): + Base.__repr__ = func + try: + del Base.__repr__ + except AttributeError: + pass + + def subclasser(): + barrier.wait() + while not done.is_set(): + for _ in range(20): + type("Sub", (Base,), {}) + + setters = [Thread(target=setter) for _ in range(2)] + others = ([Thread(target=subclasser), Thread(target=subclasser)] + + world_stopper_threads(done)) + barrier = threading.Barrier(len(setters)) + with threading_helper.start_threads(setters + others): + for t in setters: + t.join() + done.set() + + def test_concurrent_bases_assignment(self): + # Several threads reassign __bases__ of the same class, and of its + # base, while others stop the world. The validated bases and the + # computed MRO can go stale while the type lock is released and + # must be recomputed. At the end, the MRO must agree with __bases__. + N = 100 + done = threading.Event() + + class A: + def meth(self): + return "A" + def __repr__(self): + return "A" + class B: + def meth(self): + return "B" + def __repr__(self): + return "B" + class C(A): + pass + class D(C): + pass + + def setter(): + barrier.wait() + for _ in range(N): + C.__bases__ = (B,) + C.__bases__ = (A,) + + def subsetter(): + barrier.wait() + for _ in range(N): + D.__bases__ = (C, B) + D.__bases__ = (C,) + + setters = [Thread(target=setter), Thread(target=setter), + Thread(target=subsetter)] + others = world_stopper_threads(done) + barrier = threading.Barrier(len(setters)) + with threading_helper.start_threads(setters + others): + for t in setters: + t.join() + done.set() + + self.assertEqual(C.__bases__, (A,)) + self.assertEqual(C.__mro__, (C, A, object)) + self.assertEqual(D.__bases__, (C,)) + self.assertEqual(D.__mro__, (D, C, A, object)) + self.assertEqual(D().meth(), "A") + self.assertEqual(repr(C()), "A") + self.assertEqual(repr(D()), "A") + self.assertIn(C, A.__subclasses__()) + self.assertNotIn(C, B.__subclasses__()) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-10-00-00.gh-issue-158195.UnpinTL.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-10-00-00.gh-issue-158195.UnpinTL.rst new file mode 100644 index 000000000000000..f189c7c563ca58c --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-20-10-00-00.gh-issue-158195.UnpinTL.rst @@ -0,0 +1,7 @@ +Fix a deadlock in the free-threaded build between a thread that stops the +world while running Python code under the type lock (for example a custom +metaclass ``mro()`` or the ``__eq__`` of a :class:`str` subclass used as a +class ``__dict__`` key) and a thread that assigns to a special method or to +``__bases__`` of a class. The type lock is no longer held while waiting +for a stop-the-world; the prepared slot, MRO and bases updates are +validated with the world stopped and recomputed if they went stale. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 5e772e5608d4c66..3a3d11bea803663 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -46,13 +46,6 @@ class object "PyObject *" "&PyBaseObject_Type" #define NEXT_VERSION_TAG(interp) \ (interp)->types.next_version_tag -// Storage for the mutexes saved by type_lock_prevent_release(). Defined for -// both builds so the call sites don't need to be conditionally compiled. -typedef struct { - PyMutex *mutex1; - PyMutex *mutex2; -} pinned_mutexes_t; - #ifdef Py_GIL_DISABLED // There's a global lock for types that ensures that tp_version_tag and @@ -131,55 +124,32 @@ types_start_world(void) assert(!types_world_is_stopped()); } -// Temporarily prevent the mutexes held by the topmost critical section from -// being released when the current thread blocks (blocking detaches the thread, -// which suspends its critical sections and releases the mutexes they hold). -// -// All of the mutexes held by the critical section are pinned, not just -// TYPE_LOCK. If only TYPE_LOCK was pinned then _PyCriticalSection_Resume() -// would have to re-acquire the other mutex while TYPE_LOCK is held. That -// deadlocks against a thread that holds that mutex and is waiting for -// TYPE_LOCK, which is exactly what BEGIN_TYPE_DICT_LOCK() does: the type dict -// mutex is on the heap and TYPE_LOCK is in _PyRuntime, so the address ordering -// used by two-mutex critical sections usually acquires the dict mutex first. -// By pinning both mutexes there is nothing to re-acquire on resume. -// -// Holding the mutexes while blocked does not prevent the world from being -// stopped: a thread waiting on either of them parks with _PY_LOCK_DETACH and -// so is detached while it waits. -static void -type_lock_prevent_release(pinned_mutexes_t *pinned) +// Publish generation counter. Type updates that are prepared under TYPE_LOCK +// and published with the world stopped snapshot it before preparing and +// compare it inside the stop-the-world. It is incremented by every change +// that could invalidate such a prepared update, so a mismatch means the +// update must be prepared again. +static inline uint64_t +types_publish_gen(void) { - PyThreadState *tstate = _PyThreadState_GET(); - uintptr_t tag = tstate->critical_section; - PyCriticalSection *c = (PyCriticalSection *)(tag & ~_Py_CRITICAL_SECTION_MASK); - pinned->mutex1 = c->_cs_mutex; - pinned->mutex2 = NULL; - c->_cs_mutex = NULL; - if ((tag & _Py_CRITICAL_SECTION_TWO_MUTEXES) != 0) { - PyCriticalSection2 *c2 = (PyCriticalSection2 *)c; - pinned->mutex2 = c2->_cs_mutex2; - c2->_cs_mutex2 = NULL; - } - assert(pinned->mutex1 == TYPE_LOCK || pinned->mutex2 == TYPE_LOCK); + PyInterpreterState *interp = _PyInterpreterState_GET(); + return _Py_atomic_load_uint64_relaxed(&interp->types.publish_gen); } -static void -type_lock_allow_release(pinned_mutexes_t *pinned) +static inline void +types_publish_gen_inc(void) { - PyThreadState *tstate = _PyThreadState_GET(); - uintptr_t tag = tstate->critical_section; - PyCriticalSection *c = (PyCriticalSection *)(tag & ~_Py_CRITICAL_SECTION_MASK); - assert(c->_cs_mutex == NULL); - c->_cs_mutex = pinned->mutex1; - if ((tag & _Py_CRITICAL_SECTION_TWO_MUTEXES) != 0) { - PyCriticalSection2 *c2 = (PyCriticalSection2 *)c; - assert(c2->_cs_mutex2 == NULL); - c2->_cs_mutex2 = pinned->mutex2; - } - else { - assert(pinned->mutex2 == NULL); - } + PyInterpreterState *interp = _PyInterpreterState_GET(); + _Py_atomic_add_uint64(&interp->types.publish_gen, 1); +} + +// Return true if the prepared state that was snapshotted at 'gen' is stale. +// Must be called with the world stopped. +static inline bool +types_publish_is_stale(uint64_t gen) +{ + assert(types_world_is_stopped()); + return types_publish_gen() != gen; } #else @@ -195,8 +165,9 @@ type_lock_allow_release(pinned_mutexes_t *pinned) #define types_world_is_stopped() 1 #define types_stop_world() #define types_start_world() -#define type_lock_prevent_release(pinned) ((void)(pinned)) -#define type_lock_allow_release(pinned) ((void)(pinned)) +#define types_publish_gen() 0 +#define types_publish_gen_inc() +#define types_publish_is_stale(gen) 0 #endif @@ -599,7 +570,7 @@ _PyType_GetBases(PyTypeObject *self) } static inline void -set_tp_bases(PyTypeObject *self, PyObject *bases, int initial) +set_tp_bases(PyTypeObject *self, PyObject *bases) { assert(PyTuple_Check(bases)); ASSERT_NEW_TYPE_OR_LOCKED(self); @@ -607,7 +578,6 @@ set_tp_bases(PyTypeObject *self, PyObject *bases, int initial) // XXX tp_bases can probably be statically allocated for each // static builtin type. assert(PyTuple_CheckExact(bases)); - assert(initial); assert(self->tp_bases == NULL); if (PyTuple_GET_SIZE(bases) == 0) { assert(self->tp_base == NULL); @@ -623,6 +593,30 @@ set_tp_bases(PyTypeObject *self, PyObject *bases, int initial) self->tp_bases = bases; } +// Replace tp_bases and tp_base of a type. Returns 1 on success, +// and 0 otherwise. +static int +try_set_tp_bases(PyTypeObject *self, PyObject *expected, + PyObject *bases, PyTypeObject *base, uint64_t *gen) +{ + ASSERT_TYPE_LOCK_HELD(); + assert(PyTuple_Check(bases)); + assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + int stored = 0; + types_stop_world(); + if ((expected == NULL || self->tp_bases == expected) + && (gen == NULL || !types_publish_is_stale(*gen))) + { + self->tp_bases = bases; + self->tp_base = base; + types_publish_gen_inc(); + stored = 1; + } + types_start_world(); + ASSERT_TYPE_LOCK_HELD(); + return stored; +} + static inline void clear_tp_bases(PyTypeObject *self, int final) { @@ -651,14 +645,13 @@ lookup_tp_mro(PyTypeObject *self) } static inline void -set_tp_mro(PyTypeObject *self, PyObject *mro, int initial) +set_tp_mro(PyTypeObject *self, PyObject *mro) { if (mro != NULL) { assert(PyTuple_CheckExact(mro)); if (self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { // XXX tp_mro can probably be statically allocated for each // static builtin type. - assert(initial); assert(self->tp_mro == NULL); /* Other checks are done via set_tp_bases. */ _Py_SetImmortal(mro); @@ -667,16 +660,38 @@ set_tp_mro(PyTypeObject *self, PyObject *mro, int initial) PyUnstable_Object_EnableDeferredRefcount(mro); } } - pinned_mutexes_t pinned; - if (!initial) { - type_lock_prevent_release(&pinned); - types_stop_world(); - } self->tp_mro = mro; - if (!initial) { - types_start_world(); - type_lock_allow_release(&pinned); +} + +// Replace tp_mro with the world stopped. Returns 1 on success, 0 if tp_mro +// is no longer "expected", and -1 if the generation changed. In the last two +// cases the caller still owns 'mro'. +static int +try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, + uint64_t *gen) +{ + ASSERT_TYPE_LOCK_HELD(); + assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + if (mro != NULL) { + assert(PyTuple_CheckExact(mro)); + PyUnstable_Object_EnableDeferredRefcount(mro); + } + int res; + types_stop_world(); + if (self->tp_mro != expected) { + res = 0; } + else if (gen != NULL && types_publish_is_stale(*gen)) { + res = -1; + } + else { + self->tp_mro = mro; + types_publish_gen_inc(); + res = 1; + } + types_start_world(); + ASSERT_TYPE_LOCK_HELD(); + return res; } static inline void @@ -1706,18 +1721,19 @@ type_set_abstractmethods(PyObject *tp, PyObject *value, void *Py_UNUSED(closure) return -1; } - pinned_mutexes_t pinned; BEGIN_TYPE_LOCK(); - _PyType_Modified_Unlocked(type); - type_lock_prevent_release(&pinned); types_stop_world(); if (abstract) type_add_flags(type, Py_TPFLAGS_IS_ABSTRACT); else type_clear_flags(type, Py_TPFLAGS_IS_ABSTRACT); types_start_world(); - type_lock_allow_release(&pinned); + // Invalidate the version tag after the flag update, while TYPE_LOCK is + // still held: no other thread can assign a new tag before we release it. + // The lock may have been released while waiting for the stop-the-world, + // so invalidating before the update would not be enough. ASSERT_TYPE_LOCK_HELD(); + _PyType_Modified_Unlocked(type); END_TYPE_LOCK(); return 0; @@ -1794,8 +1810,13 @@ mro_hierarchy_for_complete_type(PyTypeObject *type, PyObject *temp) Py_XDECREF(tuple); if (res < 0) { - set_tp_mro(type, old_mro, 0); - Py_DECREF(new_mro); + // Roll back, unless tp_mro has already been replaced again. + if (try_set_tp_mro(type, new_mro, old_mro, NULL) == 1) { + Py_DECREF(new_mro); + } + else { + Py_XDECREF(old_mro); + } return -1; } Py_XDECREF(old_mro); @@ -1831,8 +1852,10 @@ mro_hierarchy_for_complete_type(PyTypeObject *type, PyObject *temp) return res; } +// Checks on the new __bases__ value that do not depend on mutable type +// state. Done once per assignment. static int -type_check_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **best_base) +type_check_new_bases(PyTypeObject *type, PyObject *new_bases) { // Check arguments, this is re-entrant due to the PySys_Audit() call if (!check_set_special_type_attr(type, new_bases, "__bases__")) { @@ -1861,7 +1884,22 @@ type_check_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **bes type->tp_name, Py_TYPE(ob)->tp_name); return -1; } - PyTypeObject *base = (PyTypeObject*)ob; + } + return 0; +} + +// Checks on the new __bases__ value that depend on the current state of the +// type hierarchy (inheritance cycles, layout compatibility). These can run +// Python code and release TYPE_LOCK, so they are validated optimistically: +// the caller snapshots the publish generation before calling this and +// the new bases are only stored if the generation is unchanged. +static int +type_validate_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **best_base) +{ + ASSERT_TYPE_LOCK_HELD(); + Py_ssize_t n = PyTuple_GET_SIZE(new_bases); + for (Py_ssize_t i = 0; i < n; i++) { + PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(new_bases, i); if (is_subtype_with_mro(lookup_tp_mro(base), base, type) || /* In case of reentering here again through a custom mro() @@ -1894,8 +1932,11 @@ type_check_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **bes return 0; } +// Returns 0 on success, -1 on error and 1 if the validated bases went stale +// before they could be stored (the caller must validate again). static int -type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *best_base) +type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, + PyTypeObject *best_base, uint64_t gen) { ASSERT_TYPE_LOCK_HELD(); @@ -1903,14 +1944,14 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *b PyObject *old_bases = lookup_tp_bases(type); assert(old_bases != NULL); PyTypeObject *old_base = type->tp_base; - pinned_mutexes_t pinned; - type_lock_prevent_release(&pinned); - types_stop_world(); - set_tp_bases(type, Py_NewRef(new_bases), 0); - type->tp_base = (PyTypeObject *)Py_NewRef(best_base); - types_start_world(); - type_lock_allow_release(&pinned); + if (!try_set_tp_bases(type, NULL, Py_NewRef(new_bases), + (PyTypeObject *)Py_NewRef(best_base), &gen)) + { + Py_DECREF(new_bases); + Py_DECREF(best_base); + return 1; + } PyObject *temp = PyList_New(0); if (temp == NULL) { @@ -1960,24 +2001,19 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *b PyArg_UnpackTuple(PyList_GET_ITEM(temp, i), "", 2, 3, &cls, &new_mro, &old_mro); /* Do not rollback if cls has a newer version of MRO. */ - if (lookup_tp_mro(cls) == new_mro) { - set_tp_mro(cls, Py_XNewRef(old_mro), 0); + old_mro = Py_XNewRef(old_mro); + if (try_set_tp_mro(cls, new_mro, old_mro, NULL) == 1) { Py_DECREF(new_mro); } + else { + Py_XDECREF(old_mro); + } } Py_DECREF(temp); bail: - if (lookup_tp_bases(type) == new_bases) { - assert(type->tp_base == best_base); - - type_lock_prevent_release(&pinned); - types_stop_world(); - set_tp_bases(type, old_bases, 0); - type->tp_base = old_base; - types_start_world(); - type_lock_allow_release(&pinned); - + /* Restore the old bases, unless tp_bases has been replaced again. */ + if (try_set_tp_bases(type, new_bases, old_bases, old_base, NULL)) { Py_DECREF(new_bases); Py_DECREF(best_base); } @@ -1996,11 +2032,17 @@ type_set_bases(PyObject *tp, PyObject *new_bases, void *Py_UNUSED(closure)) PyTypeObject *type = PyTypeObject_CAST(tp); PyTypeObject *best_base; int res; - BEGIN_TYPE_LOCK(); - res = type_check_new_bases(type, new_bases, &best_base); - if (res == 0) { - res = type_set_bases_unlocked(type, new_bases, best_base); + if (type_check_new_bases(type, new_bases) < 0) { + return -1; } + BEGIN_TYPE_LOCK(); + do { + uint64_t gen = types_publish_gen(); + res = type_validate_new_bases(type, new_bases, &best_base); + if (res == 0) { + res = type_set_bases_unlocked(type, new_bases, best_base, gen); + } + } while (res == 1); END_TYPE_LOCK(); return res; } @@ -3632,25 +3674,52 @@ mro_internal(PyTypeObject *type, int initial, PyObject **p_old_mro) ASSERT_TYPE_LOCK_HELD(); PyObject *new_mro, *old_mro; - int reent; /* Keep a reference to be able to do a reentrancy check below. Don't let old_mro be GC'ed and its address be reused for another object, like (suddenly!) a new tp_mro. */ old_mro = Py_XNewRef(lookup_tp_mro(type)); - new_mro = mro_invoke(type); /* might cause reentrance */ - reent = (lookup_tp_mro(type) != old_mro); - Py_XDECREF(old_mro); - if (new_mro == NULL) { - return -1; - } - - if (reent) { + for (;;) { + uint64_t gen = types_publish_gen(); + new_mro = mro_invoke(type); /* might cause reentrance */ + if (new_mro == NULL) { + Py_XDECREF(old_mro); + return -1; + } + int res; + if (initial) { + // The type has not been revealed to other threads yet, so only + // same-thread reentrance can have replaced tp_mro. + if (lookup_tp_mro(type) == old_mro) { + set_tp_mro(type, new_mro); + res = 1; + } + else { + res = 0; + } + } + else { + // Store with the world stopped. TYPE_LOCK may have been + // released while mro_invoke() ran or while waiting to stop the + // world, so check for reentrance and for concurrent mutation of + // the hierarchy inside the stop-the-world. + res = try_set_tp_mro(type, old_mro, new_mro, &gen); + } + if (res == 1) { + break; + } Py_DECREF(new_mro); - return 0; + if (res == 0) { + /* reentrance: tp_mro was replaced by someone else */ + Py_XDECREF(old_mro); + return 0; + } + /* stale: another thread changed the hierarchy, compute again */ + assert(res == -1); } - - set_tp_mro(type, new_mro, initial); + /* Drop our temporary reference. The reference previously owned by + the type is now owned by this function and transferred below. */ + Py_XDECREF(old_mro); type_mro_modified(type, new_mro); /* corner case: the super class might have been hidden @@ -3780,6 +3849,11 @@ solid_base(PyTypeObject *type) // are finished the lookups, we stop-the-world and apply all of the updates. // The apply_slot_updates() code is simple and easy to confirm that it is // safe. +// +// TYPE_LOCK is a critical section and so it can be released while the lookups +// run (they can call Python code and block on other locks) and while waiting +// to stop the world. The queue is therefore validated with the publish +// generation before it is applied and rebuilt if it went stale. typedef struct { PyTypeObject *type; @@ -3821,14 +3895,25 @@ slot_update_new_chunk(void) } static void -slot_update_free_chunks(slot_update_t *updates) +slot_update_clear(slot_update_t *updates, slot_update_chunk_t *keep) { + assert(!types_world_is_stopped()); slot_update_chunk_t *chunk = updates->head; while (chunk != NULL) { slot_update_chunk_t *prev = chunk->prev; - PyMem_Free(chunk); + for (Py_ssize_t i = 0; i < chunk->n; i++) { + Py_DECREF(chunk->updates[i].type); + } + if (chunk == keep) { + chunk->n = 0; + chunk->prev = NULL; + } + else { + PyMem_Free(chunk); + } chunk = prev; } + updates->head = keep; } static int @@ -3847,7 +3932,7 @@ queue_slot_update(slot_update_t *updates, PyTypeObject *type, updates->head = chunk; } slot_update_item_t *item = &updates->head->updates[updates->head->n]; - item->type = type; + item->type = (PyTypeObject *)Py_NewRef(type); item->slot_ptr = slot_ptr; item->slot_value = slot_value; updates->head->n++; @@ -3855,6 +3940,12 @@ queue_slot_update(slot_update_t *updates, PyTypeObject *type, return 0; } +static inline int +slot_update_is_empty(slot_update_t *updates) +{ + return updates->head == NULL || updates->head->n == 0; +} + static void apply_slot_updates(slot_update_t *updates) { @@ -3873,38 +3964,36 @@ apply_slot_updates(slot_update_t *updates) } } -static void -apply_type_slot_updates(slot_update_t *updates) -{ - // This must be done carefully to avoid data races and deadlocks. We - // have just updated the type __dict__, while holding TYPE_LOCK. We have - // collected all of the required type slot updates into the 'updates' - // queue. Note that those updates can apply to multiple types since - // subclasses might also be affected by the dict change. - // - // We need to prevent other threads from writing to the dict before we can - // finish updating the slots. The actual stores to the slots are done - // with the world stopped. If we block on the stop-the-world mutex then - // we could release TYPE_LOCK mutex and potentially allow other threads - // to update the dict. That's because TYPE_LOCK was acquired using a - // critical section. - // - // The type_lock_prevent_release() call prevents the mutexes held by the - // critical section (TYPE_LOCK and the type dict mutex) from being released - // even if we block on the STW mutex. We need to take care that we do not - // deadlock because of that. It is safe because a thread waiting for either - // of those mutexes detaches while it waits and so does not hold up the - // stop-the-world. Pinning both mutexes rather than only TYPE_LOCK is what - // makes this safe: otherwise the dict mutex would be released when we - // block and _PyCriticalSection_Resume() would have to re-acquire it while - // holding TYPE_LOCK, deadlocking with a thread that holds the dict mutex - // and is waiting for TYPE_LOCK. - pinned_mutexes_t pinned; - type_lock_prevent_release(&pinned); +// Apply the queued slot updates with the world stopped, if the publish +// generation is still 'gen' (the value snapshotted before the +// queue was built). Returns 1 if the updates were applied and 0 if the +// queue went stale and must be rebuilt. +// +// The queue was built while holding TYPE_LOCK (and, for type_setattro(), +// the type dict mutex). Both are critical section mutexes, so they were +// released whenever the lookups blocked and are released while this thread +// waits to stop the world. Other threads can update type dicts, MROs and +// slots in the meantime, which is what the generation check detects. +// +// Must be called with TYPE_LOCK held. The lock is re-acquired when the +// thread re-attaches after waiting for the stop-the-world mutex, so it is +// held continuously from before the world is stopped until after it is +// started again. Callers rely on that to invalidate version tags after the +// update, before any other thread can assign a new one. +static int +apply_type_slot_updates(slot_update_t *updates, uint64_t gen) +{ + ASSERT_TYPE_LOCK_HELD(); + int applied = 0; types_stop_world(); - apply_slot_updates(updates); + if (!types_publish_is_stale(gen)) { + apply_slot_updates(updates); + types_publish_gen_inc(); + applied = 1; + } types_start_world(); - type_lock_allow_release(&pinned); + ASSERT_TYPE_LOCK_HELD(); + return applied; } #else @@ -4424,7 +4513,7 @@ type_new_alloc(type_new_ctx *ctx) type->tp_as_mapping = &et->as_mapping; type->tp_as_buffer = &et->as_buffer; - set_tp_bases(type, Py_NewRef(ctx->bases), 1); + set_tp_bases(type, Py_NewRef(ctx->bases)); type->tp_base = (PyTypeObject *)Py_NewRef(ctx->base); type->tp_dealloc = subtype_dealloc; @@ -5630,7 +5719,7 @@ type_from_slots_or_spec( /* Set slots we have prepared */ type->tp_base = (PyTypeObject *)Py_NewRef(base); - set_tp_bases(type, bases, 1); + set_tp_bases(type, bases); bases = NULL; // We give our reference to bases to the type type->tp_doc = tp_doc; @@ -6397,24 +6486,22 @@ void _PyType_SetFlagsRecursive(PyTypeObject *self, unsigned long mask, unsigned long flags) { BEGIN_TYPE_LOCK(); - /* Ideally, changing flags and invalidating the old version tag would - happen in one step. But _PyType_Modified_Unlocked() is re-entrant and - cannot run with the world stopped, so we must invalidate first. - Immutable/static-builtin types are skipped because - set_flags_recursive() does not modify them. */ - if (!PyType_HasFeature(self, Py_TPFLAGS_IMMUTABLETYPE) && - (self->tp_flags & mask) != flags) - { - _PyType_Modified_Unlocked(self); - } - /* Keep TYPE_LOCK held while waiting for stop-the-world so no thread - can reassign a version tag before the flag update. */ - pinned_mutexes_t pinned; - type_lock_prevent_release(&pinned); + // Immutable/static-builtin types are skipped because + // set_flags_recursive() does not modify them. + int modified = (!PyType_HasFeature(self, Py_TPFLAGS_IMMUTABLETYPE) && + (self->tp_flags & mask) != flags); types_stop_world(); set_flags_recursive(self, mask, flags); types_start_world(); - type_lock_allow_release(&pinned); + // Invalidate the version tags after the flag update, while TYPE_LOCK is + // still held: no other thread can assign a new tag before we release it. + // _PyType_Modified_Unlocked() is re-entrant and cannot run with the world + // stopped, and invalidating before the update would not be enough because + // the lock may be released while waiting for the stop-the-world. + ASSERT_TYPE_LOCK_HELD(); + if (modified) { + _PyType_Modified_Unlocked(self); + } END_TYPE_LOCK(); } @@ -6584,7 +6671,7 @@ _Py_type_getattro_stackref(PyTypeObject *type, PyObject *name, // the type versions. static int type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name, - PyObject *value, PyObject **old_value) + PyObject *value, int is_slot_name, PyObject **old_value) { // We don't want any re-entrancy between when we update the dict // and call _PyType_Modified_Unlocked, including running the destructor @@ -6611,6 +6698,10 @@ type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name, return -1; } + if (is_slot_name) { + types_publish_gen_inc(); + } + return 0; } @@ -6625,16 +6716,27 @@ update_slot_after_setattr(PyTypeObject *type, PyObject *name) // the type has more than SLOT_UPDATE_CHUNK_SIZE subclasses. slot_update_t queued_updates = {&chunk}; - int res = update_slot(type, name, &queued_updates); - if (res == 0 && queued_updates.head->n > 0) { - apply_type_slot_updates(&queued_updates); - ASSERT_TYPE_LOCK_HELD(); + int res; + for (;;) { + uint64_t gen = types_publish_gen(); + res = update_slot(type, name, &queued_updates); + int done = 1; + if (res == 0 && !slot_update_is_empty(&queued_updates)) { + done = apply_type_slot_updates(&queued_updates, gen); + } + slot_update_clear(&queued_updates, &chunk); + if (res < 0 || done) { + break; + } + // The queue went stale while TYPE_LOCK was released, build it again. } - slot_update_chunk_t *cur = queued_updates.head; - while (cur != &chunk) { - slot_update_chunk_t *prev = cur->prev; - PyMem_Free(cur); - cur = prev; + if (res == 0) { + // Invalidate again, after the slots are updated and while TYPE_LOCK + // is still held. Other threads may have assigned new version tags + // while the lock was released and cached decisions based on the old + // slots. This is a no-op if the tags are still invalid. + ASSERT_TYPE_LOCK_HELD(); + _PyType_Modified_Unlocked(type); } return res; #else @@ -6723,14 +6825,15 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value) } } + // Does the name correspond to a type slot? + int is_slot_name = is_dunder_name(name) && has_slotdef(name); + BEGIN_TYPE_DICT_LOCK(dict); - res = type_update_dict(type, (PyDictObject *)dict, name, value, &old_value); + res = type_update_dict(type, (PyDictObject *)dict, name, value, + is_slot_name, &old_value); assert(_PyType_CheckConsistency(type)); - if (res == 0) { - if (is_dunder_name(name) && has_slotdef(name)) { - // The name corresponds to a type slot. - res = update_slot_after_setattr(type, name); - } + if (res == 0 && is_slot_name) { + res = update_slot_after_setattr(type, name); } END_TYPE_DICT_LOCK(); @@ -9021,7 +9124,7 @@ type_ready_set_bases(PyTypeObject *type, int initial) if (bases == NULL) { return -1; } - set_tp_bases(type, bases, 1); + set_tp_bases(type, bases); } return 0; } @@ -12103,18 +12206,23 @@ update_all_slots(PyTypeObject* type) // can fail internally for other reasons (a lookup fails) but those // errors are suppressed. slot_update_t queued_updates = {0}; - for (pytype_slotdef *p = slotdefs; p->name; p++) { - if (update_slot(type, p->name_strobj, &queued_updates) < 0) { - if (queued_updates.head) { - slot_update_free_chunks(&queued_updates); + for (;;) { + uint64_t gen = types_publish_gen(); + for (pytype_slotdef *p = slotdefs; p->name; p++) { + if (update_slot(type, p->name_strobj, &queued_updates) < 0) { + slot_update_clear(&queued_updates, NULL); + return -1; } - return -1; } - } - if (queued_updates.head != NULL) { - apply_type_slot_updates(&queued_updates); - ASSERT_TYPE_LOCK_HELD(); - slot_update_free_chunks(&queued_updates); + int done = 1; + if (!slot_update_is_empty(&queued_updates)) { + done = apply_type_slot_updates(&queued_updates, gen); + } + slot_update_clear(&queued_updates, NULL); + if (done) { + break; + } + // The queue went stale while TYPE_LOCK was released, build it again. } return 0; } From 45823fae965603df9f885458bafeb712126b041a Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 16:55:19 -0700 Subject: [PATCH 2/8] Replace int pointer with int and bool option. This is more clear than passing the "gen" as a pointer and using NULL to indicate that we don't want to check the generation. --- Objects/typeobject.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 3a3d11bea803663..b7a5c36d98b96b7 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -597,7 +597,7 @@ set_tp_bases(PyTypeObject *self, PyObject *bases) // and 0 otherwise. static int try_set_tp_bases(PyTypeObject *self, PyObject *expected, - PyObject *bases, PyTypeObject *base, uint64_t *gen) + PyObject *bases, PyTypeObject *base, uint64_t gen, bool check_gen) { ASSERT_TYPE_LOCK_HELD(); assert(PyTuple_Check(bases)); @@ -605,7 +605,7 @@ try_set_tp_bases(PyTypeObject *self, PyObject *expected, int stored = 0; types_stop_world(); if ((expected == NULL || self->tp_bases == expected) - && (gen == NULL || !types_publish_is_stale(*gen))) + && (!check_gen || !types_publish_is_stale(gen))) { self->tp_bases = bases; self->tp_base = base; @@ -668,7 +668,7 @@ set_tp_mro(PyTypeObject *self, PyObject *mro) // cases the caller still owns 'mro'. static int try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, - uint64_t *gen) + uint64_t gen, bool check_gen) { ASSERT_TYPE_LOCK_HELD(); assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); @@ -681,7 +681,7 @@ try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, if (self->tp_mro != expected) { res = 0; } - else if (gen != NULL && types_publish_is_stale(*gen)) { + else if (check_gen && types_publish_is_stale(gen)) { res = -1; } else { @@ -1811,7 +1811,7 @@ mro_hierarchy_for_complete_type(PyTypeObject *type, PyObject *temp) if (res < 0) { // Roll back, unless tp_mro has already been replaced again. - if (try_set_tp_mro(type, new_mro, old_mro, NULL) == 1) { + if (try_set_tp_mro(type, new_mro, old_mro, 0, false) == 1) { Py_DECREF(new_mro); } else { @@ -1946,7 +1946,7 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *old_base = type->tp_base; if (!try_set_tp_bases(type, NULL, Py_NewRef(new_bases), - (PyTypeObject *)Py_NewRef(best_base), &gen)) + (PyTypeObject *)Py_NewRef(best_base), gen, true)) { Py_DECREF(new_bases); Py_DECREF(best_base); @@ -2002,7 +2002,7 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, "", 2, 3, &cls, &new_mro, &old_mro); /* Do not rollback if cls has a newer version of MRO. */ old_mro = Py_XNewRef(old_mro); - if (try_set_tp_mro(cls, new_mro, old_mro, NULL) == 1) { + if (try_set_tp_mro(cls, new_mro, old_mro, 0, false) == 1) { Py_DECREF(new_mro); } else { @@ -2013,7 +2013,7 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, bail: /* Restore the old bases, unless tp_bases has been replaced again. */ - if (try_set_tp_bases(type, new_bases, old_bases, old_base, NULL)) { + if (try_set_tp_bases(type, new_bases, old_bases, old_base, 0, false)) { Py_DECREF(new_bases); Py_DECREF(best_base); } @@ -3703,7 +3703,7 @@ mro_internal(PyTypeObject *type, int initial, PyObject **p_old_mro) // released while mro_invoke() ran or while waiting to stop the // world, so check for reentrance and for concurrent mutation of // the hierarchy inside the stop-the-world. - res = try_set_tp_mro(type, old_mro, new_mro, &gen); + res = try_set_tp_mro(type, old_mro, new_mro, gen, true); } if (res == 1) { break; From 1951c50d8e002e234b5f01c8e4bdcca0521781ca Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 20:17:47 -0700 Subject: [PATCH 3/8] Strengthen type-lock regression tests and reduce their cost Use explicit stop-the-world pauses and finite worker workloads. Replace slot-update stress with a deterministic reentrancy test, and consolidate queue reference-release coverage. --- .../test_free_threading/test_threading.py | 24 +++ Lib/test/test_free_threading/test_type.py | 200 +++++++----------- Modules/_testinternalcapi.c | 38 ++++ 3 files changed, 138 insertions(+), 124 deletions(-) diff --git a/Lib/test/test_free_threading/test_threading.py b/Lib/test/test_free_threading/test_threading.py index ecc42db7b44f3af..670d66e1d0ebb23 100644 --- a/Lib/test/test_free_threading/test_threading.py +++ b/Lib/test/test_free_threading/test_threading.py @@ -26,6 +26,30 @@ def mutate_thread(): class TestThreadState(unittest.TestCase): + def test_stop_the_world(self): + import _testinternalcapi + + _testinternalcapi.stop_the_world() + _testinternalcapi.stop_the_world(0) + + def pause(): + for _ in range(10): + _testinternalcapi.stop_the_world(0.0001) + + threading_helper.run_concurrently(pause, nthreads=2) + for duration in (-1, -0.1, float("nan")): + with self.subTest(duration=duration): + with self.assertRaises(ValueError): + _testinternalcapi.stop_the_world(duration) + for duration in (float("inf"), 10**100): + with self.subTest(duration=duration): + with self.assertRaises(OverflowError): + _testinternalcapi.stop_the_world(duration) + for duration in (None, "0"): + with self.subTest(duration=duration): + with self.assertRaises(TypeError): + _testinternalcapi.stop_the_world(duration) + @support.requires_subprocess() def test_tight_stw_loop_does_not_starve_attach(self): script = textwrap.dedent(f""" diff --git a/Lib/test/test_free_threading/test_type.py b/Lib/test/test_free_threading/test_type.py index 3717eb6b8202874..a19638b083be5dd 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -1,6 +1,7 @@ import gc import threading import unittest +import weakref from concurrent.futures import ThreadPoolExecutor from threading import Barrier, Thread @@ -20,25 +21,22 @@ class A: attr = 1 -def world_stopper_threads(done): - """Threads that stop the world until 'done' is set. +def world_stoppers(): + """Finite workloads that contend for the stop-the-world mutex. - The stopper keeps the stop-the-world mutex contended, so that threads - updating types block on it and release the type lock. The collector - makes the stop-the-world pauses long, so that other threads have time - to update types while the lock is released. + Waiting for the mutex releases the type lock, allowing queued type + updates to go stale. Use a short pause rather than full collections + to prolong the contention without depending on the size of the heap. """ def stopper(): - def g(): - pass - while not done.is_set(): - g.__qualname__ = "g" # stops the world + for _ in range(100): + _testinternalcapi.stop_the_world() - def collector(): - while not done.is_set(): - gc.collect() + def pauser(): + for _ in range(100): + _testinternalcapi.stop_the_world(0.0001) - return [Thread(target=stopper), Thread(target=collector)] + return [stopper, pauser] @threading_helper.requires_working_threading() @@ -366,6 +364,12 @@ def custom_repr(self): self.assertTrue(all(repr(cls()) != "custom repr" for cls in subclasses)) + # The slot update queue must release its references to the types, + # including those stored in heap-allocated chunks. + del subclasses + gc.collect() + self.assertEqual(Base.__subclasses__(), []) + def test_concurrent_setattr_deadlock(self): # gh-155400: two threads assigning to a special method of the same # class could deadlock. One thread held the type lock and waited for @@ -430,12 +434,6 @@ def check_stop_the_world_under_type_lock(self, setter): # assignment). The first thread released the type lock while it was # blocked on the stop-the-world mutex, was handed the mutex, and then # waited for the type lock while owning the mutex. - done = threading.Event() - - def run_setter(): - barrier.wait() - setter() - def basesetter(): class A: pass @@ -443,37 +441,27 @@ class B: pass class C(A): pass - barrier.wait() - while not done.is_set(): + for _ in range(100): C.__bases__ = (B,) C.__bases__ = (A,) def stopper(): - def g(): - pass - barrier.wait() - while not done.is_set(): - g.__qualname__ = "g" # stops the world - - # The stopper thread keeps the stop-the-world mutex contended, which - # makes the setters block on it. - setters = [Thread(target=run_setter) for _ in range(2)] - others = [Thread(target=basesetter), Thread(target=stopper)] - barrier = threading.Barrier(len(setters) + len(others)) - with threading_helper.start_threads(setters + others): - for t in setters: - t.join() - done.set() + for _ in range(100): + _testinternalcapi.stop_the_world() + + # The stopper keeps the stop-the-world mutex contended, which makes + # the setters block on it. All workers have finite workloads so + # that repeated invalidation cannot indefinitely delay a setter. + threading_helper.run_concurrently( + [setter, setter, basesetter, stopper]) def test_stop_the_world_in_mro_deadlock(self): # type_ready() calls a metaclass mro() with the type lock held. N = 50 - def f(): - pass class Meta(type): def mro(cls): - f.__qualname__ = "f" # stops the world + _testinternalcapi.stop_the_world() return type.mro(cls) def setter(): @@ -486,13 +474,11 @@ def test_stop_the_world_in_setattr_key_eq_deadlock(self): # type_setattro() compares the attribute name with a str subclass key # of the type's __dict__ (calling its __eq__) with the type lock held. N = 50 - def f(): - pass class S(str): __hash__ = str.__hash__ def __eq__(self, other): - f.__qualname__ = "f" # stops the world + _testinternalcapi.stop_the_world() return str.__eq__(self, other) C = type("C", (), {S("x"): 0}) @@ -503,89 +489,63 @@ def setter(): self.check_stop_the_world_under_type_lock(setter) - def test_concurrent_slot_update_consistency(self): - # Several threads assign and delete the same special method of a - # class with subclasses while other threads stop the world. The - # type lock is released while a thread waits for the stop-the-world, - # so the slot updates it prepared can go stale and must be redone. - # At the end, the type slots must agree with the class dicts. - N = 300 - done = threading.Event() + def test_reentrant_slot_update_consistency(self): + # Updating Base queues its slot update before visiting Sub. A key + # comparison in Sub's dict re-enters the setter and makes that queue + # stale. It must be rebuilt rather than overwriting the inner update. + armed = False class Base: pass - subclasses = [type(f"Sub{i}", (Base,), {}) for i in range(5)] - funcs = [lambda self, i=i: f"repr{i}" for i in range(4)] - - def setter(i): - barrier.wait() - for _ in range(N): - Base.__repr__ = funcs[i] - try: - del Base.__repr__ - except AttributeError: - pass - - setters = [Thread(target=setter, args=(i,)) for i in range(len(funcs))] - others = world_stopper_threads(done) - barrier = threading.Barrier(len(setters)) - with threading_helper.start_threads(setters + others): - for t in setters: - t.join() - done.set() - - for cls in [Base] + subclasses: - if "__repr__" in Base.__dict__: - expected = Base.__dict__["__repr__"](cls()) - self.assertEqual(repr(cls()), expected) - else: - # object.__repr__ is used - self.assertStartsWith(repr(cls()), "<") - - def test_slot_update_subclass_freed(self): - # Subclasses are created and freed while another thread updates a - # slot that is inherited by them. The slot update queue must keep - # the subclasses alive until the update is applied, since the type - # lock is released while waiting for the stop-the-world. - N = 300 - done = threading.Event() + class Key(str): + def __hash__(self): + return hash("__hash__") - class Base: - pass - - def setter(): - func = lambda self: "x" - barrier.wait() - for _ in range(N): - Base.__repr__ = func - try: - del Base.__repr__ - except AttributeError: - pass + def __eq__(self, other): + nonlocal armed + if armed and other == "__hash__": + armed = False + Base.__hash__ = object.__hash__ + return str.__eq__(self, other) - def subclasser(): - barrier.wait() - while not done.is_set(): - for _ in range(20): - type("Sub", (Base,), {}) - - setters = [Thread(target=setter) for _ in range(2)] - others = ([Thread(target=subclasser), Thread(target=subclasser)] - + world_stopper_threads(done)) - barrier = threading.Barrier(len(setters)) - with threading_helper.start_threads(setters + others): - for t in setters: - t.join() - done.set() + # Collide with the special-method name without defining it on Sub, + # so that slot updates still propagate from Base to Sub. + Sub = type("Sub", (Base,), {Key("trigger"): None}) + objects = [Base(), Sub()] + + # Check actual slot dispatch, not just the class dictionaries. None + # and object.__hash__ use distinct C slots, unlike Python functions + # which share a generic wrapper that looks up the current method. + Base.__hash__ = None + for obj in objects: + with self.assertRaises(TypeError): + hash(obj) + Base.__hash__ = object.__hash__ + for obj in objects: + self.assertEqual(hash(obj), object.__hash__(obj)) + + armed = True + Base.__hash__ = None + self.assertFalse(armed) + self.assertIs(Base.__dict__["__hash__"], object.__hash__) + for obj in objects: + self.assertEqual(hash(obj), object.__hash__(obj)) + + # Both applied and discarded queues must release their references. + base_ref = weakref.ref(Base) + sub_ref = weakref.ref(Sub) + del obj, objects, Sub, Base + gc.collect() + self.assertIsNone(base_ref()) + self.assertIsNone(sub_ref()) def test_concurrent_bases_assignment(self): # Several threads reassign __bases__ of the same class, and of its # base, while others stop the world. The validated bases and the # computed MRO can go stale while the type lock is released and # must be recomputed. At the end, the MRO must agree with __bases__. - N = 100 - done = threading.Event() + N = 50 class A: def meth(self): @@ -603,25 +563,17 @@ class D(C): pass def setter(): - barrier.wait() for _ in range(N): C.__bases__ = (B,) C.__bases__ = (A,) def subsetter(): - barrier.wait() for _ in range(N): D.__bases__ = (C, B) D.__bases__ = (C,) - setters = [Thread(target=setter), Thread(target=setter), - Thread(target=subsetter)] - others = world_stopper_threads(done) - barrier = threading.Barrier(len(setters)) - with threading_helper.start_threads(setters + others): - for t in setters: - t.join() - done.set() + threading_helper.run_concurrently( + [setter, setter, subsetter] + world_stoppers()) self.assertEqual(C.__bases__, (A,)) self.assertEqual(C.__mro__, (C, A, object)) diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 479297a996c2502..cac040efdd2b92a 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -38,6 +38,7 @@ #include "pycore_pylifecycle.h" // _PyInterpreterConfig_InitFromDict() #include "pycore_pystate.h" // _PyThreadState_GET() #include "pycore_runtime_structs.h" // _PY_NSMALLPOSINTS +#include "pycore_time.h" // _PyTime_FromSecondsObject() #include "pycore_unicodeobject.h" // _PyUnicode_TransformDecimalAndSpaceToASCII() #include "clinic/_testinternalcapi.c.h" @@ -209,6 +210,42 @@ get_stack_margin(PyObject *self, PyObject *Py_UNUSED(args)) return PyLong_FromSize_t(_PyOS_STACK_MARGIN_BYTES); } +static PyObject * +stop_the_world(PyObject *self, PyObject *args) +{ + PyObject *duration_obj = NULL; + PyTime_t duration = 0; + if (!PyArg_ParseTuple(args, "|O:stop_the_world", &duration_obj)) { + return NULL; + } + if (duration_obj != NULL && + _PyTime_FromSecondsObject(&duration, duration_obj, + _PyTime_ROUND_TIMEOUT) < 0) { + return NULL; + } + if (duration < 0) { + PyErr_SetString(PyExc_ValueError, "duration must be non-negative"); + return NULL; + } +#ifdef Py_GIL_DISABLED + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + if (duration != 0) { + PyEvent event = {0}; + PyEvent_WaitTimed(&event, duration, /*detach=*/0); + } + _PyEval_StartTheWorld(interp); +#endif + Py_RETURN_NONE; +} + +PyDoc_STRVAR(stop_the_world_doc, +"stop_the_world($module, duration=0, /)\n" +"--\n" +"\n" +"Stop the world, wait for duration seconds, then restart it.\n" +"Does nothing in GIL-enabled builds."); + static PyObject * test_stop_the_world(PyObject *self, PyObject *Py_UNUSED(args)) { @@ -3345,6 +3382,7 @@ static PyMethodDef module_functions[] = { {"get_c_recursion_remaining", get_c_recursion_remaining, METH_NOARGS}, {"get_stack_pointer", get_stack_pointer, METH_NOARGS}, {"get_stack_margin", get_stack_margin, METH_NOARGS}, + {"stop_the_world", stop_the_world, METH_VARARGS, stop_the_world_doc}, {"test_stop_the_world", test_stop_the_world, METH_NOARGS}, {"classify_stack_addresses", classify_stack_addresses, METH_VARARGS}, {"get_jit_code_ranges", get_jit_code_ranges, METH_NOARGS}, From 167ed1801de65e835020a8f254a853cb7e87a600 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 21:20:09 -0700 Subject: [PATCH 4/8] Enable deferred refcounting only on the stored MRO --- Objects/typeobject.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b7a5c36d98b96b7..d965438127b1949 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -672,10 +672,7 @@ try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, { ASSERT_TYPE_LOCK_HELD(); assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); - if (mro != NULL) { - assert(PyTuple_CheckExact(mro)); - PyUnstable_Object_EnableDeferredRefcount(mro); - } + assert(mro == NULL || PyTuple_CheckExact(mro)); int res; types_stop_world(); if (self->tp_mro != expected) { @@ -685,6 +682,9 @@ try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, res = -1; } else { + if (mro != NULL) { + PyUnstable_Object_EnableDeferredRefcount(mro); + } self->tp_mro = mro; types_publish_gen_inc(); res = 1; From be1b1fd0cdb69d2435862b7a4a0fba65bf262878 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 21:20:31 -0700 Subject: [PATCH 5/8] Store a private copy of the assigned __bases__ tuple The rollback on error restores the old bases only if tp_bases is still the tuple this assignment stored, identified by object identity. That breaks if another assignment stores the same tuple object in between. Store a private copy so that the identity check is reliable. --- Objects/typeobject.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index d965438127b1949..10e6c94e3774ab3 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2035,15 +2035,27 @@ type_set_bases(PyObject *tp, PyObject *new_bases, void *Py_UNUSED(closure)) if (type_check_new_bases(type, new_bases) < 0) { return -1; } + // Create a fresh bases tuple to store. Its identity tells whether + // tp_bases was replaced again while TYPE_LOCK was released or due to + // re-entrancy from the mro() call. + Py_ssize_t n = PyTuple_GET_SIZE(new_bases); + PyObject *bases = PyTuple_New(n); + if (bases == NULL) { + return -1; + } + for (Py_ssize_t i = 0; i < n; i++) { + PyTuple_SET_ITEM(bases, i, Py_NewRef(PyTuple_GET_ITEM(new_bases, i))); + } BEGIN_TYPE_LOCK(); do { uint64_t gen = types_publish_gen(); - res = type_validate_new_bases(type, new_bases, &best_base); + res = type_validate_new_bases(type, bases, &best_base); if (res == 0) { - res = type_set_bases_unlocked(type, new_bases, best_base, gen); + res = type_set_bases_unlocked(type, bases, best_base, gen); } } while (res == 1); END_TYPE_LOCK(); + Py_DECREF(bases); return res; } From 8ca6ee8b09a84fb1ff6908769b4350a2fd69a777 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 22:18:44 -0700 Subject: [PATCH 6/8] Invalidate type version tags before dropping queue references Required since a dropped reference could run type_dealloc(), which takes TYPE_LOCK. Move the final slot_update_clear() after the version tags are invalidated. --- Objects/typeobject.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 10e6c94e3774ab3..94f2b739c9b75f1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1975,11 +1975,11 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, add to all new_bases */ remove_all_subclasses(type, old_bases); res = add_all_subclasses(type, new_bases); + /* This also clears the VALID_VERSION flag of 'type' and all its + subclasses. */ if (update_all_slots(type) < 0) { goto bail; } - /* Clear the VALID_VERSION flag of 'type' and all its subclasses. */ - _PyType_Modified_Unlocked(type); } else { res = 0; @@ -6732,15 +6732,16 @@ update_slot_after_setattr(PyTypeObject *type, PyObject *name) for (;;) { uint64_t gen = types_publish_gen(); res = update_slot(type, name, &queued_updates); - int done = 1; - if (res == 0 && !slot_update_is_empty(&queued_updates)) { - done = apply_type_slot_updates(&queued_updates, gen); + if (res < 0 || slot_update_is_empty(&queued_updates)) { + break; } - slot_update_clear(&queued_updates, &chunk); - if (res < 0 || done) { + if (apply_type_slot_updates(&queued_updates, gen)) { break; } // The queue went stale while TYPE_LOCK was released, build it again. + // Nothing was published, so it does not matter if dropping the + // references here runs type_dealloc() and releases TYPE_LOCK. + slot_update_clear(&queued_updates, &chunk); } if (res == 0) { // Invalidate again, after the slots are updated and while TYPE_LOCK @@ -6750,6 +6751,11 @@ update_slot_after_setattr(PyTypeObject *type, PyObject *name) ASSERT_TYPE_LOCK_HELD(); _PyType_Modified_Unlocked(type); } + // Drop the references only now. If one of them is the last reference + // to a queued subclass, type_dealloc() takes TYPE_LOCK again, which + // releases it until it is re-acquired. That must not happen between + // publishing the slots and invalidating the version tags above. + slot_update_clear(&queued_updates, &chunk); return res; #else update_slot(type, name, NULL); @@ -12208,7 +12214,8 @@ fixup_slot_dispatchers(PyTypeObject *type) #ifdef Py_GIL_DISABLED -// Called when __bases__ is re-assigned. +// Called when __bases__ is re-assigned. Also clears the VALID_VERSION +// flag of 'type' and all its subclasses. static int update_all_slots(PyTypeObject* type) { @@ -12226,22 +12233,28 @@ update_all_slots(PyTypeObject* type) return -1; } } - int done = 1; - if (!slot_update_is_empty(&queued_updates)) { - done = apply_type_slot_updates(&queued_updates, gen); + if (slot_update_is_empty(&queued_updates)) { + break; } - slot_update_clear(&queued_updates, NULL); - if (done) { + if (apply_type_slot_updates(&queued_updates, gen)) { break; } // The queue went stale while TYPE_LOCK was released, build it again. + // Nothing was published, so it does not matter if dropping the + // references here runs type_dealloc() and releases TYPE_LOCK. + slot_update_clear(&queued_updates, NULL); } + // Invalidate the version tags while TYPE_LOCK is still held, before + // dropping the references: see update_slot_after_setattr(). + _PyType_Modified_Unlocked(type); + slot_update_clear(&queued_updates, NULL); return 0; } #else -// Called when __bases__ is re-assigned. +// Called when __bases__ is re-assigned. Also clears the VALID_VERSION +// flag of 'type' and all its subclasses. static int update_all_slots(PyTypeObject* type) { @@ -12250,6 +12263,7 @@ update_all_slots(PyTypeObject* type) /* update_slot returns int but can't actually fail in this case*/ update_slot(type, p->name_strobj, NULL); } + _PyType_Modified_Unlocked(type); return 0; } From 5f92ab052839f4b380d479d8088aaf63ec902a0e Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 21:25:17 -0700 Subject: [PATCH 7/8] Cleanup try_set_tp_bases() and try_set_tp_mro() Both helpers now return 1 when stored, 0 when the expected value was replaced and -1 when the publish generation changed. The check_gen flag is replaced by the TYPES_PUBLISH_GEN_NONE sentinel. --- Objects/typeobject.c | 57 ++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 94f2b739c9b75f1..af41f9c29637e89 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -143,13 +143,17 @@ types_publish_gen_inc(void) _Py_atomic_add_uint64(&interp->types.publish_gen, 1); } +// Passed as 'gen' to the try_set_tp_*() helpers to store without checking +// the publish generation. +#define TYPES_PUBLISH_GEN_NONE UINT64_MAX + // Return true if the prepared state that was snapshotted at 'gen' is stale. // Must be called with the world stopped. static inline bool types_publish_is_stale(uint64_t gen) { assert(types_world_is_stopped()); - return types_publish_gen() != gen; + return gen != TYPES_PUBLISH_GEN_NONE && types_publish_gen() != gen; } #else @@ -165,6 +169,7 @@ types_publish_is_stale(uint64_t gen) #define types_world_is_stopped() 1 #define types_stop_world() #define types_start_world() +#define TYPES_PUBLISH_GEN_NONE 0 #define types_publish_gen() 0 #define types_publish_gen_inc() #define types_publish_is_stale(gen) 0 @@ -593,28 +598,35 @@ set_tp_bases(PyTypeObject *self, PyObject *bases) self->tp_bases = bases; } -// Replace tp_bases and tp_base of a type. Returns 1 on success, -// and 0 otherwise. +// Replace tp_bases and tp_base of a type with the world stopped. Returns 1 +// on success, 0 if tp_bases is no longer 'expected' (NULL matches anything) +// and -1 if the publish generation is no longer 'gen' (pass +// TYPES_PUBLISH_GEN_NONE to skip that check). In the last two cases the +// caller still owns 'bases' and 'base'. static int try_set_tp_bases(PyTypeObject *self, PyObject *expected, - PyObject *bases, PyTypeObject *base, uint64_t gen, bool check_gen) + PyObject *bases, PyTypeObject *base, uint64_t gen) { ASSERT_TYPE_LOCK_HELD(); assert(PyTuple_Check(bases)); assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); - int stored = 0; + int res; types_stop_world(); - if ((expected == NULL || self->tp_bases == expected) - && (!check_gen || !types_publish_is_stale(gen))) - { + if (expected != NULL && self->tp_bases != expected) { + res = 0; + } + else if (types_publish_is_stale(gen)) { + res = -1; + } + else { self->tp_bases = bases; self->tp_base = base; types_publish_gen_inc(); - stored = 1; + res = 1; } types_start_world(); ASSERT_TYPE_LOCK_HELD(); - return stored; + return res; } static inline void @@ -663,12 +675,13 @@ set_tp_mro(PyTypeObject *self, PyObject *mro) self->tp_mro = mro; } -// Replace tp_mro with the world stopped. Returns 1 on success, 0 if tp_mro -// is no longer "expected", and -1 if the generation changed. In the last two -// cases the caller still owns 'mro'. +// Replace tp_mro of a type with the world stopped. Returns 1 on success, 0 +// if tp_mro is no longer 'expected' and -1 if the publish generation is no +// longer 'gen' (pass TYPES_PUBLISH_GEN_NONE to skip that check). In the +// last two cases the caller still owns 'mro'. static int try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, - uint64_t gen, bool check_gen) + uint64_t gen) { ASSERT_TYPE_LOCK_HELD(); assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); @@ -678,7 +691,7 @@ try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, if (self->tp_mro != expected) { res = 0; } - else if (check_gen && types_publish_is_stale(gen)) { + else if (types_publish_is_stale(gen)) { res = -1; } else { @@ -1811,7 +1824,7 @@ mro_hierarchy_for_complete_type(PyTypeObject *type, PyObject *temp) if (res < 0) { // Roll back, unless tp_mro has already been replaced again. - if (try_set_tp_mro(type, new_mro, old_mro, 0, false) == 1) { + if (try_set_tp_mro(type, new_mro, old_mro, TYPES_PUBLISH_GEN_NONE) == 1) { Py_DECREF(new_mro); } else { @@ -1945,8 +1958,8 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, assert(old_bases != NULL); PyTypeObject *old_base = type->tp_base; - if (!try_set_tp_bases(type, NULL, Py_NewRef(new_bases), - (PyTypeObject *)Py_NewRef(best_base), gen, true)) + if (try_set_tp_bases(type, NULL, Py_NewRef(new_bases), + (PyTypeObject *)Py_NewRef(best_base), gen) != 1) { Py_DECREF(new_bases); Py_DECREF(best_base); @@ -2002,7 +2015,7 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, "", 2, 3, &cls, &new_mro, &old_mro); /* Do not rollback if cls has a newer version of MRO. */ old_mro = Py_XNewRef(old_mro); - if (try_set_tp_mro(cls, new_mro, old_mro, 0, false) == 1) { + if (try_set_tp_mro(cls, new_mro, old_mro, TYPES_PUBLISH_GEN_NONE) == 1) { Py_DECREF(new_mro); } else { @@ -2013,7 +2026,9 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, bail: /* Restore the old bases, unless tp_bases has been replaced again. */ - if (try_set_tp_bases(type, new_bases, old_bases, old_base, 0, false)) { + if (try_set_tp_bases(type, new_bases, old_bases, old_base, + TYPES_PUBLISH_GEN_NONE) == 1) + { Py_DECREF(new_bases); Py_DECREF(best_base); } @@ -3715,7 +3730,7 @@ mro_internal(PyTypeObject *type, int initial, PyObject **p_old_mro) // released while mro_invoke() ran or while waiting to stop the // world, so check for reentrance and for concurrent mutation of // the hierarchy inside the stop-the-world. - res = try_set_tp_mro(type, old_mro, new_mro, gen, true); + res = try_set_tp_mro(type, old_mro, new_mro, gen); } if (res == 1) { break; From 7917d6aa5d68974a626ca95064c780a4876ee1c3 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 21:25:17 -0700 Subject: [PATCH 8/8] Correct the comment on type_validate_new_bases() The checks are plain C and do not run Python code. --- Objects/typeobject.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index af41f9c29637e89..b4abcd09405cac7 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -1902,10 +1902,7 @@ type_check_new_bases(PyTypeObject *type, PyObject *new_bases) } // Checks on the new __bases__ value that depend on the current state of the -// type hierarchy (inheritance cycles, layout compatibility). These can run -// Python code and release TYPE_LOCK, so they are validated optimistically: -// the caller snapshots the publish generation before calling this and -// the new bases are only stored if the generation is unchanged. +// type hierarchy (inheritance cycles, layout compatibility). static int type_validate_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **best_base) {