From 03624a45e5e964786c78c6bffd14774b953eb22c Mon Sep 17 00:00:00 2001 From: Alper Date: Mon, 11 May 2026 08:39:55 -0700 Subject: [PATCH 1/2] [3.14] gh-145235: Make dict watcher API thread-safe for free-threaded builds (gh-145233) In free-threaded builds, concurrent calls to PyDict_AddWatcher, PyDict_ClearWatcher, PyDict_Watch, and PyDict_Unwatch can race on the shared callback array and the per-dict watcher tags. This change adds a mutex to serialize watcher registration and removal, atomic operations for tag updates, and atomic acquire/release synchronization for callback dispatch in _PyDict_SendEvent. (cherry picked from commit 8a4895985f42282504d83b9bd0c77b129f95a5d5) Co-authored-by: Alper --- Include/internal/pycore_dict_state.h | 2 + .../internal/pycore_pyatomic_ft_wrappers.h | 2 + .../test_free_threading/test_dict_watcher.py | 89 +++++++++++++++++++ ...-02-25-13-37-10.gh-issue-145235.-1ySNR.rst | 3 + Modules/_testcapi/watchers.c | 26 +++--- Objects/dictobject.c | 42 ++++++--- Python/optimizer_analysis.c | 22 +++-- Python/pystate.c | 1 + Tools/c-analyzer/cpython/ignored.tsv | 1 + 9 files changed, 159 insertions(+), 29 deletions(-) create mode 100644 Lib/test/test_free_threading/test_dict_watcher.py create mode 100644 Misc/NEWS.d/next/C_API/2026-02-25-13-37-10.gh-issue-145235.-1ySNR.rst diff --git a/Include/internal/pycore_dict_state.h b/Include/internal/pycore_dict_state.h index 11932b8d1e1ab6..bb6fe262597559 100644 --- a/Include/internal/pycore_dict_state.h +++ b/Include/internal/pycore_dict_state.h @@ -13,6 +13,8 @@ extern "C" { struct _Py_dict_state { uint32_t next_keys_version; + PyMutex watcher_mutex; // Protects the watchers array (free-threaded builds) + _PyOnceFlag watcher_setup_once; // One-time optimizer watcher setup PyDict_WatchCallback watchers[DICT_MAX_WATCHERS]; }; diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index 31628528321e1b..bcb9e01a5c8e25 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -132,6 +132,7 @@ extern "C" { #define FT_ATOMIC_ADD_SSIZE(value, new_value) \ (void)_Py_atomic_add_ssize(&value, new_value) #define FT_MUTEX_LOCK(lock) PyMutex_Lock(lock) +#define FT_MUTEX_LOCK_FLAGS(lock, flags) PyMutex_LockFlags(lock, flags) #define FT_MUTEX_UNLOCK(lock) PyMutex_Unlock(lock) #else @@ -192,6 +193,7 @@ extern "C" { #define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_ADD_SSIZE(value, new_value) (void)(value += new_value) #define FT_MUTEX_LOCK(lock) do {} while (0) +#define FT_MUTEX_LOCK_FLAGS(lock, flags) do {} while (0) #define FT_MUTEX_UNLOCK(lock) do {} while (0) #endif diff --git a/Lib/test/test_free_threading/test_dict_watcher.py b/Lib/test/test_free_threading/test_dict_watcher.py new file mode 100644 index 00000000000000..6a6843f9344f64 --- /dev/null +++ b/Lib/test/test_free_threading/test_dict_watcher.py @@ -0,0 +1,89 @@ +import unittest + +from test.support import import_helper, threading_helper + +_testcapi = import_helper.import_module("_testcapi") + +ITERS = 100 +NTHREADS = 20 + + +@threading_helper.requires_working_threading() +class TestDictWatcherThreadSafety(unittest.TestCase): + # Watcher kinds from _testcapi + EVENTS = 0 # appends dict events as strings to global event list + + def test_concurrent_add_clear_watchers(self): + """Race AddWatcher and ClearWatcher from multiple threads. + + Uses more threads than available watcher slots (5 user slots out + of DICT_MAX_WATCHERS=8). + """ + results = [] + + def worker(): + for _ in range(ITERS): + try: + wid = _testcapi.add_dict_watcher(self.EVENTS) + except RuntimeError: + continue # All slots taken + self.assertGreaterEqual(wid, 0) + results.append(wid) + _testcapi.clear_dict_watcher(wid) + + threading_helper.run_concurrently(worker, NTHREADS) + + # Verify at least some watchers were successfully added + self.assertGreater(len(results), 0) + + def test_concurrent_watch_unwatch(self): + """Race Watch and Unwatch on the same dict from multiple threads.""" + wid = _testcapi.add_dict_watcher(self.EVENTS) + dicts = [{} for _ in range(10)] + + def worker(): + for _ in range(ITERS): + for d in dicts: + _testcapi.watch_dict(wid, d) + for d in dicts: + _testcapi.unwatch_dict(wid, d) + + try: + threading_helper.run_concurrently(worker, NTHREADS) + + # Verify watching still works after concurrent watch/unwatch + _testcapi.watch_dict(wid, dicts[0]) + dicts[0]["key"] = "value" + events = _testcapi.get_dict_watcher_events() + self.assertIn("new:key:value", events) + finally: + _testcapi.clear_dict_watcher(wid) + + def test_concurrent_modify_watched_dict(self): + """Race dict mutations (triggering callbacks) with watch/unwatch.""" + wid = _testcapi.add_dict_watcher(self.EVENTS) + d = {} + _testcapi.watch_dict(wid, d) + + def mutator(): + for i in range(ITERS): + d[f"key_{i}"] = i + d.pop(f"key_{i}", None) + + def toggler(): + for i in range(ITERS): + _testcapi.watch_dict(wid, d) + d[f"toggler_{i}"] = i + _testcapi.unwatch_dict(wid, d) + + workers = [mutator, toggler] * (NTHREADS // 2) + try: + threading_helper.run_concurrently(workers) + events = _testcapi.get_dict_watcher_events() + self.assertGreater(len(events), 0) + finally: + _testcapi.clear_dict_watcher(wid) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/C_API/2026-02-25-13-37-10.gh-issue-145235.-1ySNR.rst b/Misc/NEWS.d/next/C_API/2026-02-25-13-37-10.gh-issue-145235.-1ySNR.rst new file mode 100644 index 00000000000000..98a8c268735726 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-02-25-13-37-10.gh-issue-145235.-1ySNR.rst @@ -0,0 +1,3 @@ +Made :c:func:`PyDict_AddWatcher`, :c:func:`PyDict_ClearWatcher`, +:c:func:`PyDict_Watch`, and :c:func:`PyDict_Unwatch` thread-safe on the +:term:`free threaded ` build. diff --git a/Modules/_testcapi/watchers.c b/Modules/_testcapi/watchers.c index 5a756a87c15fe9..66fd2f1746831d 100644 --- a/Modules/_testcapi/watchers.c +++ b/Modules/_testcapi/watchers.c @@ -9,6 +9,7 @@ #include "pycore_function.h" // FUNC_MAX_WATCHERS #include "pycore_interp_structs.h" // CODE_MAX_WATCHERS #include "pycore_context.h" // CONTEXT_MAX_WATCHERS +#include "pycore_lock.h" // _PyOnceFlag /*[clinic input] module _testcapi @@ -18,6 +19,14 @@ module _testcapi // Test dict watching static PyObject *g_dict_watch_events = NULL; static int g_dict_watchers_installed = 0; +static _PyOnceFlag g_dict_watch_once = {0}; + +static int +_init_dict_watch_events(void *arg) +{ + g_dict_watch_events = PyList_New(0); + return g_dict_watch_events ? 0 : -1; +} static int dict_watch_callback(PyDict_WatchEvent event, @@ -106,13 +115,10 @@ add_dict_watcher(PyObject *self, PyObject *kind) if (watcher_id < 0) { return NULL; } - if (!g_dict_watchers_installed) { - assert(!g_dict_watch_events); - if (!(g_dict_watch_events = PyList_New(0))) { - return NULL; - } + if (_PyOnceFlag_CallOnce(&g_dict_watch_once, _init_dict_watch_events, NULL) < 0) { + return NULL; } - g_dict_watchers_installed++; + _Py_atomic_add_int(&g_dict_watchers_installed, 1); return PyLong_FromLong(watcher_id); } @@ -122,10 +128,8 @@ clear_dict_watcher(PyObject *self, PyObject *watcher_id) if (PyDict_ClearWatcher(PyLong_AsLong(watcher_id))) { return NULL; } - g_dict_watchers_installed--; - if (!g_dict_watchers_installed) { - assert(g_dict_watch_events); - Py_CLEAR(g_dict_watch_events); + if (_Py_atomic_add_int(&g_dict_watchers_installed, -1) == 1) { + PyList_Clear(g_dict_watch_events); } Py_RETURN_NONE; } @@ -164,7 +168,7 @@ _testcapi_unwatch_dict_impl(PyObject *module, int watcher_id, PyObject *dict) static PyObject * get_dict_watcher_events(PyObject *self, PyObject *Py_UNUSED(args)) { - if (!g_dict_watch_events) { + if (_Py_atomic_load_int(&g_dict_watchers_installed) <= 0) { PyErr_SetString(PyExc_RuntimeError, "no watchers active"); return NULL; } diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 79a73b5971af30..68f55893e5aaa0 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -7700,13 +7700,19 @@ validate_watcher_id(PyInterpreterState *interp, int watcher_id) PyErr_Format(PyExc_ValueError, "Invalid dict watcher ID %d", watcher_id); return -1; } - if (!interp->dict_state.watchers[watcher_id]) { + PyDict_WatchCallback cb = FT_ATOMIC_LOAD_PTR_RELAXED( + interp->dict_state.watchers[watcher_id]); + if (cb == NULL) { PyErr_Format(PyExc_ValueError, "No dict watcher set for ID %d", watcher_id); return -1; } return 0; } +// In free-threaded builds, Add/Clear serialize on watcher_mutex and publish +// callbacks with release stores. SendEvent reads them lock-free using +// acquire loads. + int PyDict_Watch(int watcher_id, PyObject* dict) { @@ -7718,7 +7724,8 @@ PyDict_Watch(int watcher_id, PyObject* dict) if (validate_watcher_id(interp, watcher_id)) { return -1; } - FT_ATOMIC_OR_UINT64(((PyDictObject*)dict)->_ma_watcher_tag, (1LL << watcher_id)); + FT_ATOMIC_OR_UINT64(((PyDictObject*)dict)->_ma_watcher_tag, + 1ULL << watcher_id); return 0; } @@ -7733,36 +7740,48 @@ PyDict_Unwatch(int watcher_id, PyObject* dict) if (validate_watcher_id(interp, watcher_id)) { return -1; } - FT_ATOMIC_AND_UINT64(((PyDictObject*)dict)->_ma_watcher_tag, ~(1LL << watcher_id)); + FT_ATOMIC_AND_UINT64(((PyDictObject*)dict)->_ma_watcher_tag, + ~(1ULL << watcher_id)); return 0; } int PyDict_AddWatcher(PyDict_WatchCallback callback) { + int watcher_id = -1; PyInterpreterState *interp = _PyInterpreterState_GET(); + FT_MUTEX_LOCK_FLAGS(&interp->dict_state.watcher_mutex, + _Py_LOCK_DONT_DETACH); /* Start at 2, as 0 and 1 are reserved for CPython */ for (int i = 2; i < DICT_MAX_WATCHERS; i++) { if (!interp->dict_state.watchers[i]) { - interp->dict_state.watchers[i] = callback; - return i; + FT_ATOMIC_STORE_PTR_RELEASE(interp->dict_state.watchers[i], callback); + watcher_id = i; + goto done; } } - PyErr_SetString(PyExc_RuntimeError, "no more dict watcher IDs available"); - return -1; +done: + FT_MUTEX_UNLOCK(&interp->dict_state.watcher_mutex); + return watcher_id; } int PyDict_ClearWatcher(int watcher_id) { + int res = 0; PyInterpreterState *interp = _PyInterpreterState_GET(); + FT_MUTEX_LOCK_FLAGS(&interp->dict_state.watcher_mutex, + _Py_LOCK_DONT_DETACH); if (validate_watcher_id(interp, watcher_id)) { - return -1; + res = -1; + goto done; } - interp->dict_state.watchers[watcher_id] = NULL; - return 0; + FT_ATOMIC_STORE_PTR_RELEASE(interp->dict_state.watchers[watcher_id], NULL); +done: + FT_MUTEX_UNLOCK(&interp->dict_state.watcher_mutex); + return res; } static const char * @@ -7787,7 +7806,8 @@ _PyDict_SendEvent(int watcher_bits, PyInterpreterState *interp = _PyInterpreterState_GET(); for (int i = 0; i < DICT_MAX_WATCHERS; i++) { if (watcher_bits & 1) { - PyDict_WatchCallback cb = interp->dict_state.watchers[i]; + PyDict_WatchCallback cb = FT_ATOMIC_LOAD_PTR_ACQUIRE( + interp->dict_state.watchers[i]); if (cb && (cb(event, (PyObject*)mp, key, value) < 0)) { // We don't want to resurrect the dict by potentially having an // unraisablehook keep a reference to it, so we don't pass the diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 84a660658e3cb7..5584ae67945971 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -18,6 +18,7 @@ #include "pycore_opcode_metadata.h" #include "pycore_opcode_utils.h" #include "pycore_pystate.h" // _PyInterpreterState_GET() +#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_* #include "pycore_uop_metadata.h" #include "pycore_long.h" #include "pycore_interpframe.h" // _PyFrame_GetCode @@ -62,7 +63,7 @@ static void increment_mutations(PyObject* dict) { assert(PyDict_CheckExact(dict)); PyDictObject *d = (PyDictObject *)dict; - FT_ATOMIC_ADD_UINT64(d->_ma_watcher_tag, (1 << DICT_MAX_WATCHERS)); + FT_ATOMIC_ADD_UINT64(d->_ma_watcher_tag, 1ULL << DICT_MAX_WATCHERS); } /* The first two dict watcher IDs are reserved for CPython, @@ -91,6 +92,17 @@ type_watcher_callback(PyTypeObject* type) return 0; } +static int +_setup_optimizer_watchers(void *Py_UNUSED(arg)) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + FT_ATOMIC_STORE_PTR_RELEASE( + interp->dict_state.watchers[GLOBALS_WATCHER_ID], + globals_watcher_callback); + interp->type_watchers[TYPE_WATCHER_ID] = type_watcher_callback; + return 0; +} + static PyObject * convert_global_to_const(_PyUOpInstruction *inst, PyObject *obj, bool pop) { @@ -172,12 +184,8 @@ remove_globals(_PyInterpreterFrame *frame, _PyUOpInstruction *buffer, uint32_t builtins_watched = 0; uint32_t globals_watched = 0; uint32_t prechecked_function_version = 0; - if (interp->dict_state.watchers[GLOBALS_WATCHER_ID] == NULL) { - interp->dict_state.watchers[GLOBALS_WATCHER_ID] = globals_watcher_callback; - } - if (interp->type_watchers[TYPE_WATCHER_ID] == NULL) { - interp->type_watchers[TYPE_WATCHER_ID] = type_watcher_callback; - } + _PyOnceFlag_CallOnce(&interp->dict_state.watcher_setup_once, + _setup_optimizer_watchers, NULL); for (int pc = 0; pc < buffer_size; pc++) { _PyUOpInstruction *inst = &buffer[pc]; int opcode = inst->opcode; diff --git a/Python/pystate.c b/Python/pystate.c index 2b67a40fe4f787..06672ceb2143c9 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -408,6 +408,7 @@ _Py_COMP_DIAG_POP &(runtime)->allocators.mutex, \ &(runtime)->_main_interpreter.types.mutex, \ &(runtime)->_main_interpreter.code_state.mutex, \ + &(runtime)->_main_interpreter.dict_state.watcher_mutex, \ } static void diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index ba9119a06d57e2..737924fd8581c0 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -454,6 +454,7 @@ Modules/_testcapi/object.c - MyObject_dealloc_called - Modules/_testcapi/object.c - MyType - Modules/_testcapi/structmember.c - test_structmembersType_OldAPI - Modules/_testcapi/watchers.c - g_dict_watch_events - +Modules/_testcapi/watchers.c - g_dict_watch_once - Modules/_testcapi/watchers.c - g_dict_watchers_installed - Modules/_testcapi/watchers.c - g_type_modified_events - Modules/_testcapi/watchers.c - g_type_watchers_installed - From 50e2ab0ff1af3efe09165db6af9c8a16c02c8e5a Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Mon, 11 May 2026 14:23:15 -0400 Subject: [PATCH 2/2] Regenerate abidump --- Doc/data/python3.14.abi | 624 ++++++++++++++++++++-------------------- 1 file changed, 315 insertions(+), 309 deletions(-) diff --git a/Doc/data/python3.14.abi b/Doc/data/python3.14.abi index 09ef37cf4fa434..52432e8ea6ef04 100644 --- a/Doc/data/python3.14.abi +++ b/Doc/data/python3.14.abi @@ -7318,30 +7318,30 @@ - - - + + + - - - + + + - - + + - - + + - - - - - - + + + + + + @@ -17923,8 +17923,14 @@ + + + + + + - + @@ -21077,318 +21083,318 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -21400,385 +21406,385 @@ - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - - + + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -25190,7 +25196,7 @@ - + @@ -30758,97 +30764,97 @@ - + - - + + - - + + - - + + - - + + - - + + - - + + - - + + - - - + + + - - + + - - + + - - + + - + - - + + - - + + - - - + + + - + - + - - + + - - + + - + - + - - + + - - - + + +