Repository navigation
PyType_AddWatcher and friends are not thread safe in free-threading #131544
Copy link
Copy link
Closed
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-free-threadingtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
PyType_AddWatcher and PyType_ClearWatcher are not thread safe as it modifies the interp's type watchers non-atomically.
PyType_AddWatcher:
Lines 949 to 963 in d3f6063
| PyType_AddWatcher(PyType_WatchCallback callback) | |
| { | |
| PyInterpreterState *interp = _PyInterpreterState_GET(); | |
| // start at 1, 0 is reserved for cpython optimizer | |
| for (int i = 1; i < TYPE_MAX_WATCHERS; i++) { | |
| if (!interp->type_watchers[i]) { | |
| interp->type_watchers[i] = callback; | |
| return i; | |
| } | |
| } | |
| PyErr_SetString(PyExc_RuntimeError, "no more type watcher IDs available"); | |
| return -1; | |
| } |
PyType_ClearWatcher:
Lines 980 to 989 in d3f6063
| PyType_ClearWatcher(int watcher_id) | |
| { | |
| PyInterpreterState *interp = _PyInterpreterState_GET(); | |
| if (validate_watcher_id(interp, watcher_id) < 0) { | |
| return -1; | |
| } | |
| interp->type_watchers[watcher_id] = NULL; | |
| return 0; | |
| } | |
I think adding and removing of type watchers is a rare event so maybe instead of adding atomics or locks it would be better to change them to use stop-the-world pause event.
Linked PRs
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
interpreter-core(Objects, Python, Grammar, and Parser dirs)(Objects, Python, Grammar, and Parser dirs)topic-free-threadingtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Activity
colesbury commentedon Mar 21, 2025
I think it may be fine to just document that
PyType_AddWatcher()isn't thread-safe and should be called at start up.colesbury commentedon Mar 21, 2025
I'm pretty sure that Cinder does this already (@mpage ?) and I'm not sure there are any other current users of the API:
https://grep.app/search?q=PyType_AddWatcher
corona10 commentedon Mar 23, 2025
Hmm, I think that we can use CAS operation at
PyType_AddWatcherand use atomic store operation forPyType_ClearWatcherwill be fine if we consider that PyType_AddWatcher only 8.Is there any concerns to add extra atomic operations for those APIs at free-threading?
colesbury commentedon Mar 23, 2025
If you rely on atomics, you have to use them for every access, so also every read of
interp->type_watchers. This adds a bunch of complexity and I don't think there's a good reason to do it.pythongh-131544: Update docs for PyType_AddWatcher
Merge remote-tracking branch 'upstream/main' into pythongh-131544
gh-131544: Update docs for PyType_AddWatcher (gh-132015)
pythongh-131544: Update docs for PyType_AddWatcher (pythongh-132015)
[3.13] gh-131544: Update docs for PyType_AddWatcher (gh-132015) (gh-1…