gh-153291: Fix data race in readline.get_completer() and get_pre_input_hook()#153362
Open
harjothkhara wants to merge 1 commit into
Open
gh-153291: Fix data race in readline.get_completer() and get_pre_input_hook()#153362harjothkhara wants to merge 1 commit into
harjothkhara wants to merge 1 commit into
Conversation
…e_input_hook() The setters store these hooks while holding the module critical section (via set_hook's Py_XSETREF), but the getters read and Py_NewRef the same fields without it. Annotate both getters with @critical_section, matching the other readline functions (pythongh-126895).
brijkapadia
reviewed
Jul 9, 2026
| readline.set_pre_input_hook(my_hook) | ||
| self.assertIs(readline.get_pre_input_hook(), my_hook) | ||
|
|
||
| def test_get_completer(self): |
Contributor
There was a problem hiding this comment.
Is this test necessary? It didn't fail on main for me.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
readline.get_completer()readsstate->completerand returns a new reference without taking the module critical section, whileset_completer()swaps the same field under the critical section withPy_XSETREF. On the free-threaded build a concurrent getter can be incrementing the refcount of a hook the setter is simultaneously dropping.get_pre_input_hook()has the same problem withstate->pre_input_hook— those are the only two hook fields with both aPy_XSETREF-based setter and a Python-level getter, so this covers the whole Python-visible getter surface.The fix adds
@critical_sectionto both getters, the same way gh-126895 did for the rest of the module.Verified with a free-threaded TSAN build (
--disable-gil --with-thread-sanitizer): the reproducer from the issue reports the race on main (thereadline_get_completerread vs theset_hookwrite) and runs clean with this change. The new tests in test_readline.py trigger the same TSAN reports without the fix and pass with it.Deliberately not included here:
get_begidx()/get_endidx()and the C callback paths (on_completion()etc.) read module state without the critical section too, but their writer/reader sides live in readline callbacks (flex_complete(),on_hook()), where taking the module critical section is a design question (it would serialize completion callbacks against all other readline calls) — that's a follow-up, not a one-line annotation.I used AI assistance while working on this change; I've reviewed it and can explain it.
readline.get_completer()readsstate->completerwithout the critical section #153291