[3.14] gh-148393: Use atomic ops on _ma_watcher_tag in free threading build (gh-14839...

Fixes data races between dict mutation and watch/unwatch on the same dict.
(cherry picked from commit 3ab94d684286b49144bf2e43cc1041f3e4c0cda8)

Co-authored-by: Sam Gross <colesbury@gmail.com>

diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h
index cfdc63f4d919a37d4e3031b611572a398fc195e2..346e4e64a42a56e370dafac3fd0c09d66206872b 100644 (file)
--- a/Include/internal/pycore_dict.h
+++ b/Include/internal/pycore_dict.h
@@ -286,7 +286,7 @@ _PyDict_NotifyEvent(PyInterpreterState *interp,
                     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);
@@ -362,7 +362,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

diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h
index be0335a2389c0af19aaaecbb968c9767d81b040b..3019fada11ee4f032e2c3b824208d284f2ec8b31 100644 (file)
--- a/Include/internal/pycore_pyatomic_ft_wrappers.h
+++ b/Include/internal/pycore_pyatomic_ft_wrappers.h
@@ -47,6 +47,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) \
@@ -63,6 +65,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) \
@@ -139,6 +147,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
@@ -148,6 +157,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

diff --git a/Lib/test/test_free_threading/test_dict.py b/Lib/test/test_free_threading/test_dict.py
index 1ffd924e9f477a690d5f205748f26b59492b90d4..55272a00c3ad501707c30484286263913dd10e0d 100644 (file)
--- a/Lib/test/test_free_threading/test_dict.py
+++ b/Lib/test/test_free_threading/test_dict.py
@@ -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
+++ b/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`.

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index dac9d016db0183389e4de0702602012a3c461e91..79a73b5971af3021fb1b1c59d7680edeca1905e2 100644 (file)
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -7718,7 +7718,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;
 }
 
@@ -7733,7 +7733,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;
 }
 

diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c
index 2bcd99b8bcdc6d5188158dced430d02d6d76c775..84a660658e3cb7ae661aa2ce6ea893a6142aed6b 100644 (file)
--- a/Python/optimizer_analysis.c
+++ b/Python/optimizer_analysis.c
@@ -54,14 +54,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,