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/Lib/test/test_descr.py b/Lib/test/test_descr.py index 93b902a632de35..c4c4d0858ec00b 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -5098,6 +5098,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_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 a55c4815a03837..e87a773c43bb80 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,24 @@ class A: attr = 1 +def world_stoppers(): + """Finite workloads that contend for the stop-the-world mutex. + + 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(): + for _ in range(100): + _testinternalcapi.stop_the_world() + + def pauser(): + for _ in range(100): + _testinternalcapi.stop_the_world(0.0001) + + return [stopper, pauser] + + @threading_helper.requires_working_threading() class TestType(TestCase): def test_attr_cache(self): @@ -325,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 @@ -344,6 +362,11 @@ def custom_repr(self): self.assertTrue(all(repr(cls()) != "custom repr" for cls in subclasses)) + # The slot update must not keep references to the types. + 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 @@ -401,6 +424,151 @@ 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. + def basesetter(): + class A: + pass + class B: + pass + class C(A): + pass + for _ in range(100): + C.__bases__ = (B,) + C.__bases__ = (A,) + + def stopper(): + 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 + + class Meta(type): + def mro(cls): + _testinternalcapi.stop_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_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_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 type lock is released while + # waiting for the stop-the-world. At the end, the MRO must agree + # with __bases__. + N = 50 + + 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(): + for _ in range(N): + C.__bases__ = (B,) + C.__bases__ = (A,) + + def subsetter(): + for _ in range(N): + D.__bases__ = (C, B) + D.__bases__ = (C,) + + threading_helper.run_concurrently( + [setter, setter, subsetter] + world_stoppers()) + + 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/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 16be593763bf66..953d5af466c480 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)) { @@ -3348,6 +3385,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}, diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 1c3b98032b8af1..278bfb17695fb9 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 87da481f0a5f60..1ee2fd969cd013 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 @@ -77,7 +70,6 @@ typedef struct { #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) @@ -85,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 @@ -131,57 +122,6 @@ 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) -{ - 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); -} - -static void -type_lock_allow_release(pinned_mutexes_t *pinned) -{ - 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); - } -} - #else #define BEGIN_TYPE_LOCK() @@ -195,8 +135,6 @@ 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)) #endif @@ -599,7 +537,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 +545,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 +560,28 @@ set_tp_bases(PyTypeObject *self, PyObject *bases, int initial) self->tp_bases = bases; } +// Replace tp_bases and tp_base of a type with the world stopped. Returns 1 +// 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) +{ + ASSERT_TYPE_LOCK_HELD(); + assert(PyTuple_Check(bases)); + assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + int res = 0; + types_stop_world(); + if (self->tp_bases == expected) { + self->tp_bases = bases; + self->tp_base = base; + res = 1; + } + types_start_world(); + ASSERT_TYPE_LOCK_HELD(); + return res; +} + static inline void clear_tp_bases(PyTypeObject *self, int final) { @@ -651,14 +610,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 +625,30 @@ 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 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) +{ + ASSERT_TYPE_LOCK_HELD(); + assert(!(self->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + assert(mro == NULL || PyTuple_CheckExact(mro)); + int res = 0; + types_stop_world(); + if (self->tp_mro == expected) { + if (mro != NULL) { + PyUnstable_Object_EnableDeferredRefcount(mro); + } + self->tp_mro = mro; + res = 1; } + types_start_world(); + ASSERT_TYPE_LOCK_HELD(); + return res; } static inline void @@ -1706,18 +1678,17 @@ 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 update, while TYPE_LOCK is + // still held. ASSERT_TYPE_LOCK_HELD(); + _PyType_Modified_Unlocked(type); END_TYPE_LOCK(); return 0; @@ -1748,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, @@ -1794,8 +1767,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) == 1) { + Py_DECREF(new_mro); + } + else { + Py_XDECREF(old_mro); + } return -1; } Py_XDECREF(old_mro); @@ -1831,8 +1809,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 +1841,42 @@ 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; + 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). 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(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); if (is_subtype_with_mro(lookup_tp_mro(base), base, type) || /* In case of reentering here again through a custom mro() @@ -1876,41 +1891,81 @@ type_check_new_bases(PyTypeObject *type, PyObject *new_bases, PyTypeObject **bes (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; +} + +// 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) +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; - 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); + 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(); - type_lock_allow_release(&pinned); + 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); if (temp == NULL) { @@ -1934,11 +1989,9 @@ type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, PyTypeObject *b add to all new_bases */ remove_all_subclasses(type, old_bases); res = add_all_subclasses(type, new_bases); - if (update_all_slots(type) < 0) { - goto bail; - } - /* Clear the VALID_VERSION flag of 'type' and all its subclasses. */ - _PyType_Modified_Unlocked(type); + /* This also clears the VALID_VERSION flag of 'type' and all its + subclasses. */ + update_all_slots(type); } else { res = 0; @@ -1960,24 +2013,20 @@ 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) == 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) == 1) + { Py_DECREF(new_bases); Py_DECREF(best_base); } @@ -1994,14 +2043,25 @@ static int 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; + } + // 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(); + res = type_set_bases_unlocked(type, bases); END_TYPE_LOCK(); + Py_DECREF(bases); return res; } @@ -3632,25 +3692,45 @@ 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) { + Py_XDECREF(old_mro); return -1; } - - if (reent) { + 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, so check for reentrance inside the + // 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); + } + if (res == 0) { + /* reentrance: tp_mro was replaced by someone else */ Py_DECREF(new_mro); + Py_XDECREF(old_mro); return 0; } - - 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 @@ -3675,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". */ @@ -3682,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); @@ -3709,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 " @@ -3761,169 +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. - -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_free_chunks(slot_update_t *updates) -{ - slot_update_chunk_t *chunk = updates->head; - while (chunk != NULL) { - slot_update_chunk_t *prev = chunk->prev; - PyMem_Free(chunk); - chunk = prev; - } -} - -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 = 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 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; - } -} - -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); - types_stop_world(); - apply_slot_updates(updates); - types_start_world(); - type_lock_allow_release(&pinned); -} - -#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); @@ -4424,7 +4376,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; @@ -4889,10 +4841,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; } @@ -5630,7 +5644,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; @@ -6177,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 @@ -6397,24 +6457,19 @@ 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 update, while TYPE_LOCK is + // still held. + ASSERT_TYPE_LOCK_HELD(); + if (modified) { + _PyType_Modified_Unlocked(self); + } END_TYPE_LOCK(); } @@ -6589,9 +6644,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; } @@ -6614,32 +6671,20 @@ type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name, 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 = update_slot(type, name, &queued_updates); - if (res == 0 && queued_updates.head->n > 0) { - apply_type_slot_updates(&queued_updates); - ASSERT_TYPE_LOCK_HELD(); - } - slot_update_chunk_t *cur = queued_updates.head; - while (cur != &chunk) { - slot_update_chunk_t *prev = cur->prev; - PyMem_Free(cur); - cur = prev; - } - 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 } @@ -6729,7 +6774,7 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value) 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); + update_slot_after_setattr(type, name); } } END_TYPE_DICT_LOCK(); @@ -7488,6 +7533,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) { @@ -7510,7 +7573,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); } @@ -7528,28 +7591,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. @@ -7559,12 +7615,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 @@ -7585,17 +7641,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; } @@ -8715,10 +8797,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; } @@ -9023,7 +9118,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; } @@ -11845,17 +11940,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); @@ -11882,14 +11974,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; @@ -11980,9 +12076,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); @@ -11995,26 +12089,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. @@ -12027,16 +12107,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; @@ -12070,7 +12149,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); } @@ -12086,57 +12164,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. -static int +// Called when __bases__ is re-assigned. Also clears the VALID_VERSION +// flag of 'type' and all its subclasses. +static void 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}; + types_stop_world(); 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); - } - return -1; - } - } - if (queued_updates.head != NULL) { - apply_type_slot_updates(&queued_updates); - ASSERT_TYPE_LOCK_HELD(); - slot_update_free_chunks(&queued_updates); - } - return 0; -} - -#else - -// Called when __bases__ is re-assigned. -static int -update_all_slots(PyTypeObject* type) -{ - pytype_slotdef *p; - for (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); } - return 0; + 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); } -#endif - int _PyType_InitSlotDefs(PyInterpreterState *interp) { @@ -12331,6 +12378,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; @@ -12355,6 +12427,7 @@ recurse_down_subclasses(PyTypeObject *type, PyObject *attr_name, return -1; } Py_DECREF(subclass); +#endif } return 0; }