https://github.com/python/cpython/commit/a89b2419e063b19c6d59428c40cd1ab58606e0b2 commit: a89b2419e063b19c6d59428c40cd1ab58606e0b2 branch: 3.14 author: Miss Islington (bot) <[email protected]> committer: colesbury <[email protected]> date: 2026-04-12T15:05:34Z summary:
[3.14] gh-148393: Use atomic ops on _ma_watcher_tag in free threading build (gh-148397) (#148451) Fixes data races between dict mutation and watch/unwatch on the same dict. (cherry picked from commit 3ab94d684286b49144bf2e43cc1041f3e4c0cda8) Co-authored-by: Sam Gross <[email protected]> files: A Misc/NEWS.d/next/Core_and_Builtins/2026-04-11-17-28-52.gh-issue-148393.lX6gwN.rst M Include/internal/pycore_dict.h M Include/internal/pycore_pyatomic_ft_wrappers.h M Lib/test/test_free_threading/test_dict.py M Objects/dictobject.c M Python/optimizer_analysis.c diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index cfdc63f4d919a3..346e4e64a42a56 100644 --- 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 be0335a2389c0a..3019fada11ee4f 100644 --- 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 1ffd924e9f477a..55272a00c3ad50 100644 --- a/Lib/test/test_free_threading/test_dict.py +++ b/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 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 index 00000000000000..33c4b75bfb944c --- /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 dac9d016db0183..79a73b5971af30 100644 --- 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 2bcd99b8bcdc6d..84a660658e3cb7 100644 --- 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, _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
