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.

diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index 423c8113d7ac030e7d414c37c3591f2debb2768d..0cd5550cfda5c407e73a92792667aa345ed90a28 100644 (file)
--- a/Include/internal/pycore_pystate.h
+++ b/Include/internal/pycore_pystate.h
@@ -144,6 +144,9 @@ PyAPI_FUNC(int) _PyState_AddModule(
     PyObject* module,
     struct PyModuleDef* def);
 
+
+PyAPI_FUNC(int) _PyOS_InterruptOccurred(PyThreadState *tstate);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/Lib/test/test_repl.py b/Lib/test/test_repl.py
index 71f192f90d9a1db33dd4648fda6da23278e348a2..563f188706b93fc89959c69591c097096e8eaeff 100644 (file)
--- a/Lib/test/test_repl.py
+++ b/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()

diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst b/Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst
index f79f20d21d49c6f9a13913646ba4420dc19deba0..a03ed180eb952b7eeb0f40816142ed589c36920f 100644 (file)
--- a/Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst
+++ b/Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst
@@ -1 +1,2 @@
-Fix GIL usage in :c:func:`PyOS_Readline`: lock the GIL to set an exception.
+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.

diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c
index 24dbd4255a6e4609daa28de7b330a69c5a42a638..ef3536a210b04ce40bc0acfc5d652483bb3a0476 100644 (file)
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -1779,10 +1779,11 @@ PyOS_FiniInterrupts(void)
     finisignal();
 }
 
+
+// The caller doesn't have to hold the GIL
 int
-PyOS_InterruptOccurred(void)
+_PyOS_InterruptOccurred(PyThreadState *tstate)
 {
-    PyThreadState *tstate = _PyThreadState_GET();
     _Py_EnsureTstateNotNULL(tstate);
     if (!_Py_ThreadCanHandleSignals(tstate->interp)) {
         return 0;
@@ -1797,6 +1798,15 @@ PyOS_InterruptOccurred(void)
 }
 
 
+// The caller must to hold the GIL
+int
+PyOS_InterruptOccurred(void)
+{
+    PyThreadState *tstate = _PyThreadState_GET();
+    return _PyOS_InterruptOccurred(tstate);
+}
+
+
 #ifdef HAVE_FORK
 static void
 _clear_pending_signals(void)

diff --git a/Parser/myreadline.c b/Parser/myreadline.c
index d2787f0d345cf47acf82f49c26a3502d2ac3a18d..2dd362321aaf304519a59bffdde357c37dc68e1a 100644 (file)
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@@ -24,14 +24,23 @@ 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(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
+
     while (1) {
         if (PyOS_InputHook != NULL) {
             (void)(PyOS_InputHook)();
@@ -60,7 +69,7 @@ my_fgets(PyThreadState* tstate, 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);
@@ -90,7 +99,7 @@ my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp)
         }
 #endif
 
-        if (PyOS_InterruptOccurred()) {
+        if (_PyOS_InterruptOccurred(tstate)) {
             return 1; /* Interrupt */
         }
         return -2; /* Error */