From 562ff730f3532489813289c50f2671bcd7f4c7a7 Mon Sep 17 00:00:00 2001
From: "Miss Islington (bot)"
 <31488909+miss-islington@users.noreply.github.com>
Date: Thu, 5 Sep 2024 14:25:38 +0200
Subject: [PATCH] [3.12] gh-123321: Fix Parser/myreadline.c to prevent a
 segfault during a multi-threaded race (GH-123323) (#123677)

* gh-123321: Fix Parser/myreadline.c to prevent a segfault during a multi-threaded race (GH-123323)
(cherry picked from commit a4562fedadb73fe1e978dece65c3bcefb4606678)

Co-authored-by: Bar Harel <bharel@barharel.com>

* Remove @requires_gil_enabled for 3.12

---------

Co-authored-by: Bar Harel <bharel@barharel.com>
Co-authored-by: Sam Gross <colesbury@gmail.com>
---
 Lib/test/test_readline.py                     | 25 +++++++++++++++++++
 ...-08-26-00-58-26.gh-issue-123321.ApxcnE.rst |  2 ++
 Parser/myreadline.c                           | 13 +++++++---
 3 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-08-26-00-58-26.gh-issue-123321.ApxcnE.rst

diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py
index 58cc6b7e7f4803..fab124ae4ad45c 100644
--- a/Lib/test/test_readline.py
+++ b/Lib/test/test_readline.py
@@ -12,6 +12,7 @@
 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')
@@ -346,6 +347,30 @@ def test_history_size(self):
             self.assertEqual(len(lines), history_size)
             self.assertEqual(lines[-1].strip(), b"last input")
 
+    @requires_working_threading()
+    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 7074aba74b728c..2890ff83f3f64b 100644
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@@ -386,9 +386,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
@@ -412,11 +417,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;