]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-148393: Use atomic ops on _ma_watcher_tag in free threading build (gh-148397)
authorSam Gross <colesbury@gmail.com>
Sun, 12 Apr 2026 14:40:41 +0000 (10:40 -0400)
committerGitHub <noreply@github.com>
Sun, 12 Apr 2026 14:40:41 +0000 (10:40 -0400)
Fixes data races between dict mutation and watch/unwatch on the same dict.

Include/internal/pycore_dict.h
Include/internal/pycore_pyatomic_ft_wrappers.h
Lib/test/test_free_threading/test_dict.py
Misc/NEWS.d/next/Core_and_Builtins/2026-04-11-17-28-52.gh-issue-148393.lX6gwN.rst [new file with mode: 0644]
Objects/dictobject.c
Python/optimizer_analysis.c

index d58539fa84656361506b17eaea63f8a96b5a5f2a..5bbea187394db6cc9e2508ed02d62a6927b12489 100644 (file)
@@ -292,7 +292,7 @@ _PyDict_NotifyEvent(PyDict_WatchEvent event,
                     PyObject *value)
 {
     assert(Py_REFCNT((PyObject*)mp) > 0);
-    int watcher_bits = mp->_ma_watcher_tag & DICT_WATCHER_MASK;
+    int watcher_bits = FT_ATOMIC_LOAD_UINT64_RELAXED(mp->_ma_watcher_tag) & DICT_WATCHER_MASK;
     if (watcher_bits) {
         RARE_EVENT_STAT_INC(watched_dict_modification);
         _PyDict_SendEvent(watcher_bits, event, mp, key, value);
@@ -368,7 +368,7 @@ PyDictObject *_PyObject_MaterializeManagedDict_LockHeld(PyObject *);
 static inline Py_ssize_t
 _PyDict_UniqueId(PyDictObject *mp)
 {
-    return (Py_ssize_t)(mp->_ma_watcher_tag >> DICT_UNIQUE_ID_SHIFT);
+    return (Py_ssize_t)(FT_ATOMIC_LOAD_UINT64_RELAXED(mp->_ma_watcher_tag) >> DICT_UNIQUE_ID_SHIFT);
 }
 
 static inline void
index c0f859a23e10b893adf0bb11ef6e470dbcfa2856..3155481bb5c36b9b4f9388e07ffaf7d219974bfc 100644 (file)
@@ -49,6 +49,8 @@ extern "C" {
     _Py_atomic_load_uint16_relaxed(&value)
 #define FT_ATOMIC_LOAD_UINT32_RELAXED(value) \
     _Py_atomic_load_uint32_relaxed(&value)
+#define FT_ATOMIC_LOAD_UINT64_RELAXED(value) \
+    _Py_atomic_load_uint64_relaxed(&value)
 #define FT_ATOMIC_LOAD_ULONG_RELAXED(value) \
     _Py_atomic_load_ulong_relaxed(&value)
 #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) \
@@ -71,6 +73,12 @@ extern "C" {
     _Py_atomic_store_uint16_relaxed(&value, new_value)
 #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \
     _Py_atomic_store_uint32_relaxed(&value, new_value)
+#define FT_ATOMIC_AND_UINT64(value, new_value) \
+    (void)_Py_atomic_and_uint64(&value, new_value)
+#define FT_ATOMIC_OR_UINT64(value, new_value) \
+    (void)_Py_atomic_or_uint64(&value, new_value)
+#define FT_ATOMIC_ADD_UINT64(value, new_value) \
+    (void)_Py_atomic_add_uint64(&value, new_value)
 #define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) \
     _Py_atomic_store_char_relaxed(&value, new_value)
 #define FT_ATOMIC_LOAD_CHAR_RELAXED(value) \
@@ -146,6 +154,7 @@ extern "C" {
 #define FT_ATOMIC_LOAD_UINT8_RELAXED(value) value
 #define FT_ATOMIC_LOAD_UINT16_RELAXED(value) value
 #define FT_ATOMIC_LOAD_UINT32_RELAXED(value) value
+#define FT_ATOMIC_LOAD_UINT64_RELAXED(value) value
 #define FT_ATOMIC_LOAD_ULONG_RELAXED(value) value
 #define FT_ATOMIC_STORE_PTR_RELAXED(value, new_value) value = new_value
 #define FT_ATOMIC_STORE_PTR_RELEASE(value, new_value) value = new_value
@@ -157,6 +166,9 @@ extern "C" {
 #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
+#define FT_ATOMIC_AND_UINT64(value, new_value) (void)(value &= new_value)
+#define FT_ATOMIC_OR_UINT64(value, new_value) (void)(value |= new_value)
+#define FT_ATOMIC_ADD_UINT64(value, new_value) (void)(value += new_value)
 #define FT_ATOMIC_LOAD_CHAR_RELAXED(value) value
 #define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) value = new_value
 #define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) value
index 1ffd924e9f477a690d5f205748f26b59492b90d4..55272a00c3ad501707c30484286263913dd10e0d 100644 (file)
@@ -245,6 +245,29 @@ class TestDict(TestCase):
         with threading_helper.start_threads([t1, t2]):
             pass
 
+    @unittest.skipIf(_testcapi is None, "requires _testcapi")
+    def test_racing_watch_unwatch_dict(self):
+        # gh-148393: race between PyDict_Watch / PyDict_Unwatch
+        # and concurrent dict mutation reading _ma_watcher_tag.
+        wid = _testcapi.add_dict_watcher(0)
+        try:
+            d = {}
+            ITERS = 1000
+
+            def writer():
+                for i in range(ITERS):
+                    d[i] = i
+                    del d[i]
+
+            def watcher():
+                for _ in range(ITERS):
+                    _testcapi.watch_dict(wid, d)
+                    _testcapi.unwatch_dict(wid, d)
+
+            threading_helper.run_concurrently([writer, watcher])
+        finally:
+            _testcapi.clear_dict_watcher(wid)
+
     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-04-11-17-28-52.gh-issue-148393.lX6gwN.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-04-11-17-28-52.gh-issue-148393.lX6gwN.rst
new file mode 100644 (file)
index 0000000..33c4b75
--- /dev/null
@@ -0,0 +1,2 @@
+Fix data races between :c:func:`PyDict_Watch` / :c:func:`PyDict_Unwatch`
+and concurrent dict mutation in the :term:`free-threaded build`.
index 67bc4319e0bae201529eddff2e207666e44d37ee..b5300eb410c69cee86adfea331548d00a55c1d7c 100644 (file)
@@ -8028,7 +8028,7 @@ PyDict_Watch(int watcher_id, PyObject* dict)
     if (validate_watcher_id(interp, watcher_id)) {
         return -1;
     }
-    ((PyDictObject*)dict)->_ma_watcher_tag |= (1LL << watcher_id);
+    FT_ATOMIC_OR_UINT64(((PyDictObject*)dict)->_ma_watcher_tag, (1LL << watcher_id));
     return 0;
 }
 
@@ -8043,7 +8043,7 @@ PyDict_Unwatch(int watcher_id, PyObject* dict)
     if (validate_watcher_id(interp, watcher_id)) {
         return -1;
     }
-    ((PyDictObject*)dict)->_ma_watcher_tag &= ~(1LL << watcher_id);
+    FT_ATOMIC_AND_UINT64(((PyDictObject*)dict)->_ma_watcher_tag, ~(1LL << watcher_id));
     return 0;
 }
 
index ca9bcc8a40c35e4b8b4bf8b20564cc4ed5e5dc8c..6742488a0d06c25f1e16f3dff493ba1eb1dc7d83 100644 (file)
@@ -119,14 +119,15 @@ static int
 get_mutations(PyObject* dict) {
     assert(PyDict_CheckExact(dict));
     PyDictObject *d = (PyDictObject *)dict;
-    return (d->_ma_watcher_tag >> DICT_MAX_WATCHERS) & ((1 << DICT_WATCHED_MUTATION_BITS)-1);
+    uint64_t tag = FT_ATOMIC_LOAD_UINT64_RELAXED(d->_ma_watcher_tag);
+    return (tag >> DICT_MAX_WATCHERS) & ((1 << DICT_WATCHED_MUTATION_BITS) - 1);
 }
 
 static void
 increment_mutations(PyObject* dict) {
     assert(PyDict_CheckExact(dict));
     PyDictObject *d = (PyDictObject *)dict;
-    d->_ma_watcher_tag += (1 << DICT_MAX_WATCHERS);
+    FT_ATOMIC_ADD_UINT64(d->_ma_watcher_tag, (1 << DICT_MAX_WATCHERS));
 }
 
 /* The first two dict watcher IDs are reserved for CPython,