From 704749cb4984752b26d9776779758d0867336811 Mon Sep 17 00:00:00 2001 From: harjoth Date: Wed, 8 Jul 2026 08:31:27 -0700 Subject: [PATCH 1/2] gh-153291: Fix data race in readline.get_completer() and get_pre_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 (gh-126895). --- Lib/test/test_readline.py | 38 +++++++++++++++++++ ...-07-08-00-00-00.gh-issue-153291.rdlnCS.rst | 4 ++ Modules/clinic/readline.c.h | 18 +++++++-- Modules/readline.c | 6 ++- 4 files changed, 61 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-08-00-00-00.gh-issue-153291.rdlnCS.rst diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 3982686dd10aecf..3cd2103f702dcf2 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -452,6 +452,44 @@ def completer_delims(b): with threading_helper.start_threads(threads): pass + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_free_threading_get_completer(self): + def completer(b): + b.wait() + for _ in range(100): + readline.get_completer() + readline.set_completer(lambda text, state: None) + readline.set_completer(None) + readline.get_completer() + + count = 40 + barrier = threading.Barrier(count) + threads = [threading.Thread(target=completer, args=(barrier,)) for _ in range(count)] + + with threading_helper.start_threads(threads): + pass + + @unittest.skipUnless(hasattr(readline, "get_pre_input_hook"), + "get_pre_input_hook not available") + @threading_helper.reap_threads + @threading_helper.requires_working_threading() + def test_free_threading_get_pre_input_hook(self): + def pre_input_hook(b): + b.wait() + for _ in range(100): + readline.get_pre_input_hook() + readline.set_pre_input_hook(lambda: None) + readline.set_pre_input_hook(None) + readline.get_pre_input_hook() + + count = 40 + barrier = threading.Barrier(count) + threads = [threading.Thread(target=pre_input_hook, args=(barrier,)) for _ in range(count)] + + with threading_helper.start_threads(threads): + pass + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Library/2026-07-08-00-00-00.gh-issue-153291.rdlnCS.rst b/Misc/NEWS.d/next/Library/2026-07-08-00-00-00.gh-issue-153291.rdlnCS.rst new file mode 100644 index 000000000000000..dbe3cd417e0d607 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-08-00-00-00.gh-issue-153291.rdlnCS.rst @@ -0,0 +1,4 @@ +Fix a data race in :func:`readline.get_completer` and +:func:`readline.get_pre_input_hook` on the :term:`free-threaded ` build: the getters read the stored hook without the critical +section that the corresponding setters hold. diff --git a/Modules/clinic/readline.c.h b/Modules/clinic/readline.c.h index dc9381e4b976acd..a7df757f3f6c85c 100644 --- a/Modules/clinic/readline.c.h +++ b/Modules/clinic/readline.c.h @@ -366,7 +366,13 @@ readline_get_pre_input_hook_impl(PyObject *module); static PyObject * readline_get_pre_input_hook(PyObject *module, PyObject *Py_UNUSED(ignored)) { - return readline_get_pre_input_hook_impl(module); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(module); + return_value = readline_get_pre_input_hook_impl(module); + Py_END_CRITICAL_SECTION(); + + return return_value; } #endif /* defined(HAVE_RL_PRE_INPUT_HOOK) */ @@ -651,7 +657,13 @@ readline_get_completer_impl(PyObject *module); static PyObject * readline_get_completer(PyObject *module, PyObject *Py_UNUSED(ignored)) { - return readline_get_completer_impl(module); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(module); + return_value = readline_get_completer_impl(module); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(readline_get_history_item__doc__, @@ -823,4 +835,4 @@ readline_redisplay(PyObject *module, PyObject *Py_UNUSED(ignored)) #ifndef READLINE_CLEAR_HISTORY_METHODDEF #define READLINE_CLEAR_HISTORY_METHODDEF #endif /* !defined(READLINE_CLEAR_HISTORY_METHODDEF) */ -/*[clinic end generated code: output=4bd95070973cd0e2 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=acf4e4c35191cf09 input=a9049054013a1b77]*/ diff --git a/Modules/readline.c b/Modules/readline.c index c580d2022fccf3d..9d4904d0085b8e5 100644 --- a/Modules/readline.c +++ b/Modules/readline.c @@ -578,6 +578,7 @@ readline_set_pre_input_hook_impl(PyObject *module, PyObject *function) /* Get pre-input hook */ /*[clinic input] +@critical_section readline.get_pre_input_hook Get the current pre-input hook function. @@ -585,7 +586,7 @@ Get the current pre-input hook function. static PyObject * readline_get_pre_input_hook_impl(PyObject *module) -/*[clinic end generated code: output=ad56b77a8e8981ca input=fb1e1b1fbd94e4e5]*/ +/*[clinic end generated code: output=ad56b77a8e8981ca input=fbbf0106bd015414]*/ { readlinestate *state = get_readline_state(module); if (state->pre_input_hook == NULL) { @@ -886,6 +887,7 @@ readline_set_completer_impl(PyObject *module, PyObject *function) } /*[clinic input] +@critical_section readline.get_completer Get the current completer function. @@ -893,7 +895,7 @@ Get the current completer function. static PyObject * readline_get_completer_impl(PyObject *module) -/*[clinic end generated code: output=6e6bbd8226d14475 input=6457522e56d70d13]*/ +/*[clinic end generated code: output=6e6bbd8226d14475 input=0df9ba4107115c44]*/ { readlinestate *state = get_readline_state(module); if (state->completer == NULL) { From f86b02900dd1d377b101bc494b3816b7d8300cde Mon Sep 17 00:00:00 2001 From: harjoth Date: Wed, 8 Jul 2026 19:31:01 -0700 Subject: [PATCH 2/2] gh-153291: Move the free-threaded readline tests to test_free_threading The Argument Clinic getters are exercised there under the TSan CI job (test_free_threading is in TSAN_TESTS; test_readline is not). --- Lib/test/test_free_threading/test_readline.py | 47 +++++++++++++++++++ Lib/test/test_readline.py | 38 --------------- 2 files changed, 47 insertions(+), 38 deletions(-) create mode 100644 Lib/test/test_free_threading/test_readline.py diff --git a/Lib/test/test_free_threading/test_readline.py b/Lib/test/test_free_threading/test_readline.py new file mode 100644 index 000000000000000..d3ddeaad80cb582 --- /dev/null +++ b/Lib/test/test_free_threading/test_readline.py @@ -0,0 +1,47 @@ +import unittest + +from test.support import threading_helper +from test.support import import_helper + +readline = import_helper.import_module("readline") + + +@threading_helper.requires_working_threading() +class TestReadlineRaces(unittest.TestCase): + # get_completer()/get_pre_input_hook() must take the module critical + # section like their setters do; otherwise reading and Py_NewRef-ing the + # stored hook races the setter replacing it (gh-153291). + + def test_completer_get_set(self): + def setter(): + for _ in range(1000): + readline.set_completer(lambda text, state: None) + readline.set_completer(None) + + def getter(): + for _ in range(1000): + readline.get_completer() + + original = readline.get_completer() + self.addCleanup(readline.set_completer, original) + threading_helper.run_concurrently([setter] * 2 + [getter] * 6) + + @unittest.skipUnless(hasattr(readline, "set_pre_input_hook"), + "needs readline.set_pre_input_hook") + def test_pre_input_hook_get_set(self): + def setter(): + for _ in range(1000): + readline.set_pre_input_hook(lambda: None) + readline.set_pre_input_hook(None) + + def getter(): + for _ in range(1000): + readline.get_pre_input_hook() + + original = readline.get_pre_input_hook() + self.addCleanup(readline.set_pre_input_hook, original) + threading_helper.run_concurrently([setter] * 2 + [getter] * 6) + + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 3cd2103f702dcf2..3982686dd10aecf 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -452,44 +452,6 @@ def completer_delims(b): with threading_helper.start_threads(threads): pass - @threading_helper.reap_threads - @threading_helper.requires_working_threading() - def test_free_threading_get_completer(self): - def completer(b): - b.wait() - for _ in range(100): - readline.get_completer() - readline.set_completer(lambda text, state: None) - readline.set_completer(None) - readline.get_completer() - - count = 40 - barrier = threading.Barrier(count) - threads = [threading.Thread(target=completer, args=(barrier,)) for _ in range(count)] - - with threading_helper.start_threads(threads): - pass - - @unittest.skipUnless(hasattr(readline, "get_pre_input_hook"), - "get_pre_input_hook not available") - @threading_helper.reap_threads - @threading_helper.requires_working_threading() - def test_free_threading_get_pre_input_hook(self): - def pre_input_hook(b): - b.wait() - for _ in range(100): - readline.get_pre_input_hook() - readline.set_pre_input_hook(lambda: None) - readline.set_pre_input_hook(None) - readline.get_pre_input_hook() - - count = 40 - barrier = threading.Barrier(count) - threads = [threading.Thread(target=pre_input_hook, args=(barrier,)) for _ in range(count)] - - with threading_helper.start_threads(threads): - pass - if __name__ == "__main__": unittest.main()