From 560ccee7bcc66ab52721ac32d7eb2ecb20e6baa2 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sat, 28 Oct 2017 22:13:11 +0200 Subject: [PATCH 1/7] bpo-30696: Fix the REPL looping endlessly when no memory This also fixes partly ``PyRun_InteractiveOneFlags()`` that was returning -1 with no exception set. --- Lib/test/test_readline.py | 31 ++++++++++++++- .../2017-10-28-22-06-03.bpo-30696.lhC3HE.rst | 3 ++ Python/pythonrun.c | 39 +++++++++++-------- 3 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index b4c25dee9d3c20a..14eaf313a3c4ef2 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -9,7 +9,9 @@ import sys import tempfile import unittest -from test.support import import_module, unlink, temp_dir, TESTFN, verbose +from textwrap import dedent +from test.support import (import_module, unlink, temp_dir, TESTFN, verbose, + SuppressCrashReport) from test.support.script_helper import assert_python_ok # Skip tests if there is no readline module @@ -271,11 +273,36 @@ def test_history_size(self): self.assertEqual(lines[-1].strip(), b"last input") +# Tests for the interactive interpreter. +class TestInteractiveInterpreter(unittest.TestCase): + + def test_interactive_no_memory(self): + # Issue #30696: Fix the interactive interpreter looping endlessly when + # no memory. Check also that the fix does not break the interactive + # loop when an exception is raised. + user_input = """ + import sys, _testcapi + 1/0 + _testcapi.set_nomemory(0) + sys.exit(0) + """ + user_input = dedent(user_input) + user_input = b'\r'.join(x.encode() for x in user_input.split('\n')) + user_input += b'\r' + with SuppressCrashReport(): + output = run_pty(None, input=user_input) + self.assertIn(b"Fatal Python error: Cannot recover from MemoryErrors", + output) + + def run_pty(script, input=b"dummy input\r", env=None): pty = import_module('pty') output = bytearray() [master, slave] = pty.openpty() - args = (sys.executable, '-c', script) + if script is None: + args = (sys.executable, '-q') + else: + args = (sys.executable, '-c', script) proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave, env=env) os.close(slave) with ExitStack() as cleanup: diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst new file mode 100644 index 000000000000000..f0a4d30c4756faa --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst @@ -0,0 +1,3 @@ +Fix the interactive interpreter looping endlessly when no memory. This also +fixes partly ``PyRun_InteractiveOneFlags()`` that was returning -1 with no +exception set. diff --git a/Python/pythonrun.c b/Python/pythonrun.c index 17ec182b74cc3f7..68e711e8d432552 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -86,9 +86,10 @@ PyRun_AnyFileExFlags(FILE *fp, const char *filename, int closeit, int PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags *flags) { - PyObject *filename, *v; - int ret, err; + PyObject *filename, *v, *curexc; PyCompilerFlags local_flags; + int ret = -1; + static int nomem_count = 0; filename = PyUnicode_DecodeFSDefault(filename_str); if (filename == NULL) { @@ -110,24 +111,34 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * _PySys_SetObjectId(&PyId_ps2, v = PyUnicode_FromString("... ")); Py_XDECREF(v); } - err = -1; - for (;;) { + while (ret != E_EOF) { ret = PyRun_InteractiveOneObject(fp, filename, flags); + /* Save the current exception that may be cleared by + * _PyDebug_XOptionShowRefCount(). */ + curexc = ret == -1 ? PyErr_Occurred() : NULL; #ifdef Py_REF_DEBUG if (_PyDebug_XOptionShowRefCount() == Py_True) _PyDebug_PrintTotalRefs(); #endif - if (ret == E_EOF) { - err = 0; - break; + if (curexc) { + /* Prevent an endless loop after multiple consecutive MemoryErrors + * while still allowing an interactive command to fail with a + * MemoryError. */ + if (PyErr_GivenExceptionMatches(curexc, PyExc_MemoryError)) { + if (++nomem_count > 16) { + Py_FatalError("Cannot recover from MemoryErrors."); + } + PyErr_Print(); + flush_io(); + continue; + } + PyErr_Print(); + flush_io(); } - /* - if (ret == E_NOMEM) - break; - */ + nomem_count = 0; } Py_DECREF(filename); - return err; + return 0; } /* compute parser flags based on compiler flags */ @@ -167,7 +178,6 @@ PyRun_InteractiveOneObject(FILE *fp, PyObject *filename, PyCompilerFlags *flags) mod_name = _PyUnicode_FromId(&PyId___main__); /* borrowed */ if (mod_name == NULL) { - PyErr_Print(); return -1; } @@ -227,7 +237,6 @@ PyRun_InteractiveOneObject(FILE *fp, PyObject *filename, PyCompilerFlags *flags) PyErr_Clear(); return E_EOF; } - PyErr_Print(); return -1; } m = PyImport_AddModuleObject(mod_name); @@ -239,8 +248,6 @@ PyRun_InteractiveOneObject(FILE *fp, PyObject *filename, PyCompilerFlags *flags) v = run_mod(mod, filename, d, d, flags, arena); PyArena_Free(arena); if (v == NULL) { - PyErr_Print(); - flush_io(); return -1; } Py_DECREF(v); From 15230d1ec057a3b3997063147d8ee2eb3f8e5688 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sun, 29 Oct 2017 11:46:27 +0100 Subject: [PATCH 2/7] Do not break backward compatibility --- Python/pythonrun.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/Python/pythonrun.c b/Python/pythonrun.c index 68e711e8d432552..a871a2ce47e016e 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -65,6 +65,7 @@ static PyObject *run_pyc_file(FILE *, const char *, PyObject *, PyObject *, PyCompilerFlags *); static void err_input(perrdetail *); static void err_free(perrdetail *); +static int PyRun_InteractiveOneObjectEx(FILE *, PyObject *, PyCompilerFlags *); /* Parse input from a file and execute it */ int @@ -112,7 +113,7 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * Py_XDECREF(v); } while (ret != E_EOF) { - ret = PyRun_InteractiveOneObject(fp, filename, flags); + ret = PyRun_InteractiveOneObjectEx(fp, filename, flags); /* Save the current exception that may be cleared by * _PyDebug_XOptionShowRefCount(). */ curexc = ret == -1 ? PyErr_Occurred() : NULL; @@ -165,8 +166,9 @@ static int PARSER_FLAGS(PyCompilerFlags *flags) PyPARSE_WITH_IS_KEYWORD : 0)) : 0) #endif -int -PyRun_InteractiveOneObject(FILE *fp, PyObject *filename, PyCompilerFlags *flags) +static int +PyRun_InteractiveOneObjectEx(FILE *fp, PyObject *filename, + PyCompilerFlags *flags) { PyObject *m, *d, *v, *w, *oenc = NULL, *mod_name; mod_ty mod; @@ -255,6 +257,19 @@ PyRun_InteractiveOneObject(FILE *fp, PyObject *filename, PyCompilerFlags *flags) return 0; } +int +PyRun_InteractiveOneObject(FILE *fp, PyObject *filename, PyCompilerFlags *flags) +{ + int res; + + res = PyRun_InteractiveOneObjectEx(fp, filename, flags); + if (res == -1) { + PyErr_Print(); + flush_io(); + } + return res; +} + int PyRun_InteractiveOneFlags(FILE *fp, const char *filename_str, PyCompilerFlags *flags) { From ac6003ae6d4612cd1d5ab71f2948540f624eadf9 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sun, 29 Oct 2017 14:15:20 +0100 Subject: [PATCH 3/7] Call _PyDebug_PrintTotalRefs() after printing the exception --- Python/pythonrun.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/Python/pythonrun.c b/Python/pythonrun.c index a871a2ce47e016e..b2d7dbd406fb011 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -87,7 +87,7 @@ PyRun_AnyFileExFlags(FILE *fp, const char *filename, int closeit, int PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags *flags) { - PyObject *filename, *v, *curexc; + PyObject *filename, *v; PyCompilerFlags local_flags; int ret = -1; static int nomem_count = 0; @@ -114,29 +114,26 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * } while (ret != E_EOF) { ret = PyRun_InteractiveOneObjectEx(fp, filename, flags); - /* Save the current exception that may be cleared by - * _PyDebug_XOptionShowRefCount(). */ - curexc = ret == -1 ? PyErr_Occurred() : NULL; -#ifdef Py_REF_DEBUG - if (_PyDebug_XOptionShowRefCount() == Py_True) - _PyDebug_PrintTotalRefs(); -#endif - if (curexc) { + if (ret == -1 && PyErr_Occurred()) { /* Prevent an endless loop after multiple consecutive MemoryErrors * while still allowing an interactive command to fail with a * MemoryError. */ - if (PyErr_GivenExceptionMatches(curexc, PyExc_MemoryError)) { + if (PyErr_ExceptionMatches(PyExc_MemoryError)) { if (++nomem_count > 16) { Py_FatalError("Cannot recover from MemoryErrors."); } - PyErr_Print(); - flush_io(); - continue; + } else { + nomem_count = 0; } PyErr_Print(); flush_io(); + } else { + nomem_count = 0; } - nomem_count = 0; +#ifdef Py_REF_DEBUG + if (_PyDebug_XOptionShowRefCount() == Py_True) + _PyDebug_PrintTotalRefs(); +#endif } Py_DECREF(filename); return 0; From 44c335e318ac824aaf268b06e6e02432a1078359 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sun, 29 Oct 2017 15:04:07 +0100 Subject: [PATCH 4/7] Create test_repl.py and move run_pty() to test.support --- Lib/test/support/__init__.py | 56 ++++++++++++++++++++++++ Lib/test/test_readline.py | 83 +----------------------------------- Lib/test/test_repl.py | 28 ++++++++++++ 3 files changed, 85 insertions(+), 82 deletions(-) create mode 100644 Lib/test/test_repl.py diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 4f60507919fe4d0..a7e2aeb033d5d57 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -32,6 +32,9 @@ import unittest import urllib.error import warnings +import selectors +from contextlib import ExitStack +from errno import EIO try: import multiprocessing.process @@ -105,6 +108,7 @@ "run_with_locale", "swap_item", "swap_attr", "Matcher", "set_memlimit", "SuppressCrashReport", "sortdict", "run_with_tz", "PGO", "missing_compiler_executable", "fd_count", + "run_pty", ] class Error(Exception): @@ -2755,3 +2759,55 @@ def fd_count(): msvcrt.CrtSetReportMode(report_type, old_modes[report_type]) return count + +def run_pty(script, input=b"dummy input\r", env=None): + pty = import_module('pty') + output = bytearray() + [master, slave] = pty.openpty() + if script is None: + args = (sys.executable, '-q') + else: + args = (sys.executable, '-c', script) + proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave, env=env) + os.close(slave) + with ExitStack() as cleanup: + cleanup.enter_context(proc) + def terminate(proc): + try: + proc.terminate() + except ProcessLookupError: + # Workaround for Open/Net BSD bug (Issue 16762) + pass + cleanup.callback(terminate, proc) + cleanup.callback(os.close, master) + # Avoid using DefaultSelector and PollSelector. Kqueue() does not + # work with pseudo-terminals on OS X < 10.9 (Issue 20365) and Open + # BSD (Issue 20667). Poll() does not work with OS X 10.6 or 10.4 + # either (Issue 20472). Hopefully the file descriptor is low enough + # to use with select(). + sel = cleanup.enter_context(selectors.SelectSelector()) + sel.register(master, selectors.EVENT_READ | selectors.EVENT_WRITE) + os.set_blocking(master, False) + while True: + for [_, events] in sel.select(): + if events & selectors.EVENT_READ: + try: + chunk = os.read(master, 0x10000) + except OSError as err: + # Linux raises EIO when slave is closed (Issue 5380) + if err.errno != EIO: + raise + chunk = b"" + if not chunk: + return output + output.extend(chunk) + if events & selectors.EVENT_WRITE: + try: + input = input[os.write(master, input):] + except OSError as err: + # Apparently EIO means the slave was closed + if err.errno != EIO: + raise + input = b"" # Stop writing + if not input: + sel.modify(master, selectors.EVENT_READ) diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 14eaf313a3c4ef2..320dd08ded96837 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -1,17 +1,11 @@ """ Very minimal unittests for parts of the readline module. """ -from contextlib import ExitStack -from errno import EIO import os -import selectors -import subprocess -import sys import tempfile import unittest -from textwrap import dedent from test.support import (import_module, unlink, temp_dir, TESTFN, verbose, - SuppressCrashReport) + run_pty) from test.support.script_helper import assert_python_ok # Skip tests if there is no readline module @@ -273,80 +267,5 @@ def test_history_size(self): self.assertEqual(lines[-1].strip(), b"last input") -# Tests for the interactive interpreter. -class TestInteractiveInterpreter(unittest.TestCase): - - def test_interactive_no_memory(self): - # Issue #30696: Fix the interactive interpreter looping endlessly when - # no memory. Check also that the fix does not break the interactive - # loop when an exception is raised. - user_input = """ - import sys, _testcapi - 1/0 - _testcapi.set_nomemory(0) - sys.exit(0) - """ - user_input = dedent(user_input) - user_input = b'\r'.join(x.encode() for x in user_input.split('\n')) - user_input += b'\r' - with SuppressCrashReport(): - output = run_pty(None, input=user_input) - self.assertIn(b"Fatal Python error: Cannot recover from MemoryErrors", - output) - - -def run_pty(script, input=b"dummy input\r", env=None): - pty = import_module('pty') - output = bytearray() - [master, slave] = pty.openpty() - if script is None: - args = (sys.executable, '-q') - else: - args = (sys.executable, '-c', script) - proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave, env=env) - os.close(slave) - with ExitStack() as cleanup: - cleanup.enter_context(proc) - def terminate(proc): - try: - proc.terminate() - except ProcessLookupError: - # Workaround for Open/Net BSD bug (Issue 16762) - pass - cleanup.callback(terminate, proc) - cleanup.callback(os.close, master) - # Avoid using DefaultSelector and PollSelector. Kqueue() does not - # work with pseudo-terminals on OS X < 10.9 (Issue 20365) and Open - # BSD (Issue 20667). Poll() does not work with OS X 10.6 or 10.4 - # either (Issue 20472). Hopefully the file descriptor is low enough - # to use with select(). - sel = cleanup.enter_context(selectors.SelectSelector()) - sel.register(master, selectors.EVENT_READ | selectors.EVENT_WRITE) - os.set_blocking(master, False) - while True: - for [_, events] in sel.select(): - if events & selectors.EVENT_READ: - try: - chunk = os.read(master, 0x10000) - except OSError as err: - # Linux raises EIO when slave is closed (Issue 5380) - if err.errno != EIO: - raise - chunk = b"" - if not chunk: - return output - output.extend(chunk) - if events & selectors.EVENT_WRITE: - try: - input = input[os.write(master, input):] - except OSError as err: - # Apparently EIO means the slave was closed - if err.errno != EIO: - raise - input = b"" # Stop writing - if not input: - sel.modify(master, selectors.EVENT_READ) - - if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_repl.py b/Lib/test/test_repl.py new file mode 100644 index 000000000000000..984f36120827ac5 --- /dev/null +++ b/Lib/test/test_repl.py @@ -0,0 +1,28 @@ +"""Test the interactive interpreter.""" + +import unittest +from textwrap import dedent +from test.support import run_pty, SuppressCrashReport + +class TestInteractiveInterpreter(unittest.TestCase): + + def test_no_memory(self): + # Issue #30696: Fix the interactive interpreter looping endlessly when + # no memory. Check also that the fix does not break the interactive + # loop when an exception is raised. + user_input = """ + import sys, _testcapi + 1/0 + _testcapi.set_nomemory(0) + sys.exit(0) + """ + user_input = dedent(user_input) + user_input = b'\r'.join(x.encode() for x in user_input.split('\n')) + user_input += b'\r' + with SuppressCrashReport(): + output = run_pty(None, input=user_input) + self.assertIn(b"Fatal Python error: Cannot recover from MemoryErrors", + output) + +if __name__ == "__main__": + unittest.main() From be1df915920c04a440fb96ccd8e10d627fa25ed2 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sun, 29 Oct 2017 15:14:00 +0100 Subject: [PATCH 5/7] Update Misc/NEWS. --- .../2017-10-28-22-06-03.bpo-30696.lhC3HE.rst | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst b/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst index f0a4d30c4756faa..76bc683488017df 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2017-10-28-22-06-03.bpo-30696.lhC3HE.rst @@ -1,3 +1 @@ -Fix the interactive interpreter looping endlessly when no memory. This also -fixes partly ``PyRun_InteractiveOneFlags()`` that was returning -1 with no -exception set. +Fix the interactive interpreter looping endlessly when no memory. From 477f9c5cb29fd41ba040312e1fa4e0742e566f31 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sun, 5 Nov 2017 13:53:06 +0100 Subject: [PATCH 6/7] Do not call Py_FatalError() and return an error --- Lib/test/support/__init__.py | 13 ++++++++----- Lib/test/test_readline.py | 6 +++--- Lib/test/test_repl.py | 7 ++++--- Python/pythonrun.c | 13 ++++++++----- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index a7e2aeb033d5d57..c206e8a239e802e 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -2774,7 +2774,7 @@ def run_pty(script, input=b"dummy input\r", env=None): cleanup.enter_context(proc) def terminate(proc): try: - proc.terminate() + proc.kill() except ProcessLookupError: # Workaround for Open/Net BSD bug (Issue 16762) pass @@ -2786,10 +2786,10 @@ def terminate(proc): # either (Issue 20472). Hopefully the file descriptor is low enough # to use with select(). sel = cleanup.enter_context(selectors.SelectSelector()) - sel.register(master, selectors.EVENT_READ | selectors.EVENT_WRITE) os.set_blocking(master, False) - while True: - for [_, events] in sel.select(): + sel.register(master, selectors.EVENT_READ | selectors.EVENT_WRITE) + while sel.get_map(): + for [key, events] in sel.select(): if events & selectors.EVENT_READ: try: chunk = os.read(master, 0x10000) @@ -2799,7 +2799,8 @@ def terminate(proc): raise chunk = b"" if not chunk: - return output + sel.unregister(key.fileobj) + break output.extend(chunk) if events & selectors.EVENT_WRITE: try: @@ -2811,3 +2812,5 @@ def terminate(proc): input = b"" # Stop writing if not input: sel.modify(master, selectors.EVENT_READ) + proc.wait() + return proc.returncode, output diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 320dd08ded96837..2dd0e9116857317 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -141,11 +141,11 @@ def test_init(self): """ def test_auto_history_enabled(self): - output = run_pty(self.auto_history_script.format(True)) + rc, output = run_pty(self.auto_history_script.format(True)) self.assertIn(b"History length: 1\r\n", output) def test_auto_history_disabled(self): - output = run_pty(self.auto_history_script.format(False)) + rc, output = run_pty(self.auto_history_script.format(False)) self.assertIn(b"History length: 0\r\n", output) def test_nonascii(self): @@ -211,7 +211,7 @@ def display(substitution, matches, longest_match_length): input += b"\t\t" # Display possible completions input += b"x\t" # Complete "t\xEBx" -> "t\xEBxt" input += b"\r" - output = run_pty(script, input) + rc, output = run_pty(script, input) self.assertIn(b"text 't\\xeb'\r\n", output) self.assertIn(b"line '[\\xefnserted]|t\\xeb[after]'\r\n", output) self.assertIn(b"indexes 11 13\r\n", output) diff --git a/Lib/test/test_repl.py b/Lib/test/test_repl.py index 984f36120827ac5..c1de4547edcc195 100644 --- a/Lib/test/test_repl.py +++ b/Lib/test/test_repl.py @@ -13,6 +13,7 @@ def test_no_memory(self): user_input = """ import sys, _testcapi 1/0 + print('After the exception.') _testcapi.set_nomemory(0) sys.exit(0) """ @@ -20,9 +21,9 @@ def test_no_memory(self): user_input = b'\r'.join(x.encode() for x in user_input.split('\n')) user_input += b'\r' with SuppressCrashReport(): - output = run_pty(None, input=user_input) - self.assertIn(b"Fatal Python error: Cannot recover from MemoryErrors", - output) + rc, output = run_pty(None, input=user_input) + self.assertIn(b'After the exception.\r\n>>>', output) + self.assertIn(rc, (1, 120)) if __name__ == "__main__": unittest.main() diff --git a/Python/pythonrun.c b/Python/pythonrun.c index b2d7dbd406fb011..ba5dbf9b7c4c0a7 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -88,8 +88,8 @@ int PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags *flags) { PyObject *filename, *v; + int ret, err; PyCompilerFlags local_flags; - int ret = -1; static int nomem_count = 0; filename = PyUnicode_DecodeFSDefault(filename_str); @@ -112,7 +112,8 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * _PySys_SetObjectId(&PyId_ps2, v = PyUnicode_FromString("... ")); Py_XDECREF(v); } - while (ret != E_EOF) { + err = 0; + do { ret = PyRun_InteractiveOneObjectEx(fp, filename, flags); if (ret == -1 && PyErr_Occurred()) { /* Prevent an endless loop after multiple consecutive MemoryErrors @@ -120,7 +121,9 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * * MemoryError. */ if (PyErr_ExceptionMatches(PyExc_MemoryError)) { if (++nomem_count > 16) { - Py_FatalError("Cannot recover from MemoryErrors."); + PyErr_Clear(); + err = -1; + break; } } else { nomem_count = 0; @@ -134,9 +137,9 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * if (_PyDebug_XOptionShowRefCount() == Py_True) _PyDebug_PrintTotalRefs(); #endif - } + } while (ret != E_EOF); Py_DECREF(filename); - return 0; + return err; } /* compute parser flags based on compiler flags */ From 1fe98b2de1b9c372a97bed2ee3c48c737c095fd7 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Mon, 6 Nov 2017 15:31:56 +0100 Subject: [PATCH 7/7] Updates after the second round of review --- Doc/c-api/veryhigh.rst | 3 +- Lib/test/support/__init__.py | 58 -------------------------------- Lib/test/test_readline.py | 64 +++++++++++++++++++++++++++++++++--- Lib/test/test_repl.py | 45 +++++++++++++++++++++---- Python/pythonrun.c | 4 ++- 5 files changed, 103 insertions(+), 71 deletions(-) diff --git a/Doc/c-api/veryhigh.rst b/Doc/c-api/veryhigh.rst index 6ab5942929fccea..3897fdd828216dd 100644 --- a/Doc/c-api/veryhigh.rst +++ b/Doc/c-api/veryhigh.rst @@ -141,7 +141,8 @@ the same library that the Python runtime is using. Read and execute statements from a file associated with an interactive device until EOF is reached. The user will be prompted using ``sys.ps1`` and ``sys.ps2``. *filename* is decoded from the filesystem encoding - (:func:`sys.getfilesystemencoding`). Returns ``0`` at EOF. + (:func:`sys.getfilesystemencoding`). Returns ``0`` at EOF or a negative + number upon failure. .. c:var:: int (*PyOS_InputHook)(void) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 6f6a6f6bc9d4f6c..adc4e8649244cca 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -32,9 +32,6 @@ import unittest import urllib.error import warnings -import selectors -from contextlib import ExitStack -from errno import EIO try: import multiprocessing.process @@ -108,7 +105,6 @@ "run_with_locale", "swap_item", "swap_attr", "Matcher", "set_memlimit", "SuppressCrashReport", "sortdict", "run_with_tz", "PGO", "missing_compiler_executable", "fd_count", - "run_pty", ] class Error(Exception): @@ -2760,60 +2756,6 @@ def fd_count(): return count -def run_pty(script, input=b"dummy input\r", env=None): - pty = import_module('pty') - output = bytearray() - [master, slave] = pty.openpty() - if script is None: - args = (sys.executable, '-q') - else: - args = (sys.executable, '-c', script) - proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave, env=env) - os.close(slave) - with ExitStack() as cleanup: - cleanup.enter_context(proc) - def terminate(proc): - try: - proc.kill() - except ProcessLookupError: - # Workaround for Open/Net BSD bug (Issue 16762) - pass - cleanup.callback(terminate, proc) - cleanup.callback(os.close, master) - # Avoid using DefaultSelector and PollSelector. Kqueue() does not - # work with pseudo-terminals on OS X < 10.9 (Issue 20365) and Open - # BSD (Issue 20667). Poll() does not work with OS X 10.6 or 10.4 - # either (Issue 20472). Hopefully the file descriptor is low enough - # to use with select(). - sel = cleanup.enter_context(selectors.SelectSelector()) - os.set_blocking(master, False) - sel.register(master, selectors.EVENT_READ | selectors.EVENT_WRITE) - while sel.get_map(): - for [key, events] in sel.select(): - if events & selectors.EVENT_READ: - try: - chunk = os.read(master, 0x10000) - except OSError as err: - # Linux raises EIO when slave is closed (Issue 5380) - if err.errno != EIO: - raise - chunk = b"" - if not chunk: - sel.unregister(key.fileobj) - break - output.extend(chunk) - if events & selectors.EVENT_WRITE: - try: - input = input[os.write(master, input):] - except OSError as err: - # Apparently EIO means the slave was closed - if err.errno != EIO: - raise - input = b"" # Stop writing - if not input: - sel.modify(master, selectors.EVENT_READ) - proc.wait() - return proc.returncode, output class SaveSignals: """ diff --git a/Lib/test/test_readline.py b/Lib/test/test_readline.py index 2dd0e9116857317..b4c25dee9d3c20a 100644 --- a/Lib/test/test_readline.py +++ b/Lib/test/test_readline.py @@ -1,11 +1,15 @@ """ Very minimal unittests for parts of the readline module. """ +from contextlib import ExitStack +from errno import EIO import os +import selectors +import subprocess +import sys import tempfile import unittest -from test.support import (import_module, unlink, temp_dir, TESTFN, verbose, - run_pty) +from test.support import import_module, unlink, temp_dir, TESTFN, verbose from test.support.script_helper import assert_python_ok # Skip tests if there is no readline module @@ -141,11 +145,11 @@ def test_init(self): """ def test_auto_history_enabled(self): - rc, output = run_pty(self.auto_history_script.format(True)) + output = run_pty(self.auto_history_script.format(True)) self.assertIn(b"History length: 1\r\n", output) def test_auto_history_disabled(self): - rc, output = run_pty(self.auto_history_script.format(False)) + output = run_pty(self.auto_history_script.format(False)) self.assertIn(b"History length: 0\r\n", output) def test_nonascii(self): @@ -211,7 +215,7 @@ def display(substitution, matches, longest_match_length): input += b"\t\t" # Display possible completions input += b"x\t" # Complete "t\xEBx" -> "t\xEBxt" input += b"\r" - rc, output = run_pty(script, input) + output = run_pty(script, input) self.assertIn(b"text 't\\xeb'\r\n", output) self.assertIn(b"line '[\\xefnserted]|t\\xeb[after]'\r\n", output) self.assertIn(b"indexes 11 13\r\n", output) @@ -267,5 +271,55 @@ def test_history_size(self): self.assertEqual(lines[-1].strip(), b"last input") +def run_pty(script, input=b"dummy input\r", env=None): + pty = import_module('pty') + output = bytearray() + [master, slave] = pty.openpty() + args = (sys.executable, '-c', script) + proc = subprocess.Popen(args, stdin=slave, stdout=slave, stderr=slave, env=env) + os.close(slave) + with ExitStack() as cleanup: + cleanup.enter_context(proc) + def terminate(proc): + try: + proc.terminate() + except ProcessLookupError: + # Workaround for Open/Net BSD bug (Issue 16762) + pass + cleanup.callback(terminate, proc) + cleanup.callback(os.close, master) + # Avoid using DefaultSelector and PollSelector. Kqueue() does not + # work with pseudo-terminals on OS X < 10.9 (Issue 20365) and Open + # BSD (Issue 20667). Poll() does not work with OS X 10.6 or 10.4 + # either (Issue 20472). Hopefully the file descriptor is low enough + # to use with select(). + sel = cleanup.enter_context(selectors.SelectSelector()) + sel.register(master, selectors.EVENT_READ | selectors.EVENT_WRITE) + os.set_blocking(master, False) + while True: + for [_, events] in sel.select(): + if events & selectors.EVENT_READ: + try: + chunk = os.read(master, 0x10000) + except OSError as err: + # Linux raises EIO when slave is closed (Issue 5380) + if err.errno != EIO: + raise + chunk = b"" + if not chunk: + return output + output.extend(chunk) + if events & selectors.EVENT_WRITE: + try: + input = input[os.write(master, input):] + except OSError as err: + # Apparently EIO means the slave was closed + if err.errno != EIO: + raise + input = b"" # Stop writing + if not input: + sel.modify(master, selectors.EVENT_READ) + + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_repl.py b/Lib/test/test_repl.py index c1de4547edcc195..9efd459a6f0763e 100644 --- a/Lib/test/test_repl.py +++ b/Lib/test/test_repl.py @@ -1,11 +1,42 @@ """Test the interactive interpreter.""" +import sys +import os import unittest +import subprocess from textwrap import dedent -from test.support import run_pty, SuppressCrashReport +from test.support import cpython_only, SuppressCrashReport +from test.support.script_helper import kill_python + +def spawn_repl(*args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, **kw): + """Run the Python REPL with the given arguments. + + kw is extra keyword args to pass to subprocess.Popen. Returns a Popen + object. + """ + + # To run the REPL without using a terminal, spawn python with the command + # line option '-i' and the process name set to ''. + # The directory of argv[0] must match the directory of the Python + # executable for the Popen() call to python to succeed as the directory + # path may be used by Py_GetPath() to build the default module search + # path. + stdin_fname = os.path.join(os.path.dirname(sys.executable), "") + cmd_line = [stdin_fname, '-E', '-i'] + cmd_line.extend(args) + + # Set TERM=vt100, for the rationale see the comments in spawn_python() of + # test.support.script_helper. + env = kw.setdefault('env', dict(os.environ)) + env['TERM'] = 'vt100' + return subprocess.Popen(cmd_line, executable=sys.executable, + stdin=subprocess.PIPE, + stdout=stdout, stderr=stderr, + **kw) class TestInteractiveInterpreter(unittest.TestCase): + @cpython_only def test_no_memory(self): # Issue #30696: Fix the interactive interpreter looping endlessly when # no memory. Check also that the fix does not break the interactive @@ -18,12 +49,14 @@ def test_no_memory(self): sys.exit(0) """ user_input = dedent(user_input) - user_input = b'\r'.join(x.encode() for x in user_input.split('\n')) - user_input += b'\r' + user_input = user_input.encode() + p = spawn_repl() with SuppressCrashReport(): - rc, output = run_pty(None, input=user_input) - self.assertIn(b'After the exception.\r\n>>>', output) - self.assertIn(rc, (1, 120)) + p.stdin.write(user_input) + output = kill_python(p) + self.assertIn(b'After the exception.', output) + # Exit code 120: Py_FinalizeEx() failed to flush stdout and stderr. + self.assertIn(p.returncode, (1, 120)) if __name__ == "__main__": unittest.main() diff --git a/Python/pythonrun.c b/Python/pythonrun.c index ba5dbf9b7c4c0a7..b057f80635bb937 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -90,7 +90,7 @@ PyRun_InteractiveLoopFlags(FILE *fp, const char *filename_str, PyCompilerFlags * PyObject *filename, *v; int ret, err; PyCompilerFlags local_flags; - static int nomem_count = 0; + int nomem_count = 0; filename = PyUnicode_DecodeFSDefault(filename_str); if (filename == NULL) { @@ -166,6 +166,8 @@ static int PARSER_FLAGS(PyCompilerFlags *flags) PyPARSE_WITH_IS_KEYWORD : 0)) : 0) #endif +/* A PyRun_InteractiveOneObject() auxiliary function that does not print the + * error on failure. */ static int PyRun_InteractiveOneObjectEx(FILE *fp, PyObject *filename, PyCompilerFlags *flags)