gh-144295: Fix data race in dict method lookup and global load (gh-144312)

In `_PyDict_GetMethodStackRef`, only use the fast-path unicode lookup
when the dict is owned by the current thread or already marked as shared.
This prevents a race between the lookup and concurrent dict resizes,
which may free the PyDictKeysObject (i.e., it ensures that the resize
uses QSBR).

Address a similar issue in `_Py_dict_lookup_threadsafe_stackref` by
calling `ensure_shared_on_read()`.

diff --git a/Lib/test/test_free_threading/test_dict.py b/Lib/test/test_free_threading/test_dict.py
index 5d5d4e226caa4085458b971d4af348ac8f074b4d..1ffd924e9f477a690d5f205748f26b59492b90d4 100644 (file)
--- a/Lib/test/test_free_threading/test_dict.py
+++ b/Lib/test/test_free_threading/test_dict.py
@@ -245,5 +245,27 @@ class TestDict(TestCase):
         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 aea9ea84202b074e961344ef6be0319280f2b173..c1584be3f0ed4a38bac83d84d718a7941af7240b 100644 (file)
--- 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