Fix shutdown with multiple unhandled HW exceptions (#105578)

When multiple threads crash with hardware unhandled exceptions at the
same time, the fact that we were uninstalling async signal handlers
at process exit caused crashes when some thread reached the signal
handler after .NET handler was removed.

This change fixes it by not restoring the signal handlers during
process exit. It actually stops restoring any signal handlers except
for SIGABRT that has to be restored to actually enable the process
exit with abort().

Close #46175

Author: janvorli

src/coreclr/pal/src/exception/seh.cpp

@@ -109,7 +109,7 @@ SEHCleanup()
 {
     TRACE("Cleaning up SEH\n");
 
-    SEHCleanupSignals();
+    SEHCleanupSignals(false /* isChildProcess */);
 }
 
 /*++

src/coreclr/pal/src/exception/signal.cpp

@@ -279,7 +279,7 @@ Function :
     Restore default signal handlers
 
 Parameters :
-    None
+    isChildProcess - indicates that it is called from a child process fork
 
     (no return value)
 
@@ -288,34 +288,42 @@ reason for this function is that during PAL_Terminate, we reach a point where
 SEH isn't possible anymore (handle manager is off, etc). Past that point,
 we can't avoid crashing on a signal.
 --*/
-void SEHCleanupSignals()
+void SEHCleanupSignals(bool isChildProcess)
 {
     TRACE("Restoring default signal handlers\n");
 
-    if (g_registered_signal_handlers)
+    if (isChildProcess)
     {
-        restore_signal(SIGILL, &g_previous_sigill);
+        if (g_registered_signal_handlers)
+        {
+            restore_signal(SIGILL, &g_previous_sigill);
 #if !HAVE_MACH_EXCEPTIONS
-        restore_signal(SIGTRAP, &g_previous_sigtrap);
+            restore_signal(SIGTRAP, &g_previous_sigtrap);
 #endif
-        restore_signal(SIGFPE, &g_previous_sigfpe);
-        restore_signal(SIGBUS, &g_previous_sigbus);
-        restore_signal(SIGABRT, &g_previous_sigabrt);
-        restore_signal(SIGSEGV, &g_previous_sigsegv);
-        restore_signal(SIGINT, &g_previous_sigint);
-        restore_signal(SIGQUIT, &g_previous_sigquit);
-    }
+            restore_signal(SIGFPE, &g_previous_sigfpe);
+            restore_signal(SIGBUS, &g_previous_sigbus);
+            restore_signal(SIGSEGV, &g_previous_sigsegv);
+            restore_signal(SIGINT, &g_previous_sigint);
+            restore_signal(SIGQUIT, &g_previous_sigquit);
+        }
 
 #ifdef INJECT_ACTIVATION_SIGNAL
-    if (g_registered_activation_handler)
-    {
-        restore_signal(INJECT_ACTIVATION_SIGNAL, &g_previous_activation);
-    }
+        if (g_registered_activation_handler)
+        {
+            restore_signal(INJECT_ACTIVATION_SIGNAL, &g_previous_activation);
+        }
 #endif
 
-    if (g_registered_sigterm_handler)
+        if (g_registered_sigterm_handler)
+        {
+            restore_signal(SIGTERM, &g_previous_sigterm);
+        }
+    }
+
+    // Restore only the SIGABRT so that abort that ends up the process can actually end it
+    if (g_registered_signal_handlers)
     {
-        restore_signal(SIGTERM, &g_previous_sigterm);
+        restore_signal(SIGABRT, &g_previous_sigabrt);
     }
 }
 

src/coreclr/pal/src/include/pal/signal.hpp

@@ -112,10 +112,12 @@ Function :
     SEHCleanupSignals
 
     Restore default signal handlers
+Parameters :
+    isChildProcess - indicates that it is called from a child process fork
 
     (no parameters, no return value)
 --*/
-void SEHCleanupSignals();
+void SEHCleanupSignals(bool isChildProcess);
 
 /*++
 Function :

src/coreclr/pal/src/thread/process.cpp

@@ -2257,7 +2257,7 @@ PROCCreateCrashDump(
         if (g_createdumpCallback != nullptr)
         {
             // Remove the signal handlers inherited from the runtime process
-            SEHCleanupSignals();
+            SEHCleanupSignals(true /* isChildProcess */);
 
             // Call the statically linked createdump code
             g_createdumpCallback(argv.size(), argv.data());
