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.

diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py
index 7d07906df20cc1bc02ab67517ebbc8045a8fca69..50e77cbbb6be13ad0a57b471d329b91ddc382199 100644 (file)
--- a/Lib/test/test_readline.py
+++ b/Lib/test/test_readline.py
@@ -7,7 +7,7 @@ import sys
 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 @@ readline.write_history_file(history_file)
             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 6eab56ac52dc0880b23e48e0612ffabd401487e7..74c44ff77717f588783360b09334240b35ee49df 100644 (file)
--- 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)