[Python-checkins] [3.9] bpo-40826: Fix GIL usage in PyOS_Readline() (GH-20613) (GH-20616)
Victor Stinner
webhook-mailer at python.org
Wed Jun 3 12:28:27 EDT 2020
https://github.com/python/cpython/commit/6f7346bb3983cd7a6aa97eeeafffb3cecd5292b8
commit: 6f7346bb3983cd7a6aa97eeeafffb3cecd5292b8
branch: 3.8
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020年06月03日T18:28:18+02:00
summary:
[3.9] bpo-40826: Fix GIL usage in PyOS_Readline() (GH-20613) (GH-20616)
* 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)
files:
A Misc/NEWS.d/next/Core and Builtins/2020-06-01-20-31-07.bpo-40826.XCI4M2.rst
M Include/internal/pycore_pystate.h
M Lib/test/test_repl.py
M Modules/signalmodule.c
M Parser/myreadline.c
diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index f90e7e1ab78e3..96d5e31d83a6e 100644
--- a/Include/internal/pycore_pystate.h
+++ b/Include/internal/pycore_pystate.h
@@ -317,6 +317,9 @@ PyAPI_FUNC(void) _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime);
PyAPI_FUNC(void) _PyGILState_Reinit(_PyRuntimeState *runtime);
+
+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 71f192f90d9a1..563f188706b93 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 @@ def test_no_memory(self):
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 @@ def test_multiline_string_parsing(self):
</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 0000000000000..a03ed180eb952
--- /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 0c9a2671fe19b..119fc355ff1fd 100644
--- a/Modules/signalmodule.c
+++ b/Modules/signalmodule.c
@@ -187,14 +187,20 @@ itimer_retval(struct itimerval *iv)
#endif
static int
-is_main(_PyRuntimeState *runtime)
+is_main_interp(_PyRuntimeState *runtime, PyInterpreterState *interp)
{
unsigned long thread = PyThread_get_thread_ident();
- PyInterpreterState *interp = _PyRuntimeState_GetThreadState(runtime)->interp;
return (thread == runtime->main_thread
&& interp == runtime->interpreters.main);
}
+static int
+is_main(_PyRuntimeState *runtime)
+{
+ PyInterpreterState *interp = _PyRuntimeState_GetThreadState(runtime)->interp;
+ return is_main_interp(runtime, interp);
+}
+
static PyObject *
signal_default_int_handler(PyObject *self, PyObject *args)
{
@@ -1726,12 +1732,14 @@ PyOS_FiniInterrupts(void)
finisignal();
}
+
+// The caller doesn't have to hold the GIL
int
-PyOS_InterruptOccurred(void)
+_PyOS_InterruptOccurred(PyThreadState *tstate)
{
if (_Py_atomic_load_relaxed(&Handlers[SIGINT].tripped)) {
_PyRuntimeState *runtime = &_PyRuntime;
- if (!is_main(runtime)) {
+ if (!is_main_interp(runtime, tstate->interp)) {
return 0;
}
_Py_atomic_store_relaxed(&Handlers[SIGINT].tripped, 0);
@@ -1740,6 +1748,16 @@ PyOS_InterruptOccurred(void)
return 0;
}
+
+// 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 43e5583b8bcc4..d7ed357faa383 100644
--- a/Parser/myreadline.c
+++ b/Parser/myreadline.c
@@ -25,25 +25,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
@@ -59,7 +70,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);
@@ -69,23 +80,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 */
@@ -99,7 +114,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;
@@ -134,11 +149,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;
}
@@ -151,17 +167,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;
@@ -170,33 +191,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;
}
@@ -210,6 +243,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) {
@@ -231,7 +266,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,
@@ -250,7 +287,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
@@ -258,16 +295,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 */
@@ -279,29 +319,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;
@@ -324,7 +375,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;
@@ -343,7 +395,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);
More information about the Python-checkins
mailing list