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.
This commit is contained in:
Victor Stinner 2020-06-03 14:39:59 +02:00 committed by GitHub
parent 18a90248fd
commit fa7ab6aa0f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 46 additions and 13 deletions

View File

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

View File

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

View File

@ -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.

View File

@ -1779,10 +1779,11 @@ PyOS_FiniInterrupts(void)
finisignal(); finisignal();
} }
// The caller doesn't have to hold the GIL
int int
PyOS_InterruptOccurred(void) _PyOS_InterruptOccurred(PyThreadState *tstate)
{ {
PyThreadState *tstate = _PyThreadState_GET();
_Py_EnsureTstateNotNULL(tstate); _Py_EnsureTstateNotNULL(tstate);
if (!_Py_ThreadCanHandleSignals(tstate->interp)) { if (!_Py_ThreadCanHandleSignals(tstate->interp)) {
return 0; 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 #ifdef HAVE_FORK
static void static void
_clear_pending_signals(void) _clear_pending_signals(void)

View File

@ -24,14 +24,23 @@ static PyThread_type_lock _PyOS_ReadlineLock = NULL;
int (*PyOS_InputHook)(void) = NULL; int (*PyOS_InputHook)(void) = NULL;
/* This function restarts a fgets() after an EINTR error occurred /* This function restarts a fgets() after an EINTR error occurred
except if PyOS_InterruptOccurred() returns true. */ except if _PyOS_InterruptOccurred() returns true. */
static int static int
my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp) my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp)
{ {
#ifdef MS_WINDOWS #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 #endif
while (1) { while (1) {
if (PyOS_InputHook != NULL) { if (PyOS_InputHook != NULL) {
(void)(PyOS_InputHook)(); (void)(PyOS_InputHook)();
@ -60,7 +69,7 @@ my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp)
through to check for EOF. through to check for EOF.
*/ */
if (GetLastError()==ERROR_OPERATION_ABORTED) { if (GetLastError()==ERROR_OPERATION_ABORTED) {
hInterruptEvent = _PyOS_SigintEvent(); HANDLE hInterruptEvent = _PyOS_SigintEvent();
switch (WaitForSingleObjectEx(hInterruptEvent, 10, FALSE)) { switch (WaitForSingleObjectEx(hInterruptEvent, 10, FALSE)) {
case WAIT_OBJECT_0: case WAIT_OBJECT_0:
ResetEvent(hInterruptEvent); ResetEvent(hInterruptEvent);
@ -90,7 +99,7 @@ my_fgets(PyThreadState* tstate, char *buf, int len, FILE *fp)
} }
#endif #endif
if (PyOS_InterruptOccurred()) { if (_PyOS_InterruptOccurred(tstate)) {
return 1; /* Interrupt */ return 1; /* Interrupt */
} }
return -2; /* Error */ return -2; /* Error */