gh-128400: Only show the current thread in `faulthandler` if the GIL is disabled (GH-128425)
https://github.com/python/cpython/commit/befcfdfdaba15ecae38739ecabebd8046c1... commit: befcfdfdaba15ecae38739ecabebd8046c1b1977 branch: main author: Peter Bierma <zintensitydev@gmail.com> committer: colesbury <colesbury@gmail.com> date: 2025-01-03T14:14:57-05:00 summary: gh-128400: Only show the current thread in `faulthandler` if the GIL is disabled (GH-128425) files: A Misc/NEWS.d/next/Library/2025-01-02-15-20-17.gh-issue-128400.UMiG4f.rst M Doc/library/faulthandler.rst M Lib/test/test_faulthandler.py M Modules/faulthandler.c diff --git a/Doc/library/faulthandler.rst b/Doc/library/faulthandler.rst index 4067d7912b88b2..b81da4af3cff58 100644 --- 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 index fd56dee5d842ac..2088793cbb9387 100644 --- 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 new file mode 100644 index 00000000000000..f9d5f84224c8dc --- /dev/null +++ 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 index 2d16028a5232d0..b44b964b29484b 100644 --- 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 */
participants (1)
-
colesbury