diff --git a/Lib/test/test_free_threading/test_dict.py b/Lib/test/test_free_threading/test_dict.py index 5d5d4e226caa40..1ffd924e9f477a 100644 --- a/Lib/test/test_free_threading/test_dict.py +++ b/Lib/test/test_free_threading/test_dict.py @@ -245,5 +245,27 @@ def reader(): with threading_helper.start_threads([t1, t2]): pass + 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 + # for the _PyDict_GetMethodStackRef code path. + import io + obj = io.BytesIO() + + def writer(): + for _ in range(10000): + obj.x = 1 + del obj.x + + def reader(): + for _ in range(10000): + obj.getvalue() + + t1 = Thread(target=writer) + t2 = Thread(target=reader) + + with threading_helper.start_threads([t1, t2]): + pass + if __name__ == "__main__": unittest.main() diff --git a/Objects/dictobject.c b/Objects/dictobject.c index aea9ea84202b07..c1584be3f0ed4a 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1615,7 +1615,9 @@ lookup_threadsafe_unicode(PyDictKeysObject *dk, PyObject *key, Py_hash_t hash, _ Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr) { - PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys); + ensure_shared_on_read(mp); + + PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys); if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) { Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, value_addr); if (ix != DKIX_KEY_CHANGED) { @@ -1669,19 +1671,27 @@ _PyDict_GetMethodStackRef(PyDictObject *mp, PyObject *key, _PyStackRef *method) Py_hash_t hash = hash_unicode_key(key); #ifdef Py_GIL_DISABLED - PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys); - if (dk->dk_kind == DICT_KEYS_UNICODE) { - _PyStackRef ref; - Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, &ref); - if (ix >= 0) { - assert(!PyStackRef_IsNull(ref)); - PyStackRef_XSETREF(*method, ref); - return 1; - } - else if (ix == DKIX_EMPTY) { - return 0; + // NOTE: We can only do the fast-path lookup if we are on the owning + // thread or if the dict is already marked as shared so that the load + // of ma_keys is safe without a lock. We cannot call ensure_shared_on_read() + // in this code path without incref'ing the dict because the dict is a + // borrowed reference protected by QSBR, and acquiring the lock could lead + // to a quiescent state (allowing the dict to be freed). + if (_Py_IsOwnedByCurrentThread((PyObject *)mp) || IS_DICT_SHARED(mp)) { + PyDictKeysObject *dk = _Py_atomic_load_ptr_acquire(&mp->ma_keys); + if (dk->dk_kind == DICT_KEYS_UNICODE) { + _PyStackRef ref; + Py_ssize_t ix = lookup_threadsafe_unicode(dk, key, hash, &ref); + if (ix >= 0) { + assert(!PyStackRef_IsNull(ref)); + PyStackRef_XSETREF(*method, ref); + return 1; + } + else if (ix == DKIX_EMPTY) { + return 0; + } + assert(ix == DKIX_KEY_CHANGED); } - assert(ix == DKIX_KEY_CHANGED); } #endif