faulthandler now works in non-Python threads

Issue #26563:

* Add _PyGILState_GetInterpreterStateUnsafe() function: the single
  PyInterpreterState used by this process' GILState implementation.
* Enhance _Py_DumpTracebackThreads() to retrieve the interpreter state from
  autoInterpreterState in last resort. The function now accepts NULL for interp
  and current_tstate parameters.
* test_faulthandler: fix a ResourceWarning when test is interrupted by CTRL+c

7 files changed, 164 insertions, 48 deletions

diff --git a/Include/pystate.h b/Include/pystate.h
index 550c332..9423bc7 100644
--- a/Include/pystate.h
+++ b/Include/pystate.h
@@ -243,15 +243,23 @@ PyAPI_FUNC(void) PyGILState_Release(PyGILState_STATE);
 */
 PyAPI_FUNC(PyThreadState *) PyGILState_GetThisThreadState(void);
 
-/* Helper/diagnostic function - return 1 if the current thread
- * currently holds the GIL, 0 otherwise
- */
 #ifndef Py_LIMITED_API
 /* Issue #26558: Flag to disable PyGILState_Check().
-   If set, PyGILState_Check() always return 1. */
+   If set to non-zero, PyGILState_Check() always return 1. */
 PyAPI_DATA(int) _PyGILState_check_enabled;
 
+/* Helper/diagnostic function - return 1 if the current thread
+   currently holds the GIL, 0 otherwise.
+
+   The function returns 1 if _PyGILState_check_enabled is non-zero. */
 PyAPI_FUNC(int) PyGILState_Check(void);
+
+/* Unsafe function to get the single PyInterpreterState used by this process'
+   GILState implementation.
+
+   Return NULL before _PyGILState_Init() is called and after _PyGILState_Fini()
+   is called. */
+PyAPI_FUNC(PyInterpreterState *) _PyGILState_GetInterpreterStateUnsafe(void);
 #endif
 
 #endif   /* #ifdef WITH_THREAD */
diff --git a/Include/traceback.h b/Include/traceback.h
index 7c630e3..76e169a 100644
--- a/Include/traceback.h
+++ b/Include/traceback.h
@@ -53,19 +53,32 @@ PyAPI_DATA(void) _Py_DumpTraceback(
     PyThreadState *tstate);
 
 /* Write the traceback of all threads into the file 'fd'. current_thread can be
-   NULL. Return NULL on success, or an error message on error.
+   NULL.
+
+   Return NULL on success, or an error message on error.
 
    This function is written for debug purpose only. It calls
    _Py_DumpTraceback() for each thread, and so has the same limitations. It
    only write the traceback of the first 100 threads: write "..." if there are
    more threads.
 
+   If current_tstate is NULL, the function tries to get the Python thread state
+   of the current thread. It is not an error if the function is unable to get
+   the current Python thread state.
+
+   If interp is NULL, the function tries to get the interpreter state from
+   the current Python thread state, or from
+   _PyGILState_GetInterpreterStateUnsafe() in last resort.
+
+   It is better to pass NULL to interp and current_tstate, the function tries
+   different options to retrieve these informations.
+
    This function is signal safe. */
 
 PyAPI_DATA(const char*) _Py_DumpTracebackThreads(
-    int fd, PyInterpreterState *interp,
-    PyThreadState *current_thread);
-
+    int fd,
+    PyInterpreterState *interp,
+    PyThreadState *current_tstate);
 
 #ifndef Py_LIMITED_API
 
diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py
index 12969d5..c3cd657 100644
--- a/Lib/test/test_faulthandler.py
+++ b/Lib/test/test_faulthandler.py
@@ -58,8 +58,9 @@ class FaultHandlerTests(unittest.TestCase):
             pass_fds.append(fd)
         with support.SuppressCrashReport():
             process = script_helper.spawn_python('-c', code, pass_fds=pass_fds)
-        stdout, stderr = process.communicate()
-        exitcode = process.wait()
+            with process:
+                stdout, stderr = process.communicate()
+                exitcode = process.wait()
         output = support.strip_python_stderr(stdout)
         output = output.decode('ascii', 'backslashreplace')
         if filename:
