diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index e85b360c986668c..b4b1a8ca87dd8bd 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -526,6 +526,9 @@ _Py_atomic_store_ssize_release(Py_ssize_t *obj, Py_ssize_t value); static inline void _Py_atomic_store_int8_release(int8_t *obj, int8_t value); +static inline void +_Py_atomic_store_uint8_release(uint8_t *obj, uint8_t value); + static inline void _Py_atomic_store_int_release(int *obj, int value); diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index 253b35082aafcd2..c2441a1eb913920 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -576,6 +576,10 @@ static inline void _Py_atomic_store_int8_release(int8_t *obj, int8_t value) { __atomic_store_n(obj, value, __ATOMIC_RELEASE); } +static inline void +_Py_atomic_store_uint8_release(uint8_t *obj, uint8_t value) +{ __atomic_store_n(obj, value, __ATOMIC_RELEASE); } + static inline void _Py_atomic_store_ssize_release(Py_ssize_t *obj, Py_ssize_t value) { __atomic_store_n(obj, value, __ATOMIC_RELEASE); } diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 3b3c5f7017e9575..ef5b6d2d71c5c09 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -1073,6 +1073,19 @@ _Py_atomic_store_int8_release(int8_t *obj, int8_t value) #endif } +static inline void +_Py_atomic_store_uint8_release(uint8_t *obj, uint8_t value) +{ +#if defined(_M_X64) || defined(_M_IX86) + *(uint8_t volatile *)obj = value; +#elif defined(_M_ARM64) + _Py_atomic_ASSERT_ARG_TYPE(unsigned __int8); + __stlr8((unsigned __int8 volatile *)obj, (unsigned __int8)value); +#else +# error "no implementation of _Py_atomic_store_uint8_release" +#endif +} + static inline void _Py_atomic_store_uint_release(unsigned int *obj, unsigned int value) { diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index faef303da70314c..b8540ff3baf20a6 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -1031,6 +1031,14 @@ _Py_atomic_store_int8_release(int8_t *obj, int8_t value) memory_order_release); } +static inline void +_Py_atomic_store_uint8_release(uint8_t *obj, uint8_t value) +{ + _Py_USING_STD; + atomic_store_explicit((_Atomic(uint8_t)*)obj, value, + memory_order_release); +} + static inline void _Py_atomic_store_uint_release(unsigned int *obj, unsigned int value) { diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 81f1de3a5be8650..0257aaaf4534d5e 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -354,8 +354,8 @@ _PyDictValues_AddToInsertionOrder(PyDictValues *values, Py_ssize_t ix) uint8_t *array = get_insertion_order_array(values); assert(size < values->capacity); assert(((uint8_t)ix) == ix); - array[size] = (uint8_t)ix; - values->size = size+1; + FT_ATOMIC_STORE_UINT8_RELAXED(array[size], (uint8_t)ix); + FT_ATOMIC_STORE_UINT8_RELEASE(values->size, size+1); } // Exported for external JIT support diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index d8ec306a0dae3fc..31455f6b8153123 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -69,6 +69,8 @@ extern "C" { _Py_atomic_store_ssize_relaxed(&value, new_value) #define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) \ _Py_atomic_store_ssize_release(&value, new_value) +#define FT_ATOMIC_STORE_UINT8_RELEASE(value, new_value) \ + _Py_atomic_store_uint8_release(&value, new_value) #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) \ _Py_atomic_store_uint8_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) \ @@ -167,6 +169,7 @@ extern "C" { #define FT_ATOMIC_STORE_INT8_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) value = new_value +#define FT_ATOMIC_STORE_UINT8_RELEASE(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value diff --git a/Lib/test/test_free_threading/test_dict.py b/Lib/test/test_free_threading/test_dict.py index 4a812275143bc4d..2180613f689c897 100644 --- a/Lib/test/test_free_threading/test_dict.py +++ b/Lib/test/test_free_threading/test_dict.py @@ -314,6 +314,87 @@ def reader(): threading_helper.run_concurrently([writer, reader, reader]) + def test_racing_split_dict_iteration_and_delete(self): + # Each reader owns its iterator. Mortal values exercise reference + # acquisition as well as the concurrent clearing of value slots. + class C: + pass + + names = [f"a{i}" for i in range(8)] + obj = C() + for i, name in enumerate(names): + setattr(obj, name, [i]) + d = obj.__dict__ + + def delattr_writer(): + for _ in range(50): + for i, name in enumerate(names): + delattr(obj, name) + setattr(obj, name, [i]) + + def delitem_writer(): + for _ in range(50): + for i, name in enumerate(names): + del d[name] + d[name] = [i] + + def clear_writer(): + for _ in range(50): + d.clear() + for i, name in enumerate(names): + setattr(obj, name, [i]) + + def reader(view): + for _ in range(200): + try: + for _ in view(): + pass + except RuntimeError: + pass + + for writer in (delattr_writer, delitem_writer, clear_writer): + for view in (d.values, d.items, d.keys): + with self.subTest(writer=writer.__name__, view=view.__name__): + threading_helper.run_concurrently( + [writer, partial(reader, view), partial(reader, view)]) + self.assertEqual(d, {name: [i] for i, name in enumerate(names)}) + + def test_racing_nonembedded_split_dict_iteration_and_delete(self): + class C: + pass + + names = [f"a{i}" for i in range(8)] + # Populate shared keys without populating the copied dict's order + # array, so the writer also initializes previously unused order bytes. + template = C() + for i, name in enumerate(names): + setattr(template, name, [i]) + + def writer(): + for _ in range(50): + for i, name in enumerate(names): + d.pop(name, None) + d[name] = [i] + + def reader(view): + for _ in range(200): + try: + for _ in view(): + pass + except RuntimeError: + pass + + for view_name in ("values", "items", "keys"): + with self.subTest(view=view_name): + obj = C() + obj.a0 = [0] + # Copying a split dict gives it heap-backed values. + d = obj.__dict__.copy() + view = getattr(d, view_name) + threading_helper.run_concurrently( + [writer, partial(reader, view), partial(reader, view)]) + self.assertEqual(d, {name: [i] for i, name in enumerate(names)}) + def test_racing_dict_update_and_method_lookup(self): # gh-144295: test race between dict modifications and method lookups. # Uses BytesIO because the race requires a type without Py_TPFLAGS_INLINE_VALUES diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-03-13-00-00.gh-issue-158197.Qk7sVd.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-03-13-00-00.gh-issue-158197.Qk7sVd.rst new file mode 100644 index 000000000000000..10fdabcb1f2fd84 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-03-13-00-00.gh-issue-158197.Qk7sVd.rst @@ -0,0 +1,5 @@ +Fix crashes and debug-build assertion failures in the :term:`free-threaded +build` when iterating over a split dictionary while another thread deletes +entries or clears it. Publish insertion-order updates atomically so readers +cannot observe an increased size before the corresponding order entry is +initialized. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index c6d055a412a515b..a5582007109c24f 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2925,9 +2925,9 @@ delete_index_from_values(PyDictValues *values, Py_ssize_t ix) assert(i < size); size--; for (; i < size; i++) { - array[i] = array[i+1]; + FT_ATOMIC_STORE_UINT8_RELAXED(array[i], array[i+1]); } - values->size = size; + FT_ATOMIC_STORE_UINT8_RELEASE(values->size, size); } static void @@ -3122,7 +3122,7 @@ clear_embedded_values(PyDictValues *values, Py_ssize_t nentries) refs[i] = values->values[i]; FT_ATOMIC_STORE_PTR_RELEASE(values->values[i], NULL); } - values->size = 0; + FT_ATOMIC_STORE_UINT8_RELEASE(values->size, 0); for (Py_ssize_t i = 0; i < nentries; i++) { Py_XDECREF(refs[i]); } @@ -6102,9 +6102,14 @@ dictiter_iternext_threadsafe(PyDictObject *d, PyObject *self, // We're racing against writes to the order from delete_index_from_values, but // single threaded can suffer from concurrent modification to those as well and // can have either duplicated or skipped attributes, so we strive to do no better - // here. - int index = get_index_from_order(d, i); + // here. Use the same values snapshot as the size load above. + uint8_t *order = get_insertion_order_array(values); + int index = _Py_atomic_load_uint8_relaxed(&order[i]); PyObject *value = _Py_atomic_load_ptr(&values->values[index]); + if (value == NULL) { + // Deletion clears the slot before decrementing values->size. + goto try_locked; + } if (acquire_key_value(&DK_UNICODE_ENTRIES(k)[index].me_key, value, &values->values[index], out_key, out_value) < 0) { goto try_locked;