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

diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp
index c3bdcc7..0da7f05 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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<LONG> terminator = 0;
 
+// Id of thread generating a core dump
+Volatile<LONG> 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<const char*>& 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