From 05c5144d9a3c26ac7f3576776b62e29a64a4b7a8 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Mon, 7 Aug 2023 20:12:16 -0700 Subject: [PATCH] Serialize createdump core dump generation (#90130) * Serialize createdump core dump generation Only allow one thread at a time to generate a core dump. Issue: https://github.com/dotnet/runtime/issues/82989 * Code review feedback. Move serializing code into PROCCreateCrashDump * Code review feedback - put while (true) around poll()'s --- src/coreclr/pal/src/exception/signal.cpp | 11 +++++-- src/coreclr/pal/src/include/pal/process.h | 4 ++- src/coreclr/pal/src/thread/process.cpp | 51 +++++++++++++++++++++++++------ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index c3bdcc7..0da7f05 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -388,8 +388,15 @@ static void invoke_previous_action(struct sigaction* action, int code, siginfo_t { if (signalRestarts) { + // Shutdown and create the core dump before we restore the signal to the default handler. + PROCNotifyProcessShutdown(IsRunningOnAlternateStack(context)); + + PROCCreateCrashDumpIfEnabled(code, siginfo, true); + // Restore the original and restart h/w exception. restore_signal(code, action); + + return; } else { @@ -413,7 +420,7 @@ static void invoke_previous_action(struct sigaction* action, int code, siginfo_t PROCNotifyProcessShutdown(IsRunningOnAlternateStack(context)); - PROCCreateCrashDumpIfEnabled(code, siginfo); + PROCCreateCrashDumpIfEnabled(code, siginfo, true); } /*++ @@ -746,7 +753,7 @@ static void sigterm_handler(int code, siginfo_t *siginfo, void *context) DWORD val = 0; if (enableDumpOnSigTerm.IsSet() && enableDumpOnSigTerm.TryAsInteger(10, val) && val == 1) { - PROCCreateCrashDumpIfEnabled(code, siginfo); + PROCCreateCrashDumpIfEnabled(code, siginfo, false); } // g_pSynchronizationManager shouldn't be null if PAL is initialized. _ASSERTE(g_pSynchronizationManager != nullptr); diff --git a/src/coreclr/pal/src/include/pal/process.h b/src/coreclr/pal/src/include/pal/process.h index 71788cb..5b0cd07 100644 --- a/src/coreclr/pal/src/include/pal/process.h +++ b/src/coreclr/pal/src/include/pal/process.h @@ -178,10 +178,12 @@ Function: Parameters: signal - POSIX signal number + siginfo - POSIX signal info or nullptr + serialize - allow only one thread to generate core dump (no return value) --*/ -VOID PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo); +VOID PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, bool serialize); /*++ Function: diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index dcfc4c9..f9f2703 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -207,6 +207,9 @@ LPWSTR g_lpwstrAppDir = NULL; // Thread ID of thread that has started the ExitProcess process Volatile terminator = 0; +// Id of thread generating a core dump +Volatile g_crashingThreadId = 0; + // Process and session ID of this process. DWORD gPID = (DWORD) -1; DWORD gSID = (DWORD) -1; @@ -1194,7 +1197,10 @@ ExitProcess( Update: [TODO] PROCSuspendOtherThreads has been removed. Can this code be changed? */ WARN("termination already started from another thread; blocking.\n"); - poll(NULL, 0, INFTIM); + while (true) + { + poll(NULL, 0, INFTIM); + } } /* ExitProcess may be called even if PAL is not initialized. @@ -2175,20 +2181,40 @@ PROCBuildCreateDumpCommandLine( Function: PROCCreateCrashDump - Creates crash dump of the process. Can be called from the - unhandled native exception handler. + Creates crash dump of the process. Can be called from the unhandled + native exception handler. Allows only one thread to generate the core + dump if serialize is true. -(no return value) +Return: + TRUE - succeeds, FALSE - fails --*/ BOOL PROCCreateCrashDump( std::vector& argv, LPSTR errorMessageBuffer, - INT cbErrorMessageBuffer) + INT cbErrorMessageBuffer, + bool serialize) { _ASSERTE(argv.size() > 0); _ASSERTE(errorMessageBuffer == nullptr || cbErrorMessageBuffer > 0); + if (serialize) + { + size_t currentThreadId = THREADSilentGetCurrentThreadId(); + size_t previousThreadId = InterlockedCompareExchange(&g_crashingThreadId, currentThreadId, 0); + if (previousThreadId != 0) + { + // Should never reenter or recurse + _ASSERTE(previousThreadId != currentThreadId); + + // The first thread generates the crash info and any other threads are blocked + while (true) + { + poll(NULL, 0, INFTIM); + } + } + } + int pipe_descs[2]; if (pipe(pipe_descs) == -1) { @@ -2413,7 +2439,7 @@ PAL_GenerateCoreDump( BOOL result = PROCBuildCreateDumpCommandLine(argvCreateDump, &program, &pidarg, dumpName, nullptr, dumpType, flags); if (result) { - result = PROCCreateCrashDump(argvCreateDump, errorMessageBuffer, cbErrorMessageBuffer); + result = PROCCreateCrashDump(argvCreateDump, errorMessageBuffer, cbErrorMessageBuffer, false); } free(program); free(pidarg); @@ -2429,11 +2455,13 @@ Function: Parameters: signal - POSIX signal number + siginfo - POSIX signal info or nullptr + serialize - allow only one thread to generate core dump (no return value) --*/ VOID -PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo) +PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo, bool serialize) { // If enabled, launch the create minidump utility and wait until it completes if (!g_argvCreateDump.empty()) @@ -2491,7 +2519,7 @@ PROCCreateCrashDumpIfEnabled(int signal, siginfo_t* siginfo) argv.push_back(nullptr); } - PROCCreateCrashDump(argv, nullptr, 0); + PROCCreateCrashDump(argv, nullptr, 0, serialize); free(signalArg); free(crashThreadArg); @@ -2520,7 +2548,7 @@ PROCAbort(int signal, siginfo_t* siginfo) // Do any shutdown cleanup before aborting or creating a core dump PROCNotifyProcessShutdown(); - PROCCreateCrashDumpIfEnabled(signal, siginfo); + PROCCreateCrashDumpIfEnabled(signal, siginfo, true); // Restore all signals; the SIGABORT handler to prevent recursion and // the others to prevent multiple core dumps from being generated. @@ -3222,7 +3250,10 @@ CorUnix::TerminateCurrentProcessNoExit(BOOL bTerminateUnconditionally) TerminateProcess won't call DllMain, so there's no danger to get caught in an infinite loop */ WARN("termination already started from another thread; blocking.\n"); - poll(NULL, 0, INFTIM); + while (true) + { + poll(NULL, 0, INFTIM); + } } /* Try to lock the initialization count to prevent multiple threads from -- 2.7.4