https://github.com/python/cpython/commit/0c080d7c77d826c1afab7bd6b73f61e714cffcb7
commit: 0c080d7c77d826c1afab7bd6b73f61e714cffcb7
branch: main
author: Sam Gross <colesb...@gmail.com>
committer: colesbury <colesb...@gmail.com>
date: 2024-09-06T15:07:08-04:00
summary:
gh-123321: Make Parser/myreadline.c locking safe in free-threaded build
(#123690)
Use a `PyMutex` to avoid the race in mutex initialization. Use relaxed
atomics to avoid the data race on reading `_PyOS_ReadlineTState` when
checking for re-entrant calls.
files:
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 7d07906df20cc1..50e77cbbb6be13 100644
--- a/Lib/test/test_readline.py
+++ b/Lib/test/test_readline.py
@@ -7,7 +7,7 @@
import tempfile
import textwrap
import unittest
-from test.support import requires_gil_enabled, verbose
+from test.support import 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
@@ -351,7 +351,6 @@ def test_history_size(self):
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"""
diff --git a/Parser/myreadline.c b/Parser/myreadline.c
index 6eab56ac52dc08..74c44ff77717f5 100644
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@@ -28,7 +28,7 @@
PyAPI_DATA(PyThreadState*) _PyOS_ReadlineTState;
PyThreadState *_PyOS_ReadlineTState = NULL;
-static PyThread_type_lock _PyOS_ReadlineLock = NULL;
+static PyMutex _PyOS_ReadlineLock;
int (*PyOS_InputHook)(void) = NULL;
@@ -373,34 +373,22 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const
char *prompt)
size_t len;
PyThreadState *tstate = _PyThreadState_GET();
- if (_PyOS_ReadlineTState == tstate) {
+ if (_Py_atomic_load_ptr_relaxed(&_PyOS_ReadlineTState) == tstate) {
PyErr_SetString(PyExc_RuntimeError,
"can't re-enter readline");
return NULL;
}
-
+ // GH-123321: We need to acquire the lock before setting
+ // _PyOS_ReadlineTState, otherwise the variable may be nullified by a
+ // different thread.
+ Py_BEGIN_ALLOW_THREADS
+ PyMutex_Lock(&_PyOS_ReadlineLock);
+ _Py_atomic_store_ptr_relaxed(&_PyOS_ReadlineTState, tstate);
if (PyOS_ReadlineFunctionPointer == NULL) {
PyOS_ReadlineFunctionPointer = PyOS_StdioReadline;
}
- if (_PyOS_ReadlineLock == NULL) {
- _PyOS_ReadlineLock = PyThread_allocate_lock();
- if (_PyOS_ReadlineLock == NULL) {
- PyErr_SetString(PyExc_MemoryError, "can't allocate lock");
- return NULL;
- }
- }
-
- 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
* a tty. This can happen, for example if python is run like
@@ -426,9 +414,8 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const char
*prompt)
// 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);
-
+ _Py_atomic_store_ptr_relaxed(&_PyOS_ReadlineTState, NULL);
+ PyMutex_Unlock(&_PyOS_ReadlineLock);
Py_END_ALLOW_THREADS
if (rv == NULL)
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
