From 4ddee7f5fd38137629eea15f3bf4b48dbdbcb356 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 14 Mar 2016 16:53:12 +0100 Subject: Fix Py_FatalError() if called without the GIL Issue #26558: If Py_FatalError() is called without the GIL, don't try to print the current exception, nor try to flush stdout and stderr: only dump the traceback of Python threads. --- Python/pylifecycle.c | 84 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 34 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 4f5efc9..c4a6d7c 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1241,31 +1241,62 @@ initstdio(void) } +static void +_Py_FatalError_DumpTracebacks(int fd) +{ + PyThreadState *tstate; + +#ifdef WITH_THREAD + /* PyGILState_GetThisThreadState() works even if the GIL was released */ + tstate = PyGILState_GetThisThreadState(); +#else + tstate = PyThreadState_GET(); +#endif + if (tstate == NULL) { + /* _Py_DumpTracebackThreads() requires the thread state to display + * frames */ + return; + } + + fputc('\n', stderr); + fflush(stderr); + + /* display the current Python stack */ + _Py_DumpTracebackThreads(fd, tstate->interp, tstate); +} + /* Print the current exception (if an exception is set) with its traceback, - * or display the current Python stack. - * - * Don't call PyErr_PrintEx() and the except hook, because Py_FatalError() is - * called on catastrophic cases. */ + or display the current Python stack. -static void -_Py_PrintFatalError(int fd) + Don't call PyErr_PrintEx() and the except hook, because Py_FatalError() is + called on catastrophic cases. + + Return 1 if the traceback was displayed, 0 otherwise. */ + +static int +_Py_FatalError_PrintExc(int fd) { PyObject *ferr, *res; PyObject *exception, *v, *tb; int has_tb; - PyThreadState *tstate; + + if (PyThreadState_GET() == NULL) { + /* The GIL is released: trying to acquire it is likely to deadlock, + just give up. */ + return 0; + } PyErr_Fetch(&exception, &v, &tb); if (exception == NULL) { /* No current exception */ - goto display_stack; + return 0; } ferr = _PySys_GetObjectId(&PyId_stderr); if (ferr == NULL || ferr == Py_None) { /* sys.stderr is not set yet or set to None, no need to try to display the exception */ - goto display_stack; + return 0; } PyErr_NormalizeException(&exception, &v, &tb); @@ -1276,7 +1307,7 @@ _Py_PrintFatalError(int fd) PyException_SetTraceback(v, tb); if (exception == NULL) { /* PyErr_NormalizeException() failed */ - goto display_stack; + return 0; } has_tb = (tb != Py_None); @@ -1292,28 +1323,9 @@ _Py_PrintFatalError(int fd) else Py_DECREF(res); - if (has_tb) - return; - -display_stack: -#ifdef WITH_THREAD - /* PyGILState_GetThisThreadState() works even if the GIL was released */ - tstate = PyGILState_GetThisThreadState(); -#else - tstate = PyThreadState_GET(); -#endif - if (tstate == NULL) { - /* _Py_DumpTracebackThreads() requires the thread state to display - * frames */ - return; - } - - fputc('\n', stderr); - fflush(stderr); - - /* display the current Python stack */ - _Py_DumpTracebackThreads(fd, tstate->interp, tstate); + return has_tb; } + /* Print fatal error message and abort */ void @@ -1339,10 +1351,14 @@ Py_FatalError(const char *msg) /* Print the exception (if an exception is set) with its traceback, * or display the current Python stack. */ - _Py_PrintFatalError(fd); + if (!_Py_FatalError_PrintExc(fd)) + _Py_FatalError_DumpTracebacks(fd); - /* Flush sys.stdout and sys.stderr */ - flush_std_files(); + /* Check if the current Python thread hold the GIL */ + if (PyThreadState_GET() != NULL) { + /* Flush sys.stdout and sys.stderr */ + flush_std_files(); + } /* The main purpose of faulthandler is to display the traceback. We already * did our best to display it. So faulthandler can now be disabled. -- cgit v0.12 From 57003f81ead9ff38150c1e4821811c66c296d33c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 15 Mar 2016 17:23:35 +0100 Subject: faulthandler: Test Py_FatalError() with GIL released Issue #26558. --- Lib/test/test_faulthandler.py | 8 ++++++++ Modules/faulthandler.c | 12 ++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py index 0d86cb5..12969d5 100644 --- a/Lib/test/test_faulthandler.py +++ b/Lib/test/test_faulthandler.py @@ -185,6 +185,14 @@ class FaultHandlerTests(unittest.TestCase): 2, 'xyz') + def test_fatal_error_without_gil(self): + self.check_fatal_error(""" + import faulthandler + faulthandler._fatal_error(b'xyz', True) + """, + 2, + 'xyz') + @unittest.skipIf(sys.platform.startswith('openbsd') and HAVE_THREADS, "Issue #12868: sigaltstack() doesn't work on " "OpenBSD if Python is compiled with pthread") diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index 14384b5..45c9fcb 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -935,10 +935,18 @@ static PyObject * faulthandler_fatal_error_py(PyObject *self, PyObject *args) { char *message; - if (!PyArg_ParseTuple(args, "y:fatal_error", &message)) + int release_gil = 0; + if (!PyArg_ParseTuple(args, "y|i:fatal_error", &message, &release_gil)) return NULL; faulthandler_suppress_crash_report(); - Py_FatalError(message); + if (release_gil) { + Py_BEGIN_ALLOW_THREADS + Py_FatalError(message); + Py_END_ALLOW_THREADS + } + else { + Py_FatalError(message); + } Py_RETURN_NONE; } -- cgit v0.12