@@ -73,14 +74,11 @@ class FaultHandlerTests(unittest.TestCase):
             with open(fd, "rb", closefd=False) as fp:
                 output = fp.read()
             output = output.decode('ascii', 'backslashreplace')
-        output = re.sub('Current thread 0x[0-9a-f]+',
-                        'Current thread XXX',
-                        output)
         return output.splitlines(), exitcode
 
     def check_fatal_error(self, code, line_number, name_regex,
                           filename=None, all_threads=True, other_regex=None,
-                          fd=None):
+                          fd=None, know_current_thread=True):
         """
         Check that the fault handler for fatal errors is enabled and check the
         traceback from the child process output.
@@ -88,19 +86,22 @@ class FaultHandlerTests(unittest.TestCase):
         Raise an error if the output doesn't match the expected format.
         """
         if all_threads:
-            header = 'Current thread XXX (most recent call first)'
+            if know_current_thread:
+                header = 'Current thread 0x[0-9a-f]+'
+            else:
+                header = 'Thread 0x[0-9a-f]+'
         else:
-            header = 'Stack (most recent call first)'
+            header = 'Stack'
         regex = """
             ^Fatal Python error: {name}
 
-            {header}:
+            {header} \(most recent call first\):
               File "<string>", line {lineno} in <module>
             """
         regex = dedent(regex.format(
             lineno=line_number,
             name=name_regex,
-            header=re.escape(header))).strip()
+            header=header)).strip()
         if other_regex:
             regex += '|' + other_regex
         output, exitcode = self.get_output(code, filename=filename, fd=fd)
@@ -129,6 +130,17 @@ class FaultHandlerTests(unittest.TestCase):
             3,
             'Segmentation fault')
 
+    @unittest.skipIf(not HAVE_THREADS, 'need threads')
+    def test_fatal_error_c_thread(self):
+        self.check_fatal_error("""
+            import faulthandler
+            faulthandler.enable()
+            faulthandler._fatal_error_c_thread()
+            """,
+            3,
+            'in new thread',
+            know_current_thread=False)
+
     def test_sigabrt(self):
         self.check_fatal_error("""
             import faulthandler
@@ -465,7 +477,7 @@ class FaultHandlerTests(unittest.TestCase):
               File ".*threading.py", line [0-9]+ in _bootstrap_inner
               File ".*threading.py", line [0-9]+ in _bootstrap
 
-            Current thread XXX \(most recent call first\):
+            Current thread 0x[0-9a-f]+ \(most recent call first\):
               File "<string>", line {lineno} in dump
               File "<string>", line 28 in <module>$
             """
@@ -637,7 +649,7 @@ class FaultHandlerTests(unittest.TestCase):
         trace = '\n'.join(trace)
         if not unregister:
             if all_threads:
-                regex = 'Current thread XXX \(most recent call first\):\n'
+                regex = 'Current thread 0x[0-9a-f]+ \(most recent call first\):\n'
             else:
                 regex = 'Stack \(most recent call first\):\n'
             regex = expected_traceback(14, 32, regex)
diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c
index 45c9fcb..1c247b7 100644
--- a/Modules/faulthandler.c
+++ b/Modules/faulthandler.c
@@ -202,8 +202,9 @@ faulthandler_get_fileno(PyObject **file_ptr)
 static PyThreadState*
 get_thread_state(void)
 {
-    PyThreadState *tstate = PyThreadState_Get();
+    PyThreadState *tstate = _PyThreadState_UncheckedGet();
     if (tstate == NULL) {
+        /* just in case but very unlikely... */
         PyErr_SetString(PyExc_RuntimeError,
                         "unable to get the current thread state");
         return NULL;
@@ -234,11 +235,12 @@ faulthandler_dump_traceback(int fd, int all_threads,
        PyGILState_GetThisThreadState(). */
     tstate = PyGILState_GetThisThreadState();
 #else
-    tstate = PyThreadState_Get();
+    tstate = _PyThreadState_UncheckedGet();
 #endif
 
-    if (all_threads)
-        _Py_DumpTracebackThreads(fd, interp, tstate);
+    if (all_threads) {
+        (void)_Py_DumpTracebackThreads(fd, NULL, tstate);
+    }
     else {
         if (tstate != NULL)
             _Py_DumpTraceback(fd, tstate);
@@ -272,7 +274,7 @@ faulthandler_dump_traceback_py(PyObject *self,
         return NULL;
 
     if (all_threads) {
-        errmsg = _Py_DumpTracebackThreads(fd, tstate->interp, tstate);
+        errmsg = _Py_DumpTracebackThreads(fd, NULL, tstate);
         if (errmsg != NULL) {
             PyErr_SetString(PyExc_RuntimeError, errmsg);
             return NULL;
@@ -469,7 +471,6 @@ faulthandler_thread(void *unused)
 {
     PyLockStatus st;
     const char* errmsg;
-    PyThreadState *current;
     int ok;
 #if defined(HAVE_PTHREAD_SIGMASK) && !defined(HAVE_BROKEN_PTHREAD_SIGMASK)
     sigset_t set;
@@ -489,12 +490,9 @@ faulthandler_thread(void *unused)
         /* Timeout => dump traceback */
         assert(st == PY_LOCK_FAILURE);
 
-        /* get the thread holding the GIL, NULL if no thread hold the GIL */
-        current = _PyThreadState_UncheckedGet();
-
         _Py_write_noraise(thread.fd, thread.header, (int)thread.header_len);
 
-        errmsg = _Py_DumpTracebackThreads(thread.fd, thread.interp, current);
+        errmsg = _Py_DumpTracebackThreads(thread.fd, thread.interp, NULL);
         ok = (errmsg == NULL);
 
         if (thread.exit)
@@ -894,7 +892,7 @@ static PyObject *
 faulthandler_sigsegv(PyObject *self, PyObject *args)
 {
     int release_gil = 0;
-    if (!PyArg_ParseTuple(args, "|i:_read_null", &release_gil))
+    if (!PyArg_ParseTuple(args, "|i:_sigsegv", &release_gil))
         return NULL;
 
     if (release_gil) {
@@ -907,6 +905,49 @@ faulthandler_sigsegv(PyObject *self, PyObject *args)
     Py_RETURN_NONE;
 }
 
+#ifdef WITH_THREAD
+static void
+faulthandler_fatal_error_thread(void *plock)
+{
+    PyThread_type_lock *lock = (PyThread_type_lock *)plock;
+
+    Py_FatalError("in new thread");
+
+    /* notify the caller that we are done */
+    PyThread_release_lock(lock);
+}
+
+static PyObject *
+faulthandler_fatal_error_c_thread(PyObject *self, PyObject *args)
+{
+    long thread;
+    PyThread_type_lock lock;
+
+    faulthandler_suppress_crash_report();
+
+    lock = PyThread_allocate_lock();
+    if (lock == NULL)
+        return PyErr_NoMemory();
+
+    PyThread_acquire_lock(lock, WAIT_LOCK);
+
+    thread = PyThread_start_new_thread(faulthandler_fatal_error_thread, lock);
+    if (thread == -1) {
+        PyThread_free_lock(lock);
+        PyErr_SetString(PyExc_RuntimeError, "unable to start the thread");
+        return NULL;
+    }
+
+    /* wait until the thread completes: it will never occur, since Py_FatalError()
+       exits the process immedialty. */
+    PyThread_acquire_lock(lock, WAIT_LOCK);
+    PyThread_release_lock(lock);
+    PyThread_free_lock(lock);
+
+    Py_RETURN_NONE;
+}
+#endif
+
 static PyObject *
 faulthandler_sigfpe(PyObject *self, PyObject *args)
 {
@@ -1065,6 +1106,11 @@ static PyMethodDef module_methods[] = {
                "a SIGSEGV or SIGBUS signal depending on the platform")},
     {"_sigsegv", faulthandler_sigsegv, METH_VARARGS,
      PyDoc_STR("_sigsegv(release_gil=False): raise a SIGSEGV signal")},
+#ifdef WITH_THREAD
+    {"_fatal_error_c_thread", faulthandler_fatal_error_c_thread, METH_NOARGS,
+     PyDoc_STR("fatal_error_c_thread(): "
+               "call Py_FatalError() in a new C thread.")},
+#endif
     {"_sigabrt", faulthandler_sigabrt, METH_NOARGS,
      PyDoc_STR("_sigabrt(): raise a SIGABRT signal")},
     {"_sigfpe", (PyCFunction)faulthandler_sigfpe, METH_NOARGS,
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 4fc6a15..41528cd 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -1275,25 +1275,11 @@ 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);
+    _Py_DumpTracebackThreads(fd, NULL, NULL);
 }
 
 /* Print the current exception (if an exception is set) with its traceback,
diff --git a/Python/pystate.c b/Python/pystate.c
index e8026c5..0503f32 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -714,6 +714,12 @@ _PyGILState_Init(PyInterpreterState *i, PyThreadState *t)
     _PyGILState_NoteThreadState(t);
 }
 
+PyInterpreterState *
+_PyGILState_GetInterpreterStateUnsafe(void)
+{
+    return autoInterpreterState;
+}
+
 void
 _PyGILState_Fini(void)
 {
diff --git a/Python/traceback.c b/Python/traceback.c
index 8383c16..403dba5 100644
--- a/Python/traceback.c
+++ b/Python/traceback.c
@@ -707,11 +707,56 @@ write_thread_id(int fd, PyThreadState *tstate, int is_current)
    handlers if signals were received. */
 const char*
 _Py_DumpTracebackThreads(int fd, PyInterpreterState *interp,
-                         PyThreadState *current_thread)
+                         PyThreadState *current_tstate)
 {
     PyThreadState *tstate;
     unsigned int nthreads;
 
+#ifdef WITH_THREAD
+    if (current_tstate == NULL) {
+        /* _Py_DumpTracebackThreads() is called from signal handlers by
+           faulthandler.
+
+           SIGSEGV, SIGFPE, SIGABRT, SIGBUS and SIGILL are synchronous signals
+           and are thus delivered to the thread that caused the fault. Get the
+           Python thread state of the current thread.
+
+           PyThreadState_Get() doesn't give the state of the thread that caused
+           the fault if the thread released the GIL, and so this function
+           cannot be used. Read the thread local storage (TLS) instead: call
+           PyGILState_GetThisThreadState(). */
+        current_tstate = PyGILState_GetThisThreadState();
+    }
+
+    if (interp == NULL) {
+        if (current_tstate == NULL) {
+            interp = _PyGILState_GetInterpreterStateUnsafe();
+            if (interp == NULL) {
+                /* We need the interpreter state to get Python threads */
+                return "unable to get the interpreter state";
+            }
+        }
+        else {
+            interp = current_tstate->interp;
+        }
+    }
+#else
+    if (current_tstate == NULL) {
+        /* Call _PyThreadState_UncheckedGet() instead of PyThreadState_Get()
+           to not fail with a fatal error if the thread state is NULL. */
+        current_thread = _PyThreadState_UncheckedGet();
+    }
+
+    if (interp == NULL) {
+        if (current_tstate == NULL) {
+            /* We need the interpreter state to get Python threads */
+            return "unable to get the interpreter state";
+        }
+        interp = current_tstate->interp;
+    }
+#endif
+    assert(interp != NULL);
+
     /* Get the current interpreter from the current thread */
     tstate = PyInterpreterState_ThreadHead(interp);
     if (tstate == NULL)
@@ -729,7 +774,7 @@ _Py_DumpTracebackThreads(int fd, PyInterpreterState *interp,
             PUTS(fd, "...\n");
             break;
         }
-        write_thread_id(fd, tstate, tstate == current_thread);
+        write_thread_id(fd, tstate, tstate == current_tstate);
         dump_traceback(fd, tstate, 0);
         tstate = PyThreadState_Next(tstate);
         nthreads++;