diff options
author | Victor Stinner <vstinner@python.org> | 2020-06-03 15:49:25 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-03 15:49:25 (GMT) |
commit | 5d2396c8cf68fba0a949c6ce474a505e3aba9c1f (patch) | |
tree | 5a0ebd090249175530372137c2ba465f2c020c4e | |
parent | a125561397c80a30db74f6ac289781fb53af9196 (diff) | |
download | cpython-5d2396c8cf68fba0a949c6ce474a505e3aba9c1f.zip cpython-5d2396c8cf68fba0a949c6ce474a505e3aba9c1f.tar.gz cpython-5d2396c8cf68fba0a949c6ce474a505e3aba9c1f.tar.bz2 |
[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 c353764fd564e401cf47a5d9efab18c72c60014e)
* 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 fa7ab6aa0f9a4f695e5525db5a113cd21fa93787)
-rw-r--r-- | Include/internal/pycore_pystate.h | 3 | ||||
-rw-r--r-- | Lib/test/test_repl.py | 22 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst | 2 | ||||
-rw-r--r-- | Modules/signalmodule.c | 18 | ||||
-rw-r--r-- | Parser/myreadline.c | 118 |
5 files changed, 121 insertions, 42 deletions
diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 8dc64db..a9515b4 100644 --- a/Include/internal/pycore_pystate.h +++ b/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 diff --git a/Lib/test/test_repl.py b/Lib/test/test_repl.py index 71f192f..563f188 100644 --- 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 new file mode 100644 index 0000000..a03ed18 --- /dev/null +++ b/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. diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 8348971..b3f5904 100644 --- a/Modules/signalmodule.c +++ b/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(); - if (!_Py_ThreadCanHandleSignals(interp)) { + assert(tstate != NULL); + 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) { diff --git a/Parser/myreadline.c b/Parser/myreadline.c index 04c2793..2dd3623 100644 --- a/Parser/myreadline.c +++ b/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); - if (p != NULL) + char *p = fgets(buf, len, fp); + 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(_PyOS_ReadlineTState); - s = PyErr_CheckSignals(); + PyEval_RestoreThread(tstate); + int s = PyErr_CheckSignals(); PyEval_SaveThread(); - if (s < 0) - return 1; - /* try again */ + + if (s < 0) { + 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); |