Skip to content

Commit 221355c

Browse files
harjothkharanascheme
authored andcommitted
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). (cherry picked from commit 90b6a79) Co-authored-by: Harjoth Khara <harjoth.khara@gmail.com> Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
1 parent a5e2d74 commit 221355c

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
@@ -7,10 +7,7 @@
77
import sys
88
import tempfile
99
import textwrap
10-
import threading
1110
import unittest
12-
from test import support
13-
from test.support import threading_helper
1411
from test.support import verbose
1512
from test.support.import_helper import import_module
1613
from test.support.os_helper import unlink, temp_dir, TESTFN
@@ -432,26 +429,5 @@ def my_hook():
432429
self.assertIs(readline.get_pre_input_hook(), my_hook)
433430

434431

435-
@unittest.skipUnless(support.Py_GIL_DISABLED, 'these tests can only possibly fail with GIL disabled')
436-
class FreeThreadingTest(unittest.TestCase):
437-
@threading_helper.reap_threads
438-
@threading_helper.requires_working_threading()
439-
def test_free_threading(self):
440-
def completer_delims(b):
441-
b.wait()
442-
for _ in range(100):
443-
readline.get_completer_delims()
444-
readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
445-
readline.set_completer_delims(' \t\n`@#%^&*()=+[{]}\\|;:\'",<>?')
446-
readline.get_completer_delims()
447-
448-
count = 40
449-
barrier = threading.Barrier(count)
450-
threads = [threading.Thread(target=completer_delims, args=(barrier,)) for _ in range(count)]
451-
452-
with threading_helper.start_threads(threads):
453-
pass
454-
455-
456432
if __name__ == "__main__":
457433
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)