Mirror
/
cpython
mirror of https://github.com/python/cpython
4
1
Fork 0
Code Issues Releases Wiki Activity

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.
This commit is contained in:
Sam Gross 2024-09-06 15:07:08 -04:00 committed by GitHub
parent 8a46a2ec50
commit 0c080d7c77
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 11 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
Lib/test/test_readline.py
View File

@ -7,7 +7,7 @@ import sys
import tempfile import tempfile
import textwrap import textwrap
import unittest 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.import_helper import import_module
from test.support.os_helper import unlink, temp_dir, TESTFN from test.support.os_helper import unlink, temp_dir, TESTFN
from test.support.pty_helper import run_pty from test.support.pty_helper import run_pty
@ -351,7 +351,6 @@ readline.write_history_file(history_file)
self.assertEqual(lines[-1].strip(), b"last input") self.assertEqual(lines[-1].strip(), b"last input")
@requires_working_threading() @requires_working_threading()
@requires_gil_enabled()
def test_gh123321_threadsafe(self): def test_gh123321_threadsafe(self):
"""gh-123321: readline should be thread-safe and not crash""" """gh-123321: readline should be thread-safe and not crash"""
script = textwrap.dedent(r""" script = textwrap.dedent(r"""

33
Parser/myreadline.c
View File

@ -28,7 +28,7 @@
PyAPI_DATA(PyThreadState*) _PyOS_ReadlineTState; PyAPI_DATA(PyThreadState*) _PyOS_ReadlineTState;
PyThreadState *_PyOS_ReadlineTState = NULL; PyThreadState *_PyOS_ReadlineTState = NULL;
static PyThread_type_lock _PyOS_ReadlineLock = NULL; static PyMutex _PyOS_ReadlineLock;
int (*PyOS_InputHook)(void) = NULL; int (*PyOS_InputHook)(void) = NULL;
@ -373,34 +373,22 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
size_t len; size_t len;
PyThreadState *tstate = _PyThreadState_GET(); PyThreadState *tstate = _PyThreadState_GET();
if (_PyOS_ReadlineTState == tstate) { if (_Py_atomic_load_ptr_relaxed(&_PyOS_ReadlineTState) == tstate) {
PyErr_SetString(PyExc_RuntimeError, PyErr_SetString(PyExc_RuntimeError,
"can't re-enter readline"); "can't re-enter readline");
return NULL; 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) { if (PyOS_ReadlineFunctionPointer == NULL) {
PyOS_ReadlineFunctionPointer = PyOS_StdioReadline; 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 /* This is needed to handle the unlikely case that the
* interpreter is in interactive mode *and* stdin/out are not * interpreter is in interactive mode *and* stdin/out are not
* a tty. This can happen, for example if python is run like * 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 // gh-123321: Must set the variable and then release the lock before
// taking the GIL. Otherwise a deadlock or segfault may occur. // taking the GIL. Otherwise a deadlock or segfault may occur.
_PyOS_ReadlineTState = NULL; _Py_atomic_store_ptr_relaxed(&_PyOS_ReadlineTState, NULL);
PyThread_release_lock(_PyOS_ReadlineLock); PyMutex_Unlock(&_PyOS_ReadlineLock);
Py_END_ALLOW_THREADS Py_END_ALLOW_THREADS
if (rv == NULL) if (rv == NULL)