diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 77d9989..e43a914 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -314,8 +314,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_RELAXED(values->size, size+1); } static inline size_t diff --git a/Lib/test/test_free_threading/test_dict.py b/Lib/test/test_free_threading/test_dict.py index 55272a0..d606917 100644 --- a/Lib/test/test_free_threading/test_dict.py +++ b/Lib/test/test_free_threading/test_dict.py @@ -268,6 +268,56 @@ def watcher(): finally: _testcapi.clear_dict_watcher(wid) + def test_racing_split_dict_iteration_and_delete(self): + # Lock-free iteration of a split (instance) dict must not crash when + # a value slot is cleared by a concurrent delete or clear(). + # The values are lists (mortal objects), so the reader takes the + # incref path instead of the immortal-object shortcut. + 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(500): + for i, name in enumerate(names): + delattr(obj, name) + setattr(obj, name, [i]) + + def delitem_writer(): + for _ in range(500): + for i, name in enumerate(names): + try: + del d[name] + except KeyError: + pass + d[name] = [i] + + def clear_writer(): + for _ in range(500): + d.clear() + for i, name in enumerate(names): + setattr(obj, name, [i]) + + def reader(view): + for _ in range(2000): + 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_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-09-25-12-00-00.gh-issue-NNNNNN.Qk7sVd.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-25-12-00-00.gh-issue-NNNNNN.Qk7sVd.rst new file mode 100644 index 0000000..e995caa --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-25-12-00-00.gh-issue-NNNNNN.Qk7sVd.rst @@ -0,0 +1,4 @@ +Fix a crash in the :term:`free-threaded build` when iterating over the +values or items of an instance ``__dict__`` while another thread deletes +attributes from the same object or clears its ``__dict__``. Also fix a debug +build assertion failure when iterating over its keys in the same situation. diff --git a/Objects/dictobject.c b/Objects/dictobject.c index c7023e4..6c7883d 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2782,9 +2782,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_RELAXED(values->size, size); } static void @@ -2948,9 +2948,9 @@ clear_embedded_values(PyDictValues *values, Py_ssize_t nentries) assert(nentries <= SHARED_KEYS_MAX_SIZE); for (Py_ssize_t i = 0; i < nentries; i++) { refs[i] = values->values[i]; - values->values[i] = NULL; + FT_ATOMIC_STORE_PTR_RELEASE(values->values[i], NULL); } - values->size = 0; + FT_ATOMIC_STORE_UINT8_RELAXED(values->size, 0); for (Py_ssize_t i = 0; i < nentries; i++) { Py_XDECREF(refs[i]); } @@ -5612,9 +5612,17 @@ 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. Read the order from the same `values` snapshot that `used` + // was loaded from: get_index_from_order() re-reads d->ma_values and + // asserts on its (concurrently decremented) size. + 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) { + // The entry was deleted concurrently (the slot is cleared before + // values->size is decremented). Retry under the dict lock. + 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;