[3.9] bpo-40826: Fix GIL usage in PyOS_Readline() (GH-20613)

* bpo-40826: Fix GIL usage in PyOS_Readline() (GH-20579)

Fix GIL usage in PyOS_Readline(): lock the GIL to set an exception.

Pass tstate to my_fgets() and _PyOS_WindowsConsoleReadline(). Cleanup
these functions.

(cherry picked from commit c353764fd5)

* bpo-40826: Add _PyOS_InterruptOccurred(tstate) function (GH-20599)

my_fgets() now calls _PyOS_InterruptOccurred(tstate) to check for
pending signals, rather calling PyOS_InterruptOccurred().

my_fgets() is called with the GIL released, whereas
PyOS_InterruptOccurred() must be called with the GIL held.

test_repl: use text=True and avoid SuppressCrashReport in
test_multiline_string_parsing().

Fix my_fgets() on Windows: fgets(fp) does crash if fileno(fp) is closed.

(cherry picked from commit fa7ab6aa0f)

Include/internal/pycore_pystate.h

@@ -112,6 +112,9 @@ PyAPI_FUNC(int) _PyState_AddModule(
     PyObject* module,
     struct PyModuleDef* def);
 
+
+PyAPI_FUNC(int) _PyOS_InterruptOccurred(PyThreadState *tstate);
+
 #ifdef __cplusplus
 }
 #endif

Lib/test/test_repl.py

@@ -29,7 +29,9 @@ def spawn_repl(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, **kw):
     # test.support.script_helper.
     env = kw.setdefault('env', dict(os.environ))
     env['TERM'] = 'vt100'
