https://github.com/python/cpython/commit/a4562fedadb73fe1e978dece65c3bcefb4606678
commit: a4562fedadb73fe1e978dece65c3bcefb4606678
branch: main
author: Bar Harel <[email protected]>
committer: vstinner <[email protected]>
date: 2024-09-04T17:21:30+02:00
summary:
gh-123321: Fix Parser/myreadline.c to prevent a segfault during a
multi-threaded race (#123323)
files:
A Misc/NEWS.d/next/Core and
Builtins/2024-08-26-00-58-26.gh-issue-123321.ApxcnE.rst
M Lib/test/test_readline.py
M Parser/myreadline.c
diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py
index 91fd7dd13f9063..7d07906df20cc1 100644
--- a/Lib/test/test_readline.py
+++ b/Lib/test/test_readline.py
@@ -7,11 +7,12 @@
import tempfile
import textwrap
import unittest
-from test.support import verbose
+from test.support import requires_gil_enabled, verbose
from test.support.import_helper import import_module
from test.support.os_helper import unlink, temp_dir, TESTFN
from test.support.pty_helper import run_pty
from test.support.script_helper import assert_python_ok
+from test.support.threading_helper import requires_working_threading
# Skip tests if there is no readline module
readline = import_module('readline')
@@ -349,6 +350,31 @@ def test_history_size(self):
self.assertEqual(len(lines), history_size)
self.assertEqual(lines[-1].strip(), b"last input")
+ @requires_working_threading()
+ @requires_gil_enabled()
+ def test_gh123321_threadsafe(self):
+ """gh-123321: readline should be thread-safe and not crash"""
+ script = textwrap.dedent(r"""
+ import threading
+ from test.support.threading_helper import join_thread
+
+ def func():
+ input()
+
+ thread1 = threading.Thread(target=func)
+ thread2 = threading.Thread(target=func)
+ thread1.start()
+ thread2.start()
+ join_thread(thread1)
+ join_thread(thread2)
+ print("done")
+ """)
+
+ output = run_pty(script, input=b"input1\rinput2\r")
+
+ self.assertIn(b"done", output)
+
+
def test_write_read_limited_history(self):
previous_length = readline.get_history_length()
self.addCleanup(readline.set_history_length, previous_length)
diff --git a/Misc/NEWS.d/next/Core and
Builtins/2024-08-26-00-58-26.gh-issue-123321.ApxcnE.rst b/Misc/NEWS.d/next/Core
and Builtins/2024-08-26-00-58-26.gh-issue-123321.ApxcnE.rst
new file mode 100644
index 00000000000000..b0547e0e588e3d
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and
Builtins/2024-08-26-00-58-26.gh-issue-123321.ApxcnE.rst
@@ -0,0 +1,2 @@
+Prevent Parser/myreadline race condition from segfaulting on multi-threaded
+use. Patch by Bar Harel and Amit Wienner.
diff --git a/Parser/myreadline.c b/Parser/myreadline.c
index 1825665354844b..6eab56ac52dc08 100644
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@@ -392,9 +392,14 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const
char *prompt)
}
}
- _PyOS_ReadlineTState = tstate;
Py_BEGIN_ALLOW_THREADS
+
+ // GH-123321: We need to acquire the lock before setting
+ // _PyOS_ReadlineTState and after the release of the GIL, otherwise
+ // the variable may be nullified by a different thread or a deadlock
+ // may occur if the GIL is taken in any sub-function.
PyThread_acquire_lock(_PyOS_ReadlineLock, 1);
+ _PyOS_ReadlineTState = tstate;
/* This is needed to handle the unlikely case that the
* interpreter is in interactive mode *and* stdin/out are not
@@ -418,11 +423,13 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const
char *prompt)
else {
rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout, prompt);
}
- Py_END_ALLOW_THREADS
+ // gh-123321: Must set the variable and then release the lock before
+ // taking the GIL. Otherwise a deadlock or segfault may occur.
+ _PyOS_ReadlineTState = NULL;
PyThread_release_lock(_PyOS_ReadlineLock);
- _PyOS_ReadlineTState = NULL;
+ Py_END_ALLOW_THREADS
if (rv == NULL)
return NULL;
_______________________________________________
Python-checkins mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: [email protected]
