gh-153291: Fix data race in readline.get_completer() and get_pre_input_hook() (gh...

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).

Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>

diff --git a/Lib/test/test_free_threading/test_readline.py b/Lib/test/test_free_threading/test_readline.py
new file mode 100644 (file)
index 0000000..f7aa989
--- /dev/null
+++ b/Lib/test/test_free_threading/test_readline.py
@@ -0,0 +1,59 @@
+import unittest
+
+from test.support import import_helper
+from test.support import threading_helper
+
+readline = import_helper.import_module("readline")
+
+
+@threading_helper.requires_working_threading()
+class TestReadlineRaces(unittest.TestCase):
+    def test_completer_delims_get_set(self):
+        def worker():
+            for _ in range(100):
+                readline.get_completer_delims()
+                readline.set_completer_delims(
+                    ' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
+                readline.set_completer_delims(
+                    ' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
+                readline.get_completer_delims()
+
+        threading_helper.run_concurrently(worker, nthreads=40)
+
+    # 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 97ca93ae09db343c5a0057203a56de0572a57037..11ec57a259a9abf04c9bba68ba99b7ba08b81d54 100644 (file)
--- a/Lib/test/test_readline.py
+++ b/Lib/test/test_readline.py
@@ -8,10 +8,7 @@ import sys
 import sysconfig
 import tempfile
 import textwrap
-import threading
 import unittest
-from test import support
-from test.support import threading_helper
 from test.support import verbose
 from test.support.import_helper import import_module
 from test.support.os_helper import unlink, temp_dir, TESTFN
@@ -445,26 +442,5 @@ readline.write_history_file(history_file)
         self.assertIs(readline.get_pre_input_hook(), my_hook)
 
 
-@unittest.skipUnless(support.Py_GIL_DISABLED, 'these tests can only possibly fail with GIL disabled')
-class FreeThreadingTest(unittest.TestCase):
-    @threading_helper.reap_threads
-    @threading_helper.requires_working_threading()
-    def test_free_threading(self):
-        def completer_delims(b):
-            b.wait()
-            for _ in range(100):
-                readline.get_completer_delims()
-                readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
-                readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
-                readline.get_completer_delims()
-
-        count   = 40
-        barrier = threading.Barrier(count)
-        threads = [threading.Thread(target=completer_delims, 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 (file)
index 0000000..dbe3cd4
--- /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 <free
+threading>` 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 dc9381e4b976acd3c0e10ba66a1b08b4ac73e7e9..a7df757f3f6c85c86293878d03e52a8bfd81693a 100644 (file)
--- 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 4c965e081c9d1a77ea1414142f154ac865c7568b..fc79a5866dfd38ca236cefbfa72f399c1861149c 100644 (file)
--- 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) {