gh-148393: Use atomic ops on _ma_watcher_tag in free threading build (gh-148397)

Fixes data races between dict mutation and watch/unwatch on the same dict.

Author: colesbury

Include/internal/pycore_dict.h

@@ -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

Include/internal/pycore_pyatomic_ft_wrappers.h

@@ -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

Lib/test/test_free_threading/test_dict.py

@@ -245,6 +245,29 @@ def reader():
         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

Misc/NEWS.d/next/Core_and_Builtins/2026-04-11-17-28-52.gh-issue-148393.lX6gwN.rst

@@ -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`.

Objects/dictobject.c

@@ -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;
 }
 

Python/optimizer_analysis.c

@@ -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,