@@ -2556,7 +2556,7 @@ PROCAbort(int signal, siginfo_t* siginfo)
 
     // Restore all signals; the SIGABORT handler to prevent recursion and
     // the others to prevent multiple core dumps from being generated.
-    SEHCleanupSignals();
+    SEHCleanupSignals(false /* isChildProcess */);
 
     // Abort the process after waiting for the core dump to complete
     abort();

src/tests/baseservices/exceptions/stackoverflow/stackoverflowtester.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Runtime.InteropServices;
 using System.Text;
 using Xunit;
 
@@ -22,12 +23,27 @@ static void TestStackOverflow(string testName, string testArgs, out List<string>
             testProcess.StartInfo.Arguments = $"{Path.Combine(Directory.GetCurrentDirectory(), "..", testName, $"{testName}.dll")} {testArgs}";
             testProcess.StartInfo.UseShellExecute = false;
             testProcess.StartInfo.RedirectStandardError = true;
+            testProcess.StartInfo.Environment.Add("DOTNET_DbgEnableMiniDump", "0");
+            bool endOfStackTrace = false;
+            
             testProcess.ErrorDataReceived += (sender, line) => 
             {
                 Console.WriteLine($"\"{line.Data}\"");
-                if (!string.IsNullOrEmpty(line.Data))
+                if (!endOfStackTrace && !string.IsNullOrEmpty(line.Data))
                 {
-                    lines.Add(line.Data);
+                    // Store lines only till the end of the stack trace.
+                    // In the CI it can also contain lines with createdump info.
+                    if (line.Data.StartsWith("Stack overflow.") ||
+                        line.Data.StartsWith("Repeated ") ||
+                        line.Data.StartsWith("------") ||
+                        line.Data.StartsWith("   at "))
+                    {
+                        lines.Add(line.Data);
+                    }
+                    else
+                    {
+                        endOfStackTrace = true;
+                    }
                 }
             };
 
@@ -99,6 +115,15 @@ public static void TestStackOverflowSmallFrameMainThread()
         [Fact]
         public static void TestStackOverflowLargeFrameMainThread()
         {
+            if ((RuntimeInformation.ProcessArchitecture == Architecture.Arm64) &&
+                ((Environment.OSVersion.Platform == PlatformID.Unix) || (Environment.OSVersion.Platform == PlatformID.MacOSX)))
+            {
+                // Disabled on Unix ARM64 due to https://github.com/dotnet/runtime/issues/13519
+                // The current stack probing doesn't move the stack pointer and so the runtime sometimes cannot
+                // recognize the underlying sigsegv as stack overflow when it probes too far from SP.
+                return;
+            }
+
             TestStackOverflow("stackoverflow", "largeframe main", out List<string> lines);
 
             if (!lines[lines.Count - 1].EndsWith("at TestStackOverflow.Program.Main(System.String[])"))
@@ -156,6 +181,15 @@ public static void TestStackOverflowSmallFrameSecondaryThread()
         [Fact]
         public static void TestStackOverflowLargeFrameSecondaryThread()
         {
+            if ((RuntimeInformation.ProcessArchitecture == Architecture.Arm64) &&
+                ((Environment.OSVersion.Platform == PlatformID.Unix) || (Environment.OSVersion.Platform == PlatformID.MacOSX)))
+            {
+                // Disabled on Unix ARM64 due to https://github.com/dotnet/runtime/issues/13519
+                // The current stack probing doesn't move the stack pointer and so the runtime sometimes cannot
+                // recognize the underlying sigsegv as stack overflow when it probes too far from SP.
+                return;
+            }
+
             TestStackOverflow("stackoverflow", "largeframe secondary", out List<string> lines);
 
             if (!lines.Exists(elem => elem.EndsWith("at TestStackOverflow.Program.Test(Boolean)")))

src/tests/issues.targets

@@ -89,9 +89,6 @@
 
     <!-- All Unix targets  on CoreCLR Runtime -->
     <ItemGroup Condition="'$(XunitTestBinBase)' != '' and '$(TargetsWindows)' != 'true' and '$(RuntimeFlavor)' == 'coreclr' ">
-        <ExcludeList Include="$(XunitTestBinBase)/baseservices/exceptions/stackoverflow/stackoverflowtester/*">
-            <Issue>https://github.com/dotnet/runtime/issues/46175</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/baseservices/typeequivalence/signatures/nopiatestil/*">
             <Issue>CoreCLR doesn't support type equivalence on Unix</Issue>
         </ExcludeList>