Skip to content

Commit 90b6a79

Browse files
gh-153291: Fix data race in readline.get_completer() and get_pre_input_hook() (gh-153362)
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>
1 parent 0e4940f commit 90b6a79

5 files changed

Lines changed: 82 additions & 29 deletions

File tree

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import unittest
2+
3+
from test.support import import_helper
4+
from test.support import threading_helper
5+
6+
readline = import_helper.import_module("readline")
7+
8+
9+
@threading_helper.requires_working_threading()
10+
class TestReadlineRaces(unittest.TestCase):
11+
def test_completer_delims_get_set(self):
12+
def worker():
13+
for _ in range(100):
14+
readline.get_completer_delims()
15+
readline.set_completer_delims(
16+
' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
17+
readline.set_completer_delims(
18+
' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
19+
readline.get_completer_delims()
20+
21+
threading_helper.run_concurrently(worker, nthreads=40)
22+
23+
# get_completer()/get_pre_input_hook() must take the module critical
24+
# section like their setters do; otherwise reading and Py_NewRef-ing the
25+
# stored hook races the setter replacing it (gh-153291).
26+
27+
def test_completer_get_set(self):
28+
def setter():
29+
for _ in range(1000):
30+
readline.set_completer(lambda text, state: None)
31+
readline.set_completer(None)
32+
33+
def getter():
34+
for _ in range(1000):
35+
readline.get_completer()
36+
37+
original = readline.get_completer()
38+
self.addCleanup(readline.set_completer, original)
39+
threading_helper.run_concurrently([setter] * 2 + [getter] * 6)
40+
41+
@unittest.skipUnless(hasattr(readline, "set_pre_input_hook"),
42+
"needs readline.set_pre_input_hook")
43+
def test_pre_input_hook_get_set(self):
44+
def setter():
45+
for _ in range(1000):
46+
readline.set_pre_input_hook(lambda: None)
47+
readline.set_pre_input_hook(None)
48+
49+
def getter():
50+
for _ in range(1000):
51+
readline.get_pre_input_hook()
52+
53+
original = readline.get_pre_input_hook()
54+
self.addCleanup(readline.set_pre_input_hook, original)
55+
threading_helper.run_concurrently([setter] * 2 + [getter] * 6)
56+
57+
58+
if __name__ == "__main__":
59+
unittest.main()

Lib/test/test_readline.py

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@
88
import sysconfig
99
import tempfile
1010
import textwrap
11-
import threading
1211
import unittest
13-
from test import support
14-
from test.support import threading_helper
1512
from test.support import verbose
1613
from test.support.import_helper import import_module
1714
from test.support.os_helper import unlink, temp_dir, TESTFN
@@ -445,26 +442,5 @@ def my_hook():
445442
self.assertIs(readline.get_pre_input_hook(), my_hook)
446443

447444

448-
@unittest.skipUnless(support.Py_GIL_DISABLED, 'these tests can only possibly fail with GIL disabled')
449-
class FreeThreadingTest(unittest.TestCase):
450-
@threading_helper.reap_threads
451-
@threading_helper.requires_working_threading()
452-
def test_free_threading(self):
453-
def completer_delims(b):
454-
b.wait()
455-
for _ in range(100):
456-
readline.get_completer_delims()
457-
readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
458-
readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
459-
readline.get_completer_delims()
460-
461-
count = 40
462-
barrier = threading.Barrier(count)
463-
threads = [threading.Thread(target=completer_delims, args=(barrier,)) for _ in range(count)]
464-
465-
with threading_helper.start_threads(threads):
466-
pass
467-
468-
469445
if __name__ == "__main__":
470446
unittest.main()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a data race in :func:`readline.get_completer` and
2+
:func:`readline.get_pre_input_hook` on the :term:`free-threaded <free
3+
threading>` build: the getters read the stored hook without the critical
4+
section that the corresponding setters hold.

Modules/clinic/readline.c.h

Lines changed: 15 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Modules/readline.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,14 +578,15 @@ readline_set_pre_input_hook_impl(PyObject *module, PyObject *function)
578578
/* Get pre-input hook */
579579

580580
/*[clinic input]
581+
@critical_section
581582
readline.get_pre_input_hook
582583
583584
Get the current pre-input hook function.
584585
[clinic start generated code]*/
585586

586587
static PyObject *
587588
readline_get_pre_input_hook_impl(PyObject *module)
588-
/*[clinic end generated code: output=ad56b77a8e8981ca input=fb1e1b1fbd94e4e5]*/
589+
/*[clinic end generated code: output=ad56b77a8e8981ca input=fbbf0106bd015414]*/
589590
{
590591
readlinestate *state = get_readline_state(module);
591592
if (state->pre_input_hook == NULL) {
@@ -886,14 +887,15 @@ readline_set_completer_impl(PyObject *module, PyObject *function)
886887
}
887888

888889
/*[clinic input]
890+
@critical_section
889891
readline.get_completer
890892
891893
Get the current completer function.
892894
[clinic start generated code]*/
893895

894896
static PyObject *
895897
readline_get_completer_impl(PyObject *module)
896-
/*[clinic end generated code: output=6e6bbd8226d14475 input=6457522e56d70d13]*/
898+
/*[clinic end generated code: output=6e6bbd8226d14475 input=0df9ba4107115c44]*/
897899
{
898900
readlinestate *state = get_readline_state(module);
899901
if (state->completer == NULL) {

0 commit comments

Comments
 (0)