-    return subprocess.Popen(cmd_line, executable=sys.executable,
+    return subprocess.Popen(cmd_line,
+                            executable=sys.executable,
+                            text=True,
                             stdin=subprocess.PIPE,
                             stdout=stdout, stderr=stderr,
                             **kw)
@@ -49,12 +51,11 @@ class TestInteractiveInterpreter(unittest.TestCase):
             sys.exit(0)
         """
         user_input = dedent(user_input)
-        user_input = user_input.encode()
         p = spawn_repl()
         with SuppressCrashReport():
             p.stdin.write(user_input)
         output = kill_python(p)
-        self.assertIn(b'After the exception.', output)
+        self.assertIn('After the exception.', output)
         # Exit code 120: Py_FinalizeEx() failed to flush stdout and stderr.
         self.assertIn(p.returncode, (1, 120))
 
@@ -86,13 +87,22 @@ class TestInteractiveInterpreter(unittest.TestCase):
         </test>"""
         '''
         user_input = dedent(user_input)
-        user_input = user_input.encode()
         p = spawn_repl()
-        with SuppressCrashReport():
+        p.stdin.write(user_input)
-            p.stdin.write(user_input)
         output = kill_python(p)
         self.assertEqual(p.returncode, 0)
 
+    def test_close_stdin(self):
+        user_input = dedent('''
+            import os
+            print("before close")
+            os.close(0)
+        ''')
+        process = spawn_repl()
+        output = process.communicate(user_input)[0]
+        self.assertEqual(process.returncode, 0)
+        self.assertIn('before close', output)
+
 
 if __name__ == "__main__":
     unittest.main()

Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst

@@ -0,0 +1,2 @@
+Fix GIL usage in :c:func:`PyOS_Readline`: lock the GIL to set an exception
+and pass the Python thread state when checking if there is a pending signal.

Modules/signalmodule.c

@@ -1779,11 +1779,13 @@ PyOS_FiniInterrupts(void)
     finisignal();
 }
 
+
+// The caller doesn't have to hold the GIL
 int
-PyOS_InterruptOccurred(void)
+_PyOS_InterruptOccurred(PyThreadState *tstate)
 {
-    PyInterpreterState *interp = _PyInterpreterState_GET();
+    assert(tstate != NULL);
-    if (!_Py_ThreadCanHandleSignals(interp)) {
+    if (!_Py_ThreadCanHandleSignals(tstate->interp)) {
         return 0;
     }
 
@@ -1795,6 +1797,16 @@ PyOS_InterruptOccurred(void)
     return 1;
 }
 
+
+// The caller must to hold the GIL
+int
+PyOS_InterruptOccurred(void)
+{
+    PyThreadState *tstate = _PyThreadState_GET();
+    return _PyOS_InterruptOccurred(tstate);
+}
+
+
 static void
 _clear_pending_signals(void)
 {

Parser/myreadline.c

@@ -24,25 +24,36 @@ static PyThread_type_lock _PyOS_ReadlineLock = NULL;
 int (*PyOS_InputHook)(void) = NULL;
 
 /* This function restarts a fgets() after an EINTR error occurred
-   except if PyOS_InterruptOccurred() returns true. */
+   except if _PyOS_InterruptOccurred() returns true. */
 
 static int
-my_fgets(char *buf, int len, FILE *fp)
+my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp)
 {
 #ifdef MS_WINDOWS
-    HANDLE hInterruptEvent;
+    HANDLE handle;
+    _Py_BEGIN_SUPPRESS_IPH
+    handle = (HANDLE)_get_osfhandle(fileno(fp));
+    _Py_END_SUPPRESS_IPH
+
+    /* bpo-40826: fgets(fp) does crash if fileno(fp) is closed */
+    if (handle == INVALID_HANDLE_VALUE) {
+        return -1; /* EOF */
+    }
 #endif
-    char *p;
+
-    int err;
     while (1) {
-        if (PyOS_InputHook != NULL)
+        if (PyOS_InputHook != NULL) {
             (void)(PyOS_InputHook)();
+        }
+
         errno = 0;
         clearerr(fp);
-        p = fgets(buf, len, fp);
+        char *p = fgets(buf, len, fp);
-        if (p != NULL)
+        if (p != NULL) {
             return 0; /* No error */
-        err = errno;
+        }
+        int err = errno;
+
 #ifdef MS_WINDOWS
         /* Ctrl-C anywhere on the line or Ctrl-Z if the only character
            on a line will set ERROR_OPERATION_ABORTED. Under normal
@@ -58,7 +69,7 @@ my_fgets(char *buf, int len, FILE *fp)
            through to check for EOF.
         */
         if (GetLastError()==ERROR_OPERATION_ABORTED) {
-            hInterruptEvent = _PyOS_SigintEvent();
+            HANDLE hInterruptEvent = _PyOS_SigintEvent();
             switch (WaitForSingleObjectEx(hInterruptEvent, 10, FALSE)) {
             case WAIT_OBJECT_0:
                 ResetEvent(hInterruptEvent);
@@ -68,23 +79,27 @@ my_fgets(char *buf, int len, FILE *fp)
             }
         }
 #endif /* MS_WINDOWS */
+
         if (feof(fp)) {
             clearerr(fp);
             return -1; /* EOF */
         }
+
 #ifdef EINTR
         if (err == EINTR) {
-            int s;
+            PyEval_RestoreThread(tstate);
-            PyEval_RestoreThread(_PyOS_ReadlineTState);
+            int s = PyErr_CheckSignals();
-            s = PyErr_CheckSignals();
             PyEval_SaveThread();
-            if (s < 0)
+
-                    return 1;
+            if (s < 0) {
-        /* try again */
+                return 1;
+            }
+            /* try again */
             continue;
         }
 #endif
-        if (PyOS_InterruptOccurred()) {
+
+        if (_PyOS_InterruptOccurred(tstate)) {
             return 1; /* Interrupt */
         }
         return -2; /* Error */
@@ -98,7 +113,7 @@ my_fgets(char *buf, int len, FILE *fp)
 extern char _get_console_type(HANDLE handle);
 
 char *
-_PyOS_WindowsConsoleReadline(HANDLE hStdIn)
+_PyOS_WindowsConsoleReadline(PyThreadState *tstate, HANDLE hStdIn)
 {
     static wchar_t wbuf_local[1024 * 16];
     const DWORD chunk_size = 1024;
@@ -133,11 +148,12 @@ _PyOS_WindowsConsoleReadline(HANDLE hStdIn)
             if (WaitForSingleObjectEx(hInterruptEvent, 100, FALSE)
                     == WAIT_OBJECT_0) {
                 ResetEvent(hInterruptEvent);
-                PyEval_RestoreThread(_PyOS_ReadlineTState);
+                PyEval_RestoreThread(tstate);
                 s = PyErr_CheckSignals();
                 PyEval_SaveThread();
-                if (s < 0)
+                if (s < 0) {
                     goto exit;
+                }
             }
             break;
         }
@@ -150,17 +166,22 @@ _PyOS_WindowsConsoleReadline(HANDLE hStdIn)
         if (wbuf == wbuf_local) {
             wbuf[total_read] = '\0';
             wbuf = (wchar_t*)PyMem_RawMalloc(wbuflen * sizeof(wchar_t));
-            if (wbuf)
+            if (wbuf) {
                 wcscpy_s(wbuf, wbuflen, wbuf_local);
+            }
             else {
+                PyEval_RestoreThread(tstate);
                 PyErr_NoMemory();
+                PyEval_SaveThread();
                 goto exit;
             }
         }
         else {
             wchar_t *tmp = PyMem_RawRealloc(wbuf, wbuflen * sizeof(wchar_t));
             if (tmp == NULL) {
+                PyEval_RestoreThread(tstate);
                 PyErr_NoMemory();
+                PyEval_SaveThread();
                 goto exit;
             }
             wbuf = tmp;
@@ -169,33 +190,45 @@ _PyOS_WindowsConsoleReadline(HANDLE hStdIn)
 
     if (wbuf[0] == '\x1a') {
         buf = PyMem_RawMalloc(1);
-        if (buf)
+        if (buf) {
             buf[0] = '\0';
+        }
         else {
+            PyEval_RestoreThread(tstate);
             PyErr_NoMemory();
+            PyEval_SaveThread();
         }
         goto exit;
     }
 
-    u8len = WideCharToMultiByte(CP_UTF8, 0, wbuf, total_read, NULL, 0, NULL, NULL);
+    u8len = WideCharToMultiByte(CP_UTF8, 0,
+                                wbuf, total_read,
+                                NULL, 0,
+                                NULL, NULL);
     buf = PyMem_RawMalloc(u8len + 1);
     if (buf == NULL) {
+        PyEval_RestoreThread(tstate);
         PyErr_NoMemory();
+        PyEval_SaveThread();
         goto exit;
     }
-    u8len = WideCharToMultiByte(CP_UTF8, 0, wbuf, total_read, buf, u8len, NULL, NULL);
+
+    u8len = WideCharToMultiByte(CP_UTF8, 0,
+                                wbuf, total_read,
+                                buf, u8len,
+                                NULL, NULL);
     buf[u8len] = '\0';
 
 exit:
-    if (wbuf != wbuf_local)
+    if (wbuf != wbuf_local) {
         PyMem_RawFree(wbuf);
+    }
 
     if (err) {
-        PyEval_RestoreThread(_PyOS_ReadlineTState);
+        PyEval_RestoreThread(tstate);
         PyErr_SetFromWindowsErr(err);
         PyEval_SaveThread();
     }
-
     return buf;
 }
 
@@ -209,6 +242,8 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
 {
     size_t n;
     char *p, *pr;
+    PyThreadState *tstate = _PyOS_ReadlineTState;
+    assert(tstate != NULL);
 
 #ifdef MS_WINDOWS
     if (!Py_LegacyWindowsStdioFlag && sys_stdin == stdin) {
@@ -230,7 +265,9 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
                     if (wlen) {
                         wbuf = PyMem_RawMalloc(wlen * sizeof(wchar_t));
                         if (wbuf == NULL) {
+                            PyEval_RestoreThread(tstate);
                             PyErr_NoMemory();
+                            PyEval_SaveThread();
                             return NULL;
                         }
                         wlen = MultiByteToWideChar(CP_UTF8, 0, prompt, -1,
@@ -249,7 +286,7 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
                 }
             }
             clearerr(sys_stdin);
-            return _PyOS_WindowsConsoleReadline(hStdIn);
+            return _PyOS_WindowsConsoleReadline(tstate, hStdIn);
         }
     }
 #endif
@@ -257,16 +294,19 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
     n = 100;
     p = (char *)PyMem_RawMalloc(n);
     if (p == NULL) {
+        PyEval_RestoreThread(tstate);
         PyErr_NoMemory();
+        PyEval_SaveThread();
         return NULL;
     }
 
     fflush(sys_stdout);
-    if (prompt)
+    if (prompt) {
         fprintf(stderr, "%s", prompt);
+    }
     fflush(stderr);
 
-    switch (my_fgets(p, (int)n, sys_stdin)) {
+    switch (my_fgets(tstate, p, (int)n, sys_stdin)) {
     case 0: /* Normal case */
         break;
     case 1: /* Interrupt */
@@ -278,29 +318,40 @@ PyOS_StdioReadline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
         *p = '\0';
         break;
     }
+
     n = strlen(p);
     while (n > 0 && p[n-1] != '\n') {
         size_t incr = n+2;
         if (incr > INT_MAX) {
             PyMem_RawFree(p);
+            PyEval_RestoreThread(tstate);
             PyErr_SetString(PyExc_OverflowError, "input line too long");
+            PyEval_SaveThread();
             return NULL;
         }
+
         pr = (char *)PyMem_RawRealloc(p, n + incr);
         if (pr == NULL) {
             PyMem_RawFree(p);
+            PyEval_RestoreThread(tstate);
             PyErr_NoMemory();
+            PyEval_SaveThread();
             return NULL;
         }
         p = pr;
-        if (my_fgets(p+n, (int)incr, sys_stdin) != 0)
+
+        if (my_fgets(tstate, p+n, (int)incr, sys_stdin) != 0) {
             break;
+        }
         n += strlen(p+n);
     }
+
     pr = (char *)PyMem_RawRealloc(p, n+1);
     if (pr == NULL) {
         PyMem_RawFree(p);
+        PyEval_RestoreThread(tstate);
         PyErr_NoMemory();
+        PyEval_SaveThread();
         return NULL;
     }
     return pr;
@@ -323,7 +374,8 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
     char *rv, *res;
     size_t len;
 
-    if (_PyOS_ReadlineTState == _PyThreadState_GET()) {
+    PyThreadState *tstate = _PyThreadState_GET();
+    if (_PyOS_ReadlineTState == tstate) {
         PyErr_SetString(PyExc_RuntimeError,
                         "can't re-enter readline");
         return NULL;
@@ -342,7 +394,7 @@ PyOS_Readline(FILE *sys_stdin, FILE *sys_stdout, const char *prompt)
         }
     }
 
-    _PyOS_ReadlineTState = _PyThreadState_GET();
+    _PyOS_ReadlineTState = tstate;
     Py_BEGIN_ALLOW_THREADS
     PyThread_acquire_lock(_PyOS_ReadlineLock, 1);