From 672c9a16272733ec2c6d050ed78a58b0b91efbb1 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 20 Oct 2026 13:09:58 -0700 Subject: [PATCH 01/12] 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 a9ad61f458944a..2236b158803bae 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 a55c4815a03837..3717eb6b820287 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 00000000000000..f189c7c563ca58 --- /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 5e772e5608d4c6..3a3d11bea80366 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 02/12] 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 3a3d11bea80366..b7a5c36d98b96b 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 03/12] 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 ecc42db7b44f3a..670d66e1d0ebb2 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 3717eb6b820287..a19638b083be5d 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 479297a996c250..cac040efdd2b92 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 04/12] 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 b7a5c36d98b96b..d965438127b194 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 05/12] 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 d965438127b194..10e6c94e3774ab 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 06/12] 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 10e6c94e3774ab..94f2b739c9b75f 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 07/12] 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 94f2b739c9b75f..af41f9c29637e8 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 08/12] 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 af41f9c29637e8..b4abcd09405cac 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) { From 2aca6d5aeba49f6902b77eff04412e19e83735b0 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Wed, 21 Oct 2026 10:12:10 -0700 Subject: [PATCH 09/12] Minor comments cleanup, reduce. --- Objects/typeobject.c | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b4abcd09405cac..9f8b10f0119044 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -148,7 +148,6 @@ types_publish_gen_inc(void) #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) { @@ -1741,10 +1740,8 @@ type_set_abstractmethods(PyObject *tp, PyObject *value, void *Py_UNUSED(closure) else type_clear_flags(type, Py_TPFLAGS_IS_ABSTRACT); types_start_world(); - // 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. + // Invalidate the version tag after update, while TYPE_LOCK is + // still held. ASSERT_TYPE_LOCK_HELD(); _PyType_Modified_Unlocked(type); END_TYPE_LOCK(); @@ -3872,12 +3869,8 @@ solid_base(PyTypeObject *type) // updates to perform but don't actually update the type structures. After we // 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. +// safe. We use publish_gen to determine if the queued updates have been +// potentially invalidated, we re-build it in that case. typedef struct { PyTypeObject *type; @@ -3992,18 +3985,6 @@ apply_slot_updates(slot_update_t *updates) // 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) { @@ -6517,11 +6498,8 @@ _PyType_SetFlagsRecursive(PyTypeObject *self, unsigned long mask, unsigned long types_stop_world(); set_flags_recursive(self, mask, flags); types_start_world(); - // 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. + // Invalidate the version tags after update, while TYPE_LOCK is + // still held. ASSERT_TYPE_LOCK_HELD(); if (modified) { _PyType_Modified_Unlocked(self); From 8b8a87c2a32d0f94af541b97ee83b7c3c44f5a5e Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 23 Oct 2026 05:21:17 -0700 Subject: [PATCH 10/12] Avoid looping if mro() assigns a slot. mro_internal() now calls mro() once and stores the result with TYPES_PUBLISH_GEN_NONE. The only check inside the stop-the-world is the tp_mro identity test that main already had. --- Lib/test/test_free_threading/test_type.py | 23 +++++++++ Objects/typeobject.c | 61 +++++++++++------------ 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/Lib/test/test_free_threading/test_type.py b/Lib/test/test_free_threading/test_type.py index a19638b083be5d..010882c10d5ec5 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -540,6 +540,29 @@ def __eq__(self, other): self.assertIsNone(base_ref()) self.assertIsNone(sub_ref()) + def test_reentrant_mro_always_publishes(self): + # A custom mro() that updates a slot on every call must not make + # the __bases__ assignment retry forever. + class Other: + pass + + class Meta(type): + def mro(cls): + Other.__repr__ = lambda self: "x" + return type.mro(cls) + + class A: + pass + + class B: + pass + + class C(A, metaclass=Meta): + pass + + C.__bases__ = (B,) + self.assertEqual(C.__mro__, (C, B, object)) + 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 diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9f8b10f0119044..3eedac4aa7c76a 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3700,43 +3700,38 @@ mro_internal(PyTypeObject *type, int initial, PyObject **p_old_mro) 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)); - 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; - } + 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 { - // 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; + res = 0; } + } + else { + // Store with the world stopped. TYPE_LOCK may have been released + // while mro_invoke() ran, so check for reentrance inside the + // stop-the-world. The publish generation is deliberately not + // checked: mro() can itself update types (assign a slot, for + // example) and the result is stored anyway, as on the default + // build. + res = try_set_tp_mro(type, old_mro, new_mro, TYPES_PUBLISH_GEN_NONE); + assert(res != -1); + } + if (res == 0) { + /* reentrance: tp_mro was replaced by someone else */ Py_DECREF(new_mro); - 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); + Py_XDECREF(old_mro); + return 0; } /* Drop our temporary reference. The reference previously owned by the type is now owned by this function and transferred below. */ From 04afc4ddd4e27295f47ca67ca666c0f19af63b0c Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Sat, 24 Oct 2026 02:06:18 -0700 Subject: [PATCH 11/12] Copy str subclass keys of the class namespace to exact str. type_new() now copies str subclass keys of the class namespace to exact str, as type_setattro() already does for assigned names. Lookups of a slot name in a type dict then cannot call a user __eq__ through a str subclass key. Non-str keys are still allowed (with a RuntimeWarning) and can still run __eq__; the type_update_dict() comment is corrected to say so. Remove the free-threading tests that relied on str subclass keys in the type dict. --- Lib/test/test_descr.py | 25 ++++++++ Lib/test/test_free_threading/test_type.py | 71 ---------------------- Objects/typeobject.c | 72 +++++++++++++++++++++-- 3 files changed, 93 insertions(+), 75 deletions(-) diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index a6ebda5bf4e5df..3de7f2975fe848 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -5080,6 +5080,31 @@ def __new__(mcls, name, bases, ns): with self.assertWarnsRegex(RuntimeWarning, 'MyClass'): MyClass = meta('MyClass', (), {}) + def test_str_subclass_namespace_keys(self): + # Keys of the class namespace that are str subclasses are copied + # to exact str in the type's __dict__, like type.__setattr__ does. + class Key(str): + def __hash__(self): + # Collide with the slot name assigned below. + return hash('__hash__') + def __eq__(self, other): + # Must not be called by type dict lookups. + raise AssertionError('Key.__eq__ called') + + ns = {Key('other'): 1, '__repr__': lambda self: 'r', 'c': 2} + C = type('C', (), ns) + for key in C.__dict__: + self.assertIs(type(key), str) + self.assertEqual([str(k) for k in ns], list(C.__dict__)[:3]) + self.assertEqual(repr(C()), 'r') + self.assertEqual(C.other, 1) + # Lookups and slot updates that hash-collide with 'other' do not + # call Key.__eq__. + C.__hash__ = None + with self.assertRaises(TypeError): + hash(C()) + del C.__hash__ + def test_proxy_call(self): class FakeStr: __class__ = str diff --git a/Lib/test/test_free_threading/test_type.py b/Lib/test/test_free_threading/test_type.py index 010882c10d5ec5..6e9a1169a1ed39 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -1,7 +1,6 @@ import gc import threading import unittest -import weakref from concurrent.futures import ThreadPoolExecutor from threading import Barrier, Thread @@ -470,76 +469,6 @@ def setter(): 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 - - class S(str): - __hash__ = str.__hash__ - def __eq__(self, other): - _testinternalcapi.stop_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_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 - - class Key(str): - def __hash__(self): - return hash("__hash__") - - def __eq__(self, other): - nonlocal armed - if armed and other == "__hash__": - armed = False - Base.__hash__ = object.__hash__ - return str.__eq__(self, other) - - # 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_reentrant_mro_always_publishes(self): # A custom mro() that updates a slot on every call must not make # the __bases__ assignment retry forever. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 3eedac4aa7c76a..a027580e54e995 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4978,10 +4978,72 @@ type_new_get_slots(type_new_ctx *ctx, PyObject *dict) } +/* Copy the class namespace. Keys that are str subclasses are replaced by + exact str copies so that the type dict only holds exact str keys (see + type_setattro() which does the same for assigned names). Lookups in a + dict with exact str keys cannot run user __eq__ methods, which the slot + update code relies on. Non-str keys are left alone; type_new_impl() + warns about them. */ +static PyObject * +type_new_copy_dict(PyObject *orig_dict) +{ + PyObject *dict = _PyDict_CopyAsDict(orig_dict); + if (dict == NULL) { + return NULL; + } + if (((PyDictObject *)dict)->ma_keys->dk_kind != DICT_KEYS_GENERAL) { + // Only exact str keys are possible, nothing to do. + return dict; + } + Py_ssize_t pos = 0; + PyObject *key, *value; + int rebuild = 0; + while (PyDict_Next(dict, &pos, &key, &value)) { + if (PyUnicode_Check(key) && !PyUnicode_CheckExact(key)) { + rebuild = 1; + break; + } + } + if (!rebuild) { + return dict; + } + // Rebuild rather than delete/insert so that the key order is kept. + PyObject *newdict = PyDict_New(); + if (newdict == NULL) { + Py_DECREF(dict); + return NULL; + } + pos = 0; + while (PyDict_Next(dict, &pos, &key, &value)) { + PyObject *newkey; + if (PyUnicode_Check(key) && !PyUnicode_CheckExact(key)) { + newkey = _PyUnicode_Copy(key); + if (newkey == NULL) { + goto error; + } + } + else { + newkey = Py_NewRef(key); + } + int err = PyDict_SetItem(newdict, newkey, value); + Py_DECREF(newkey); + if (err < 0) { + goto error; + } + } + Py_DECREF(dict); + return newdict; + +error: + Py_DECREF(dict); + Py_DECREF(newdict); + return NULL; +} + static PyTypeObject* type_new_init(type_new_ctx *ctx) { - PyObject *dict = _PyDict_CopyAsDict(ctx->orig_dict); + PyObject *dict = type_new_copy_dict(ctx->orig_dict); if (dict == NULL) { goto error; } @@ -6673,9 +6735,11 @@ type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name, // We don't want any re-entrancy between when we update the dict // and call _PyType_Modified_Unlocked, including running the destructor // of the current value as it can observe the cache in an inconsistent - // state. Because we have an exact unicode and our dict has exact - // unicodes we know that this will all complete without releasing - // the locks. + // state. The name is an exact unicode (see type_setattro()) and + // str subclass keys are copied to exact str by type_new_copy_dict() + // and type_setattro(). The type dict can still hold non-str keys + // (type_new_impl() only warns about them) and comparing with one of + // those can run user code that releases the locks. if (_PyDict_GetItemRef_Unicode_LockHeld(dict, name, old_value) < 0) { return -1; } From 8770238b278756863432e01152ad050d677ceb11 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Sat, 24 Oct 2026 02:48:33 -0700 Subject: [PATCH 12/12] Do type slot and __bases__ updates in a single stop-the-world. Remove the publish generation counter and the retry loops. Instead of preparing updates under TYPE_LOCK and publishing them with the world stopped (and rebuilding them if they went stale while the lock was released), do the work inside the stop-the-world: - Slot updates after type_setattro() of a slot name and after __bases__ assignment look up the descriptors and store the slots directly with the world stopped. The slot update queue is removed. The lookups use borrowed references and skip dict keys that are not exact str (_PyDict_GetItem_ExactUnicode()), so no user code runs and nothing is deallocated. - __bases__ assignment validates the new bases (inheritance cycle, best base, layout compatibility) and stores tp_bases and tp_base in the same stop-the-world. The checks return an error code and the exception is raised after the world is started. PyType_Ready() of a new base and the Py_TPFLAGS_BASETYPE check move to type_check_new_bases(). ht_slots are compared by string content. In the free-threaded build, slot computation for new types (fixup_slot_dispatchers() and overrides_hash()) also skips non-str keys, using the thread-safe _PyDict_GetItemStackRef_ExactUnicode(), so that slots do not depend on when they were computed. --- Include/internal/pycore_dict.h | 4 + Include/internal/pycore_interp_structs.h | 6 - Lib/test/test_free_threading/test_type.py | 52 +- Objects/dictobject.c | 75 +++ Objects/typeobject.c | 772 ++++++++++------------ 5 files changed, 452 insertions(+), 457 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 0257aaaf4534d5..a30a79f31e6d96 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -170,6 +170,10 @@ PyAPI_FUNC(int) _PyDict_SetItem_KnownHash_LockHeld(PyDictObject *mp, PyObject *k PyAPI_FUNC(int) _PyDict_GetItemRef_KnownHash_LockHeld(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result); PyAPI_FUNC(int) _PyDict_GetItemRef_KnownHash(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result); extern int _PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result); +extern PyObject *_PyDict_GetItem_ExactUnicode(PyDictObject *mp, PyObject *key); +extern int _PyDict_GetItemStackRef_ExactUnicode(PyDictObject *mp, + PyObject *key, + _PyStackRef *value_addr); PyAPI_FUNC(int) _PyObjectDict_SetItem(PyTypeObject *tp, PyObject *obj, PyObject **dictptr, PyObject *name, PyObject *value); extern int _PyDict_Pop_KnownHash( diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 2236b158803bae..a9ad61f458944a 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -588,12 +588,6 @@ 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 6e9a1169a1ed39..e87a773c43bb80 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -23,8 +23,8 @@ class A: def world_stoppers(): """Finite workloads that contend for the stop-the-world mutex. - Waiting for the mutex releases the type lock, allowing queued type - updates to go stale. Use a short pause rather than full collections + Waiting for the mutex releases the type lock, so that other threads + can change types in the meantime. Use a short pause rather than full collections to prolong the contention without depending on the size of the heap. """ def stopper(): @@ -344,9 +344,8 @@ def wrapper(): reader.join() def test_setattr_many_subclasses(self): - # gh-155978: Updating a special method queues a slot update for every - # affected subclass. Keep enough subclasses alive to require - # heap-allocated queue chunks in addition to the stack chunk. + # gh-155978: Updating a special method updates the slot of every + # affected subclass. class Base: pass @@ -363,8 +362,7 @@ 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. + # The slot update must not keep references to the types. del subclasses gc.collect() self.assertEqual(Base.__subclasses__(), []) @@ -492,11 +490,45 @@ class C(A, metaclass=Meta): C.__bases__ = (B,) self.assertEqual(C.__mro__, (C, B, object)) + def test_slot_update_skips_non_str_keys(self): + # Slots are computed (with the world stopped, for an update) without + # comparing the slot name with non-str keys of the type dicts, which + # could run user code. Here, that code would also update a slot + # every time. + calls = 0 + + class Other: + pass + + class Key: + def __hash__(self): + return hash("__hash__") + + def __eq__(self, other): + nonlocal calls + calls += 1 + Other.__repr__ = lambda self: "x" + return False + + class Base: + pass + + with self.assertWarns(RuntimeWarning): + Sub = type("Sub", (Base,), {Key(): None}) + self.assertEqual(calls, 0) + + Base.__hash__ = None + self.assertEqual(calls, 0) + with self.assertRaises(TypeError): + hash(Sub()) + del Base.__hash__ + hash(Sub()) + 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__. + # base, while others stop the world. The type lock is released while + # waiting for the stop-the-world. At the end, the MRO must agree + # with __bases__. N = 50 class A: diff --git a/Objects/dictobject.c b/Objects/dictobject.c index bff543d9fdede4..55b004162888df 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2631,6 +2631,81 @@ PyDict_GetItemRef(PyObject *op, PyObject *key, PyObject **result) return _PyDict_GetItemRef_KnownHash((PyDictObject *)op, key, hash, result); } +static inline int +compare_generic_exact_unicode(PyDictObject *mp, PyDictKeysObject *dk, + void *ep0, Py_ssize_t ix, PyObject *key, + Py_hash_t hash) +{ + PyDictKeyEntry *ep = &((PyDictKeyEntry *)ep0)[ix]; + assert(ep->me_key != NULL); + if (ep->me_key == key) { + return 1; + } + return (ep->me_hash == hash && PyUnicode_CheckExact(ep->me_key) && + unicode_eq(ep->me_key, key)); +} + +// Look up an exact str key, without comparing it to keys that are not exact +// str: those are skipped, so no user code can run. Returns a borrowed +// reference, or NULL (without an exception) if the key is not found. The +// caller must hold the dict lock or have the world stopped. +PyObject * +_PyDict_GetItem_ExactUnicode(PyDictObject *mp, PyObject *key) +{ + assert(PyUnicode_CheckExact(key)); + Py_hash_t hash = hash_unicode_key(key); + PyDictKeysObject *dk = mp->ma_keys; + Py_ssize_t ix; + if (dk->dk_kind == DICT_KEYS_GENERAL) { + ix = do_lookup(mp, dk, key, hash, compare_generic_exact_unicode); + } + else { + ix = unicodekeys_lookup_unicode(dk, key, hash); + } + if (ix < 0) { + return NULL; + } + if (dk->dk_kind == DICT_KEYS_SPLIT) { + return mp->ma_values->values[ix]; + } + if (dk->dk_kind == DICT_KEYS_GENERAL) { + return DK_ENTRIES(dk)[ix].me_value; + } + return DK_UNICODE_ENTRIES(dk)[ix].me_value; +} + +// Like _PyDict_GetItem_ExactUnicode() but safe to call without holding the +// dict lock. Stores a new reference (or NULL) in *value_addr. Returns 1 if +// the key was found and 0 if not. +int +_PyDict_GetItemStackRef_ExactUnicode(PyDictObject *mp, PyObject *key, + _PyStackRef *value_addr) +{ + assert(PyUnicode_CheckExact(key)); + PyObject *value; +#ifdef Py_GIL_DISABLED + ensure_shared_on_read(mp); + PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys); + if (dk->dk_kind == DICT_KEYS_UNICODE) { + // All keys are exact str, so the lock-free lookup cannot run user + // code. + Py_hash_t hash = hash_unicode_key(key); + Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, value_addr); + if (ix != DKIX_KEY_CHANGED) { + return !PyStackRef_IsNull(*value_addr); + } + } + Py_BEGIN_CRITICAL_SECTION(mp); + value = _PyDict_GetItem_ExactUnicode(mp, key); + *value_addr = value ? PyStackRef_FromPyObjectNew(value) : PyStackRef_NULL; + Py_END_CRITICAL_SECTION(); +#else + value = _PyDict_GetItem_ExactUnicode(mp, key); + *value_addr = value ? PyStackRef_FromPyObjectNew(value) : PyStackRef_NULL; +#endif + return value != NULL; +} + int _PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a027580e54e995..f2485d3878dfa5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -70,7 +70,6 @@ class object "PyObject *" "&PyBaseObject_Type" #define END_TYPE_DICT_LOCK() Py_END_CRITICAL_SECTION2() -#if !defined(NDEBUG) || defined(Py_DEBUG) // Return true if the world is currently stopped. static bool types_world_is_stopped(void) @@ -78,7 +77,6 @@ types_world_is_stopped(void) PyInterpreterState *interp = _PyInterpreterState_GET(); return interp->stoptheworld.world_stopped; } -#endif // Checks that the type has not yet been revealed (exposed) to other // threads. The _Py_TYPE_REVEALED_FLAG flag is set by type_new() and @@ -124,37 +122,6 @@ types_start_world(void) assert(!types_world_is_stopped()); } -// 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) -{ - PyInterpreterState *interp = _PyInterpreterState_GET(); - return _Py_atomic_load_uint64_relaxed(&interp->types.publish_gen); -} - -static inline void -types_publish_gen_inc(void) -{ - PyInterpreterState *interp = _PyInterpreterState_GET(); - _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. -static inline bool -types_publish_is_stale(uint64_t gen) -{ - assert(types_world_is_stopped()); - return gen != TYPES_PUBLISH_GEN_NONE && types_publish_gen() != gen; -} - #else #define BEGIN_TYPE_LOCK() @@ -168,10 +135,6 @@ 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 #endif @@ -598,29 +561,20 @@ set_tp_bases(PyTypeObject *self, PyObject *bases) } // 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'. +// on success and 0 if tp_bases is no longer 'expected'. In the latter case +// the caller still owns 'bases' and 'base'. static int try_set_tp_bases(PyTypeObject *self, PyObject *expected, - PyObject *bases, PyTypeObject *base, uint64_t gen) + PyObject *bases, PyTypeObject *base) { ASSERT_TYPE_LOCK_HELD(); assert(PyTuple_Check(bases)); assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); - int res; + int res = 0; types_stop_world(); - if (expected != NULL && self->tp_bases != expected) { - res = 0; - } - else if (types_publish_is_stale(gen)) { - res = -1; - } - else { + if (self->tp_bases == expected) { self->tp_bases = bases; self->tp_base = base; - types_publish_gen_inc(); res = 1; } types_start_world(); @@ -674,31 +628,22 @@ set_tp_mro(PyTypeObject *self, PyObject *mro) self->tp_mro = 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'. +// Replace tp_mro of a type with the world stopped. Returns 1 on success and +// 0 if tp_mro is no longer 'expected'. In the latter case the caller still +// owns 'mro'. static int -try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro, - uint64_t gen) +try_set_tp_mro(PyTypeObject *self, PyObject *expected, PyObject *mro) { ASSERT_TYPE_LOCK_HELD(); assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); assert(mro == NULL || PyTuple_CheckExact(mro)); - int res; + int res = 0; types_stop_world(); - if (self->tp_mro != expected) { - res = 0; - } - else if (types_publish_is_stale(gen)) { - res = -1; - } - else { + if (self->tp_mro == expected) { if (mro != NULL) { PyUnstable_Object_EnableDeferredRefcount(mro); } self->tp_mro = mro; - types_publish_gen_inc(); res = 1; } types_start_world(); @@ -1774,12 +1719,14 @@ type_get_mro(PyObject *tp, void *Py_UNUSED(closure)) static PyTypeObject *find_best_base(PyObject *); static int mro_internal(PyTypeObject *, int, PyObject **); static int type_is_subtype_base_chain(PyTypeObject *, PyTypeObject *); -static int compatible_for_assignment(PyTypeObject *, PyTypeObject *, const char *, int); +static int compatible_layout(PyTypeObject *, PyTypeObject *, int); +static void raise_layout_differs(const char *, PyTypeObject *, PyTypeObject *); +static int update_best_base(PyTypeObject *, PyTypeObject **, PyTypeObject **); static int add_subclass(PyTypeObject*, PyTypeObject*); static int add_all_subclasses(PyTypeObject *type, PyObject *bases); static void remove_subclass(PyTypeObject *, PyTypeObject *); static void remove_all_subclasses(PyTypeObject *type, PyObject *bases); -static int update_all_slots(PyTypeObject *); +static void update_all_slots(PyTypeObject *); typedef int (*update_callback)(PyTypeObject *, void *); static int update_subclasses(PyTypeObject *type, PyObject *attr_name, @@ -1821,7 +1768,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, TYPES_PUBLISH_GEN_NONE) == 1) { + if (try_set_tp_mro(type, new_mro, old_mro) == 1) { Py_DECREF(new_mro); } else { @@ -1894,16 +1841,39 @@ type_check_new_bases(PyTypeObject *type, PyObject *new_bases) type->tp_name, Py_TYPE(ob)->tp_name); return -1; } + PyTypeObject *base = (PyTypeObject *)ob; + if (!_PyType_IsReady(base)) { + if (PyType_Ready(base) < 0) { + return -1; + } + } + if (!_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) { + PyErr_Format(PyExc_TypeError, + "type '%.100s' is not an acceptable base type", + base->tp_name); + return -1; + } } return 0; } +typedef enum { + NEW_BASES_OK, + NEW_BASES_CYCLE, + NEW_BASES_LAYOUT_CONFLICT, + NEW_BASES_LAYOUT_DIFFERS, +} new_bases_check_t; + // Checks on the new __bases__ value that depend on the current state of the -// type hierarchy (inheritance cycles, layout compatibility). -static int -type_validate_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **best_base) +// type hierarchy (inheritance cycles, layout compatibility). This is called +// with the world stopped and does not raise; see raise_new_bases_error(). +// On success, *best_base is set to the new tp_base (a borrowed reference +// from new_bases). +static new_bases_check_t +type_validate_new_bases(PyTypeObject *type, PyObject *new_bases, + PyTypeObject **best_base) { - ASSERT_TYPE_LOCK_HELD(); + assert(types_world_is_stopped()); 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); @@ -1921,43 +1891,80 @@ type_validate_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject ** (lookup_tp_mro(base) != NULL && type_is_subtype_base_chain(base, type))) { - PyErr_SetString(PyExc_TypeError, - "a __bases__ item causes an inheritance cycle"); - return -1; + return NEW_BASES_CYCLE; } } - // Compute the new MRO and the new base class - *best_base = find_best_base(new_bases); - if (*best_base == NULL) - return -1; + // Compute the new base class. The bases are known to be ready and + // acceptable as base types (see type_check_new_bases()). + PyTypeObject *winner = NULL; + *best_base = NULL; + for (Py_ssize_t i = 0; i < n; i++) { + PyTypeObject *base = (PyTypeObject *)PyTuple_GET_ITEM(new_bases, i); + if (update_best_base(base, best_base, &winner) < 0) { + return NEW_BASES_LAYOUT_CONFLICT; + } + } - if (!compatible_for_assignment(type, *best_base, "__bases__", 0)) { - return -1; + if (!compatible_layout(type, *best_base, 0)) { + return NEW_BASES_LAYOUT_DIFFERS; } - return 0; + return NEW_BASES_OK; } -// 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). +// Raise the error for a failed type_validate_new_bases() check. 'old_base' +// is the tp_base of the type when the check was done. +static void +raise_new_bases_error(new_bases_check_t check, PyTypeObject *best_base, + PyTypeObject *old_base) +{ + switch (check) { + case NEW_BASES_CYCLE: + PyErr_SetString(PyExc_TypeError, + "a __bases__ item causes an inheritance cycle"); + break; + case NEW_BASES_LAYOUT_CONFLICT: + PyErr_SetString(PyExc_TypeError, + "multiple bases have instance lay-out conflict"); + break; + case NEW_BASES_LAYOUT_DIFFERS: + raise_layout_differs("__bases__", best_base, old_base); + break; + default: + Py_UNREACHABLE(); + } +} + +// Returns 0 on success and -1 on error. static int -type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, - PyTypeObject *best_base, uint64_t gen) +type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases) { ASSERT_TYPE_LOCK_HELD(); + // Validate and store the new bases with the world stopped, so that the + // hierarchy cannot change in between. Py_ssize_t n; + PyTypeObject *best_base; + types_stop_world(); PyObject *old_bases = lookup_tp_bases(type); 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) != 1) - { - Py_DECREF(new_bases); - Py_DECREF(best_base); - return 1; + new_bases_check_t check = type_validate_new_bases(type, new_bases, + &best_base); + if (check == NEW_BASES_OK) { + type->tp_bases = Py_NewRef(new_bases); + type->tp_base = (PyTypeObject *)Py_NewRef(best_base); + } + else { + // Keep old_base alive for the error message. + Py_INCREF(old_base); + } + types_start_world(); + if (check != NEW_BASES_OK) { + raise_new_bases_error(check, best_base, old_base); + Py_DECREF(old_base); + return -1; } PyObject *temp = PyList_New(0); @@ -1984,9 +1991,7 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_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; - } + update_all_slots(type); } else { res = 0; @@ -2009,7 +2014,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, TYPES_PUBLISH_GEN_NONE) == 1) { + if (try_set_tp_mro(cls, new_mro, old_mro) == 1) { Py_DECREF(new_mro); } else { @@ -2020,8 +2025,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, - TYPES_PUBLISH_GEN_NONE) == 1) + if (try_set_tp_bases(type, new_bases, old_bases, old_base) == 1) { Py_DECREF(new_bases); Py_DECREF(best_base); @@ -2039,7 +2043,6 @@ static int type_set_bases(PyObject *tp, PyObject *new_bases, void *Py_UNUSED(closure)) { PyTypeObject *type = PyTypeObject_CAST(tp); - PyTypeObject *best_base; int res; if (type_check_new_bases(type, new_bases) < 0) { return -1; @@ -2056,13 +2059,7 @@ type_set_bases(PyObject *tp, PyObject *new_bases, void *Py_UNUSED(closure)) 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, bases, &best_base); - if (res == 0) { - res = type_set_bases_unlocked(type, bases, best_base, gen); - } - } while (res == 1); + res = type_set_bases_unlocked(type, bases); END_TYPE_LOCK(); Py_DECREF(bases); return res; @@ -3720,12 +3717,10 @@ mro_internal(PyTypeObject *type, int initial, PyObject **p_old_mro) else { // Store with the world stopped. TYPE_LOCK may have been released // while mro_invoke() ran, so check for reentrance inside the - // stop-the-world. The publish generation is deliberately not - // checked: mro() can itself update types (assign a slot, for - // example) and the result is stored anyway, as on the default + // stop-the-world. mro() can itself update types (assign a slot, + // for example) and the result is stored anyway, as on the default // build. - res = try_set_tp_mro(type, old_mro, new_mro, TYPES_PUBLISH_GEN_NONE); - assert(res != -1); + res = try_set_tp_mro(type, old_mro, new_mro); } if (res == 0) { /* reentrance: tp_mro was replaced by someone else */ @@ -3760,6 +3755,30 @@ mro_internal(PyTypeObject *type, int initial, PyObject **p_old_mro) return 1; } +/* Update the best base candidate '*base' (with solid base '*winner') for + the next base 'base_i'. Returns -1 if the instance layouts conflict. + Does not raise. */ +static int +update_best_base(PyTypeObject *base_i, PyTypeObject **base, + PyTypeObject **winner) +{ + PyTypeObject *candidate = solid_base(base_i); + if (*winner == NULL) { + *winner = candidate; + *base = base_i; + } + else if (PyType_IsSubtype(*winner, candidate)) + ; + else if (PyType_IsSubtype(candidate, *winner)) { + *winner = candidate; + *base = base_i; + } + else { + return -1; + } + return 0; +} + /* Calculate the best base amongst multiple base classes. This is the first one that's on the path to the "solid base". */ @@ -3767,7 +3786,7 @@ static PyTypeObject * find_best_base(PyObject *bases) { Py_ssize_t i, n; - PyTypeObject *base, *winner, *candidate; + PyTypeObject *base, *winner; assert(PyTuple_Check(bases)); n = PyTuple_GET_SIZE(bases); @@ -3794,18 +3813,7 @@ find_best_base(PyObject *bases) base_i->tp_name); return NULL; } - candidate = solid_base(base_i); - if (winner == NULL) { - winner = candidate; - base = base_i; - } - else if (PyType_IsSubtype(winner, candidate)) - ; - else if (PyType_IsSubtype(candidate, winner)) { - winner = candidate; - base = base_i; - } - else { + if (update_best_base(base_i, &base, &winner) < 0) { PyErr_SetString( PyExc_TypeError, "multiple bases have " @@ -3846,173 +3854,28 @@ solid_base(PyTypeObject *type) } } -#ifdef Py_GIL_DISABLED - -// The structures and functions below are used in the free-threaded build -// to safely make updates to type slots, on type_setattro() for a slot -// or when __bases__ is re-assigned. Since the slots are read without atomic -// operations and without locking, we can only safely update them while the -// world is stopped. However, with the world stopped, we are very limited on -// which APIs can be safely used. For example, calling _PyObject_HashDictKey() -// or _PyDict_GetItemRef_KnownHash() are not safe and can potentially cause -// deadlocks. Hashing can be re-entrant and _PyDict_GetItemRef_KnownHash can -// acquire a lock if the dictionary is not owned by the current thread, to -// mark it shared on reading. -// -// We do the slot updates in two steps. First, with TYPE_LOCK held, we lookup -// the descriptor for each slot, for each subclass. We build a queue of -// updates to perform but don't actually update the type structures. After we -// 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. We use publish_gen to determine if the queued updates have been -// potentially invalidated, we re-build it in that case. - -typedef struct { - PyTypeObject *type; - void **slot_ptr; - void *slot_value; -} slot_update_item_t; - -// The number of slot updates performed is based on the number of changed -// slots and the number of subclasses. It's possible there are many updates -// required if there are many subclasses (potentially an unbounded amount). -// Usually the number of slot updates is small, most often zero or one. When -// running the unit tests, we don't exceed 20. The chunk size is set to -// handle the common case with a single chunk and to not require too many -// chunk allocations if there are many subclasses. -#define SLOT_UPDATE_CHUNK_SIZE 30 - -typedef struct _slot_update { - struct _slot_update *prev; - Py_ssize_t n; - slot_update_item_t updates[SLOT_UPDATE_CHUNK_SIZE]; -} slot_update_chunk_t; - -// a queue of updates to be performed -typedef struct { - slot_update_chunk_t *head; -} slot_update_t; - -static slot_update_chunk_t * -slot_update_new_chunk(void) -{ - slot_update_chunk_t *chunk = PyMem_Malloc(sizeof(slot_update_chunk_t)); - if (chunk == NULL) { - PyErr_NoMemory(); - return NULL; - } - chunk->prev = NULL; - chunk->n = 0; - return chunk; -} - -static void -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; - 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 -queue_slot_update(slot_update_t *updates, PyTypeObject *type, - void **slot_ptr, void *slot_value) -{ - if (*slot_ptr == slot_value) { - return 0; // slot pointer not actually changed, don't queue update - } - if (updates->head == NULL || updates->head->n == SLOT_UPDATE_CHUNK_SIZE) { - slot_update_chunk_t *chunk = slot_update_new_chunk(); - if (chunk == NULL) { - return -1; // out-of-memory - } - chunk->prev = updates->head; - updates->head = chunk; - } - slot_update_item_t *item = &updates->head->updates[updates->head->n]; - item->type = (PyTypeObject *)Py_NewRef(type); - item->slot_ptr = slot_ptr; - item->slot_value = slot_value; - updates->head->n++; - assert(updates->head->n <= SLOT_UPDATE_CHUNK_SIZE); - 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) -{ - assert(types_world_is_stopped()); - slot_update_chunk_t *chunk = updates->head; - while (chunk != NULL) { - for (Py_ssize_t i = 0; i < chunk->n; i++) { - slot_update_item_t *item = &chunk->updates[i]; - *(item->slot_ptr) = item->slot_value; - if (item->slot_value == slot_tp_call) { - /* A generic __call__ is incompatible with vectorcall */ - type_clear_flags(item->type, Py_TPFLAGS_HAVE_VECTORCALL); - } - } - chunk = chunk->prev; - } -} - -// 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. -static int -apply_type_slot_updates(slot_update_t *updates, uint64_t gen) -{ - ASSERT_TYPE_LOCK_HELD(); - int applied = 0; - types_stop_world(); - if (!types_publish_is_stale(gen)) { - apply_slot_updates(updates); - types_publish_gen_inc(); - applied = 1; - } - types_start_world(); - ASSERT_TYPE_LOCK_HELD(); - return applied; -} - -#else - -// dummy definition, this parameter is only NULL in the default build -typedef void slot_update_t; - -#endif +// In the free-threaded build, type slots and tp_flags are read without +// atomics and without locking, so they can only be updated with the world +// stopped (or before the type is revealed to other threads). When a slot +// name is assigned with type_setattro() or __bases__ is re-assigned, the +// descriptor lookups for the type and its subclasses and the slot stores are +// all done within one stop-the-world. The new slots therefore agree with +// the type dicts as they are when the world is stopped, whatever happened +// while TYPE_LOCK was released waiting for it. With the world stopped, no +// other thread runs, so dicts and the subclass lists can be read without +// locking and with borrowed references. Nothing that can run user code, +// block or deallocate is allowed; see find_slot_name_in_mro() and +// recurse_down_subclasses(). /// data passed to update_slots_callback() typedef struct { - slot_update_t *queued_updates; pytype_slotdef **defs; } update_callback_data_t; static void object_dealloc(PyObject *); 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 int update_slot(PyTypeObject *, PyObject *); static void fixup_slot_dispatchers(PyTypeObject *); static int type_ready(PyTypeObject *, int, int); static int type_ready_publish(PyTypeObject *, int); @@ -6328,6 +6191,52 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, _PyStackRef *out) return res; } +#ifdef Py_GIL_DISABLED +/* Variant of find_name_in_mro() used to compute slots. 'name' must be an + exact str. Keys of the MRO dicts that are not exact str are skipped + without being compared, so no user code runs. This can be called with the + world stopped; the result is then a borrowed reference since no other + thread can drop it. Returns 0 if the name was not found and 1 if it was. */ +static int +find_slot_name_in_mro(PyTypeObject *type, PyObject *name, _PyStackRef *out) +{ + assert(PyUnicode_CheckExact(name)); + *out = PyStackRef_NULL; + PyObject *mro = lookup_tp_mro(type); + if (mro == NULL) { + return 0; + } + int res = 0; + int world_stopped = types_world_is_stopped(); + if (!world_stopped) { + // tp_mro can be replaced while the dict locks are released. + Py_INCREF(mro); + } + Py_ssize_t n = PyTuple_GET_SIZE(mro); + for (Py_ssize_t i = 0; i < n; i++) { + PyObject *base = PyTuple_GET_ITEM(mro, i); + PyDictObject *dict = (PyDictObject *)lookup_tp_dict(_PyType_CAST(base)); + assert(dict && PyDict_Check(dict)); + if (world_stopped) { + PyObject *value = _PyDict_GetItem_ExactUnicode(dict, name); + if (value != NULL) { + *out = PyStackRef_FromPyObjectBorrow(value); + res = 1; + break; + } + } + else if (_PyDict_GetItemStackRef_ExactUnicode(dict, name, out)) { + res = 1; + break; + } + } + if (!world_stopped) { + Py_DECREF(mro); + } + return res; +} +#endif + /* Check if the "readied" PyUnicode name is a double-underscore special name. */ static int @@ -6730,7 +6639,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, int is_slot_name, PyObject **old_value) + PyObject *value, 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 @@ -6759,56 +6668,23 @@ type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name, return -1; } - if (is_slot_name) { - types_publish_gen_inc(); - } - return 0; } -static int +static void update_slot_after_setattr(PyTypeObject *type, PyObject *name) { + types_stop_world(); + update_slot(type, name); + types_start_world(); #ifdef Py_GIL_DISABLED - assert(SLOT_UPDATE_CHUNK_SIZE >= MAX_EQUIV); - slot_update_chunk_t chunk = {0}; - // Stack allocate the first chunk. It is usually the only one needed but - // updates are queued for subclasses as well, so more chunks are needed if - // the type has more than SLOT_UPDATE_CHUNK_SIZE subclasses. - slot_update_t queued_updates = {&chunk}; - - int res; - for (;;) { - uint64_t gen = types_publish_gen(); - res = update_slot(type, name, &queued_updates); - if (res < 0 || slot_update_is_empty(&queued_updates)) { - break; - } - 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 - // 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); - } - // 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); - return 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 waiting for the stop-the-world 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); #endif } @@ -6892,15 +6768,14 @@ 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, - is_slot_name, &old_value); + res = type_update_dict(type, (PyDictObject *)dict, name, value, &old_value); assert(_PyType_CheckConsistency(type)); - if (res == 0 && is_slot_name) { - res = update_slot_after_setattr(type, name); + if (res == 0) { + if (is_dunder_name(name) && has_slotdef(name)) { + // The name corresponds to a type slot. + update_slot_after_setattr(type, name); + } } END_TYPE_DICT_LOCK(); @@ -7656,6 +7531,24 @@ compatible_with_tp_base(PyTypeObject *child) child->tp_dealloc == parent->tp_dealloc)); } +// Compare two ht_slots tuples by string content, without calling a user +// __eq__, since this can be called with the world stopped. +static int +same_slot_names(PyObject *slots_a, PyObject *slots_b) +{ + Py_ssize_t n = PyTuple_GET_SIZE(slots_a); + if (n != PyTuple_GET_SIZE(slots_b)) { + return 0; + } + for (Py_ssize_t i = 0; i < n; i++) { + if (!_PyUnicode_Equal(PyTuple_GET_ITEM(slots_a, i), + PyTuple_GET_ITEM(slots_b, i))) { + return 0; + } + } + return 1; +} + static int same_slots_added(PyTypeObject *a, PyTypeObject *b) { @@ -7678,7 +7571,7 @@ same_slots_added(PyTypeObject *a, PyTypeObject *b) slots_a = ((PyHeapTypeObject *)a)->ht_slots; slots_b = ((PyHeapTypeObject *)b)->ht_slots; if (slots_a && slots_b) { - if (PyObject_RichCompareBool(slots_a, slots_b, Py_EQ) != 1) + if (!same_slot_names(slots_a, slots_b)) return 0; size += sizeof(PyObject *) * PyTuple_GET_SIZE(slots_a); } @@ -7696,28 +7589,21 @@ compatible_flags(int setclass, PyTypeObject *origto, PyTypeObject *newto, unsign : !(~(origto->tp_flags & flags) & (newto->tp_flags & flags)); } +// Return 1 if the object layout of 'newto' is compatible with 'origto' (for +// __class__ assignment) or with the base of 'origto' (for __bases__ +// assignment), else 0. Does not raise and does not run user code. static int -compatible_for_assignment(PyTypeObject *origto, PyTypeObject *newto, - const char *attr, int setclass) +compatible_layout(PyTypeObject *origto, PyTypeObject *newto, int setclass) { PyTypeObject *newbase, *oldbase; PyTypeObject *oldto = setclass ? origto : origto->tp_base; - if (setclass && newto->tp_free != oldto->tp_free) { - PyErr_Format(PyExc_TypeError, - "%s assignment: " - "'%s' deallocator differs from '%s'", - attr, - newto->tp_name, - oldto->tp_name); - return 0; - } if (!compatible_flags(setclass, origto, newto, Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_INLINE_VALUES | Py_TPFLAGS_PREHEADER)) { - goto differs; + return 0; } /* For __class__ assignment, tp_dictoffset and tp_weaklistoffset should be the same for old and new types. @@ -7727,12 +7613,12 @@ compatible_for_assignment(PyTypeObject *origto, PyTypeObject *newto, if ((setclass || newto->tp_dictoffset) && origto->tp_dictoffset != newto->tp_dictoffset) { - goto differs; + return 0; } if ((setclass || newto->tp_weaklistoffset) && origto->tp_weaklistoffset != newto->tp_weaklistoffset) { - goto differs; + return 0; } /* It's tricky to tell if two arbitrary types are sufficiently compatible as @@ -7753,17 +7639,43 @@ compatible_for_assignment(PyTypeObject *origto, PyTypeObject *newto, if (newbase != oldbase && (newbase->tp_base != oldbase->tp_base || !same_slots_added(newbase, oldbase))) { - goto differs; + return 0; } return 1; -differs: +} + +static void +raise_layout_differs(const char *attr, PyTypeObject *newto, + PyTypeObject *oldto) +{ PyErr_Format(PyExc_TypeError, "%s assignment: " "'%s' object layout differs from '%s'", attr, newto->tp_name, oldto->tp_name); - return 0; +} + +static int +compatible_for_assignment(PyTypeObject *origto, PyTypeObject *newto, + const char *attr, int setclass) +{ + PyTypeObject *oldto = setclass ? origto : origto->tp_base; + + if (setclass && newto->tp_free != oldto->tp_free) { + PyErr_Format(PyExc_TypeError, + "%s assignment: " + "'%s' deallocator differs from '%s'", + attr, + newto->tp_name, + oldto->tp_name); + return 0; + } + if (!compatible_layout(origto, newto, setclass)) { + raise_layout_differs(attr, newto, oldto); + return 0; + } + return 1; } @@ -8883,10 +8795,23 @@ overrides_hash(PyTypeObject *type) PyObject *dict = lookup_tp_dict(type); assert(dict != NULL); +#ifdef Py_GIL_DISABLED + // Ignore non-str keys, consistent with find_slot_name_in_mro(). + _PyStackRef ref; + int r = _PyDict_GetItemStackRef_ExactUnicode( + (PyDictObject *)dict, &_Py_ID(__eq__), &ref); + PyStackRef_XCLOSE(ref); + if (r == 0) { + r = _PyDict_GetItemStackRef_ExactUnicode( + (PyDictObject *)dict, &_Py_ID(__hash__), &ref); + PyStackRef_XCLOSE(ref); + } +#else int r = PyDict_Contains(dict, &_Py_ID(__eq__)); if (r == 0) { r = PyDict_Contains(dict, &_Py_ID(__hash__)); } +#endif return r; } @@ -12013,17 +11938,14 @@ has_slotdef(PyObject *name) * When done, next_p is set to the next slotdef with a different offset, * because that's convenient for fixup_slot_dispatchers(). * - * If the queued_updates pointer is provided, the actual updates to the slot - * pointers are queued, rather than being immediately performed. That argument - * is only used for the free-threaded build since those updates need to be - * done while the world is stopped. + * In the free-threaded build, this is called with the world stopped for a + * type that has been revealed to other threads. The descriptors are looked + * up with find_slot_name_in_mro(), which ignores non-str dict keys. * - * This function will only return an error if the queued_updates argument is - * provided and allocating memory for the queue fails. Other exceptions that - * occur internally are ignored, such as when looking up descriptors. */ -static int -update_one_slot(PyTypeObject *type, pytype_slotdef *p, pytype_slotdef **next_p, - slot_update_t *queued_updates) + * Exceptions that occur internally are ignored, such as when looking up + * descriptors. */ +static void +update_one_slot(PyTypeObject *type, pytype_slotdef *p, pytype_slotdef **next_p) { ASSERT_NEW_TYPE_OR_LOCKED(type); @@ -12050,14 +11972,18 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p, pytype_slotdef **next_p, if (next_p != NULL) { *next_p = p; } - return 0; + return; } /* We may end up clearing live exceptions below, so make sure it's ours. */ assert(!PyErr_Occurred()); do { /* Use faster uncached lookup as we won't get any cache hits during type setup. */ _PyStackRef descr_ref; +#ifdef Py_GIL_DISABLED + int res = find_slot_name_in_mro(type, p->name_strobj, &descr_ref); +#else int res = find_name_in_mro(type, p->name_strobj, &descr_ref); +#endif if (res <= 0) { if (ptr == (void**)&type->tp_iternext) { specific = (void *)_PyObject_NextNotImplemented; @@ -12148,9 +12074,7 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p, pytype_slotdef **next_p, } if (p->function == slot_tp_call) { /* A generic __call__ is incompatible with vectorcall */ - if (queued_updates == NULL) { - type_clear_flags(type, Py_TPFLAGS_HAVE_VECTORCALL); - } + type_clear_flags(type, Py_TPFLAGS_HAVE_VECTORCALL); } } PyStackRef_CLOSE(descr_ref); @@ -12163,26 +12087,12 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p, pytype_slotdef **next_p, slot_value = generic; } -#ifdef Py_GIL_DISABLED - if (queued_updates != NULL) { - // queue the update to perform later, while world is stopped - if (queue_slot_update(queued_updates, type, ptr, slot_value) < 0) { - return -1; - } - } else { - // do the update to the type structure now - *ptr = slot_value; - } -#else - // always do the update immediately - assert(queued_updates == NULL); + ASSERT_WORLD_STOPPED_OR_NEW_TYPE(type); *ptr = slot_value; -#endif if (next_p != NULL) { *next_p = p; } - return 0; } /* In the type, update the slots whose slotdefs are gathered in the pp array. @@ -12195,16 +12105,15 @@ update_slots_callback(PyTypeObject *type, void *data) update_callback_data_t *update_data = (update_callback_data_t *)data; pytype_slotdef **pp = update_data->defs; for (; *pp; pp++) { - if (update_one_slot(type, *pp, NULL, update_data->queued_updates) < 0) { - return -1; - } + update_one_slot(type, *pp, NULL); } return 0; } -/* Update the slots after assignment to a class (type) attribute. */ +/* Update the slots after assignment to a class (type) attribute. In the + free-threaded build, this is called with the world stopped. */ static int -update_slot(PyTypeObject *type, PyObject *name, slot_update_t *queued_updates) +update_slot(PyTypeObject *type, PyObject *name) { pytype_slotdef *ptrs[MAX_EQUIV]; pytype_slotdef *p; @@ -12238,7 +12147,6 @@ update_slot(PyTypeObject *type, PyObject *name, slot_update_t *queued_updates) update_callback_data_t callback_data; callback_data.defs = ptrs; - callback_data.queued_updates = queued_updates; return update_subclasses(type, name, update_slots_callback, (void *)&callback_data); } @@ -12254,70 +12162,26 @@ fixup_slot_dispatchers(PyTypeObject *type) ASSERT_WORLD_STOPPED_OR_NEW_TYPE(type); assert(!PyErr_Occurred()); for (pytype_slotdef *p = slotdefs; p->name; ) { - int rv = update_one_slot(type, p, &p, NULL); - // always returns 0 if queued_updates == NULL - assert (rv == 0); - (void)rv; + update_one_slot(type, p, &p); } } -#ifdef Py_GIL_DISABLED - // 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) -{ - // Note that update_slot() can fail due to out-of-memory when allocating - // the queue chunks to hold the updates. That's unlikely since the number - // of updates is normally small but we handle that case. update_slot() - // can fail internally for other reasons (a lookup fails) but those - // errors are suppressed. - slot_update_t queued_updates = {0}; - 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; - } - } - if (slot_update_is_empty(&queued_updates)) { - break; - } - 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. Also clears the VALID_VERSION -// flag of 'type' and all its subclasses. -static int +static void update_all_slots(PyTypeObject* type) { - pytype_slotdef *p; - for (p = slotdefs; p->name; p++) { + types_stop_world(); + for (pytype_slotdef *p = slotdefs; p->name; p++) { /* update_slot returns int but can't actually fail in this case*/ - update_slot(type, p->name_strobj, NULL); + update_slot(type, p->name_strobj); } + types_start_world(); + // Invalidate the version tags after the slots are updated, while + // TYPE_LOCK is still held: see update_slot_after_setattr(). _PyType_Modified_Unlocked(type); - return 0; } -#endif - int _PyType_InitSlotDefs(PyInterpreterState *interp) { @@ -12512,6 +12376,31 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, Py_ssize_t i = 0; PyObject *ref; while (PyDict_Next(subclasses, &i, NULL, &ref)) { +#ifdef Py_GIL_DISABLED + // With the world stopped, use borrowed references to the subclasses, + // so that dropping a reference cannot run type_dealloc(). Look up + // attr_name without calling __eq__ of non-str keys, consistent with + // find_slot_name_in_mro(). + assert(types_world_is_stopped()); + if (_PyWeakref_IS_DEAD(ref)) { + continue; + } + PyTypeObject *subclass = + _PyType_CAST(((PyWeakReference *)ref)->wr_object); + + /* Avoid recursing down into unaffected classes */ + PyObject *dict = lookup_tp_dict(subclass); + if (dict != NULL && PyDict_Check(dict) && + _PyDict_GetItem_ExactUnicode((PyDictObject *)dict, + attr_name) != NULL) + { + continue; + } + + if (update_subclasses(subclass, attr_name, callback, data) < 0) { + return -1; + } +#else PyTypeObject *subclass = type_from_ref(ref); if (subclass == NULL) { continue; @@ -12536,6 +12425,7 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, return -1; } Py_DECREF(subclass); +#endif } return 0; }