Skip to content

gh-145235: Make dict watcher API thread-safe for free-threaded builds#145233

Open
yoney wants to merge 3 commits intopython:mainfrom
yoney:watcher
Open

gh-145235: Make dict watcher API thread-safe for free-threaded builds#145233
yoney wants to merge 3 commits intopython:mainfrom
yoney:watcher

Conversation

@yoney
Copy link
Contributor

@yoney yoney commented Feb 25, 2026

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.

Note: Before this change, running test.test_free_threading.test_dict_watcher emitted tsan warnings in the free-threaded tsan build.

cc: @colesbury @mpage @DinoV

@yoney yoney changed the title Make dict watcher API thread-safe for free-threaded builds gh-145235: Make dict watcher API thread-safe for free-threaded builds Feb 25, 2026
PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->dict_state.watchers[GLOBALS_WATCHER_ID] == NULL) {
interp->dict_state.watchers[GLOBALS_WATCHER_ID] = globals_watcher_callback;
if (FT_ATOMIC_LOAD_PTR_RELAXED(interp->dict_state.watchers[GLOBALS_WATCHER_ID]) == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A _PyOnceFlag (maybe in JitOptContext) might be a better fit here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking into it (with Claude), JitOptContext seems to be per-optimization-run so _PyOnceFlag might not the right place. I put it in _Py_dict_state as I couldn't find a better place. It fits in the struct packing without extra space.

@yoney
Copy link
Contributor Author

yoney commented Feb 26, 2026

@colesbury Thank you for the review. I’ve addressed the comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants