pythongh-128400: Only show the current thread in faulthandler if th…

…e GIL is disabled (pythonGH-128425)
WolframAlph · Jan 4, 2025 · 4b8a3c6 · 4b8a3c6
1 parent 126f683
commit 4b8a3c6
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 5 deletions.
diff --git a/Doc/library/faulthandler.rst b/Doc/library/faulthandler.rst
@@ -91,6 +91,10 @@ Fault handler state
       The dump now mentions if a garbage collector collection is running
       if *all_threads* is true.
 
+   .. versionchanged:: next
+      Only the current thread is dumped if the :term:`GIL` is disabled to
+      prevent the risk of data races.
+
 .. function:: disable()
 
    Disable the fault handler: uninstall the signal handlers installed by

diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py
@@ -100,7 +100,12 @@ def check_error(self, code, lineno, fatal_error, *,
 
         Raise an error if the output doesn't match the expected format.
         """
-        if all_threads:
+        all_threads_disabled = (
+            (not py_fatal_error)
+            and all_threads
+            and (not sys._is_gil_enabled())
+        )
+        if all_threads and not all_threads_disabled:
             if know_current_thread:
                 header = 'Current thread 0x[0-9a-f]+'
             else:
@@ -111,8 +116,10 @@ def check_error(self, code, lineno, fatal_error, *,
         if py_fatal_error:
             regex.append("Python runtime state: initialized")
         regex.append('')
+        if all_threads_disabled:
+            regex.append("<Cannot show all threads while the GIL is disabled>")
         regex.append(fr'{header} \(most recent call first\):')
-        if garbage_collecting:
+        if garbage_collecting and not all_threads_disabled:
             regex.append('  Garbage-collecting')
         regex.append(fr'  File "<string>", line {lineno} in {function}')
         regex = '\n'.join(regex)

diff --git a/Misc/NEWS.d/next/Library/2025-01-02-15-20-17.gh-issue-128400.UMiG4f.rst b/Misc/NEWS.d/next/Library/2025-01-02-15-20-17.gh-issue-128400.UMiG4f.rst
@@ -0,0 +1,2 @@
+Only show the current thread in :mod:`faulthandler` on the :term:`free
+threaded <free threading>` build to prevent races.
diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c
@@ -1,4 +1,5 @@
 #include "Python.h"
+#include "pycore_ceval.h"         // _PyEval_IsGILEnabled
 #include "pycore_initconfig.h"    // _PyStatus_ERR
 #include "pycore_pyerrors.h"      // _Py_DumpExtensionModules
 #include "pycore_pystate.h"       // _PyThreadState_GET()
@@ -27,6 +28,8 @@
 #  include <sys/auxv.h>           // getauxval()
 #endif
 
+/* Sentinel to ignore all_threads on free-threading */
+#define FT_IGNORE_ALL_THREADS 2
 
 /* Allocate at maximum 100 MiB of the stack to raise the stack overflow */
 #define STACK_OVERFLOW_MAX_SIZE (100 * 1024 * 1024)
@@ -201,10 +204,13 @@ faulthandler_dump_traceback(int fd, int all_threads,
        PyGILState_GetThisThreadState(). */
     PyThreadState *tstate = PyGILState_GetThisThreadState();
 
-    if (all_threads) {
+    if (all_threads == 1) {
         (void)_Py_DumpTracebackThreads(fd, NULL, tstate);
     }
     else {
+        if (all_threads == FT_IGNORE_ALL_THREADS) {
+            PUTS(fd, "<Cannot show all threads while the GIL is disabled>\n");
+        }
         if (tstate != NULL)
             _Py_DumpTraceback(fd, tstate);
     }
@@ -271,6 +277,27 @@ faulthandler_disable_fatal_handler(fault_handler_t *handler)
 #endif
 }
 
+static int
+deduce_all_threads(void)
+{
+#ifndef Py_GIL_DISABLED
+    return fatal_error.all_threads;
+#else
+    if (fatal_error.all_threads == 0) {
+        return 0;
+    }
+    // We can't use _PyThreadState_GET, so use the stored GILstate one
+    PyThreadState *tstate = PyGILState_GetThisThreadState();
+    if (tstate == NULL) {
+        return 0;
+    }
+
+    /* In theory, it's safe to dump all threads if the GIL is enabled */
+    return _PyEval_IsGILEnabled(tstate)
+        ? fatal_error.all_threads
+        : FT_IGNORE_ALL_THREADS;
+#endif
+}
 
 /* Handler for SIGSEGV, SIGFPE, SIGABRT, SIGBUS and SIGILL signals.
 
@@ -325,7 +352,7 @@ faulthandler_fatal_error(int signum)
         PUTS(fd, "\n\n");
     }
 
-    faulthandler_dump_traceback(fd, fatal_error.all_threads,
+    faulthandler_dump_traceback(fd, deduce_all_threads(),
                                 fatal_error.interp);
 
     _Py_DumpExtensionModules(fd, fatal_error.interp);
@@ -401,7 +428,7 @@ faulthandler_exc_handler(struct _EXCEPTION_POINTERS *exc_info)
         }
     }
 
-    faulthandler_dump_traceback(fd, fatal_error.all_threads,
+    faulthandler_dump_traceback(fd, deduce_all_threads(),
                                 fatal_error.interp);
 
     /* call the next exception handler */
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Only show the current thread in :mod:`faulthandler` on the :term:`free
		threaded <free threading>` build to prevent races.