Prevent SOS CLR notification failures from wedging cdb

src/SOS/Strike/exts.cpp

@@ -298,6 +298,11 @@ void
 DACMessage(HRESULT Status)
 {
     ExtOut("Failed to load data access module, 0x%08x\n", Status);
+    HRESULT fallbackFailure = GetDacFallbackFailure();
+    if (FAILED(fallbackFailure))
+    {
+        ExtOut("The dbgeng data access fallback also failed to provide ISOSDacInterface, 0x%08x\n", fallbackFailure);
+    }
     if (g_pRuntime->GetRuntimeConfiguration() >= IRuntime::ConfigurationEnd)
     {
         ExtOut("Unknown runtime type. Command not supported.\n");

src/SOS/Strike/strike.cpp

@@ -5877,6 +5877,40 @@ BOOL CheckCLRNotificationEvent(DEBUG_LAST_EVENT_INFO_EXCEPTION* pdle)
 #endif
 }
 
+static bool ShouldReportCLRNotificationFailure()
+{
+#ifndef FEATURE_PAL
+    static ULONG s_processId = 0;
+    static ULONG64 s_processHandle = 0;
+    static bool s_reportedFailure = false;
+
+    ULONG processId = 0;
+    ULONG64 processHandle = 0;
+    if (g_ExtSystem->GetCurrentProcessSystemId(&processId) != S_OK ||
+        g_ExtSystem->GetCurrentProcessHandle(&processHandle) != S_OK)
+    {
+        return true;
+    }
+    if (processId != s_processId || processHandle != s_processHandle)
+    {
+        s_processId = processId;
+        s_processHandle = processHandle;
+        s_reportedFailure = false;
+    }
+    if (s_reportedFailure)
+    {
+        return false;
+    }
+    s_reportedFailure = true;
+#endif
+    return true;
+}
+
+static void ReportCLRNotificationFailure(HRESULT status)
+{
+    ExtErr("SOS CLR notification handler failed, 0x%08x; attempting to continue the target\n", status);
+}
+
 HRESULT HandleCLRNotificationEvent()
 {
     /*
@@ -5892,7 +5926,11 @@ HRESULT HandleCLRNotificationEvent()
     if (!CheckCLRNotificationEvent(&dle))
     {
 #ifndef FEATURE_PAL
-        ExtOut("Expecting first chance CLRN exception\n");
+        if (ShouldReportCLRNotificationFailure())
+        {
+            ReportCLRNotificationFailure(E_FAIL);
+            ExtOut("Expecting first chance CLRN exception\n");
+        }
         return E_FAIL;
 #else
         g_ExtControl->Execute(DEBUG_OUTCTL_NOT_LOGGED, "process continue", 0);
@@ -5905,7 +5943,11 @@ HRESULT HandleCLRNotificationEvent()
     HRESULT Status = g_clrData->TranslateExceptionRecordToNotification(&dle.ExceptionRecord, &Notification);
     if (Status != S_OK)
     {
-        ExtErr("Error processing exception notification\n");
+        if (ShouldReportCLRNotificationFailure())
+        {
+            ReportCLRNotificationFailure(Status);
+            ExtErr("Error processing exception notification\n");
+        }
         return Status;
     }
     else
@@ -5916,11 +5958,10 @@ HRESULT HandleCLRNotificationEvent()
             case DEBUG_STATUS_GO_HANDLED:
             case DEBUG_STATUS_GO_NOT_HANDLED:
 #ifndef FEATURE_PAL
-                g_ExtControl->Execute(DEBUG_OUTCTL_NOT_LOGGED, "g", 0);
+                return g_ExtControl->Execute(DEBUG_OUTCTL_NOT_LOGGED, "g", 0);
 #else
-                g_ExtControl->Execute(DEBUG_OUTCTL_NOT_LOGGED, "process continue", 0);
+                return g_ExtControl->Execute(DEBUG_OUTCTL_NOT_LOGGED, "process continue", 0);
 #endif
-                break;
             default:
                 break;
         }
@@ -5941,11 +5982,89 @@ void EnableModuleLoadUnloadCallbacks()
 
 #ifndef FEATURE_PAL
 
+class CLRNotificationResumeGuard
+{
+    ToRelease<IDebugControl2> m_control;
+    bool m_resume;
+
+public:
+    CLRNotificationResumeGuard(IDebugClient* client)
+        : m_resume(false)
+    {
+        if (FAILED(client->QueryInterface(__uuidof(IDebugControl2), (void**)&m_control)))
+        {
+            return;
+        }
+
+        ULONG debugClass = DEBUG_CLASS_UNINITIALIZED;
+        ULONG qualifier = 0;
+        if (m_control->GetDebuggeeType(&debugClass, &qualifier) == S_OK &&
+            debugClass == DEBUG_CLASS_USER_WINDOWS &&
+            qualifier < DEBUG_DUMP_SMALL)
+        {
+            m_resume = true;
+        }
+    }
+
+    ~CLRNotificationResumeGuard()
+    {
+        if (m_resume)
+        {
+            // Execute can invoke this callback reentrantly while g waits for the next event.
+            m_resume = false;
+            m_control->Execute(DEBUG_OUTCTL_NOT_LOGGED, "g", 0);
+        }
+    }
+
+    void SuppressResume()
+    {
+        m_resume = false;
+    }
+};
+
 DECLARE_API(SOSHandleCLRN)
 {
-    INIT_API();
+    INIT_API_EXT();
+    CLRNotificationResumeGuard resumeGuard(client);
+    if ((Status = ArchQuery()) != S_OK)
+    {
+        if (ShouldReportCLRNotificationFailure())
+        {
+            ReportCLRNotificationFailure(Status);
+        }
+        return Status;
+    }
+    if ((Status = GetRuntime(&g_pRuntime)) != S_OK)
+    {
+        if (ShouldReportCLRNotificationFailure())
+        {
+            ReportCLRNotificationFailure(Status);
+            EENotLoadedMessage(Status);
+        }
+        return Status;
+    }
+    if ((Status = LoadClrDebugDll()) != S_OK)
+    {
+        if (ShouldReportCLRNotificationFailure())
+        {
+            ReportCLRNotificationFailure(Status);
+            DACMessage(Status);
+        }
+        return Status;
+    }
+    g_bDacBroken = FALSE;
+    ToRelease<IXCLRDataProcess> spIDP(g_clrData);
+    ToRelease<ISOSDacInterface> spISD(g_sos);
+    ToRelease<ISOSDacInterface15> spISD15(g_sos15);
+    ToRelease<ISOSDacInterface16> spISD16(g_sos16);
+    ResetGlobals();
     MINIDUMP_NOT_SUPPORTED();
-    return HandleCLRNotificationEvent();
+    Status = HandleCLRNotificationEvent();
+    if (SUCCEEDED(Status))
+    {
+        resumeGuard.SuppressResume();
+    }
+    return Status;
 }
 
 HRESULT HandleRuntimeLoadedNotification(IDebugClient* client)

src/SOS/Strike/util.cpp

@@ -3807,10 +3807,20 @@ class SOSDacInterface15Simulator : public ISOSDacInterface15
 // Return Value:
 //      HRESULT indicating success or failure
 //
+static HRESULT s_dacFallbackFailure = S_OK;
+
+HRESULT GetDacFallbackFailure(void)
+{
+    return s_dacFallbackFailure;
+}
+
 HRESULT LoadClrDebugDll(void)
 {
     _ASSERTE(g_pRuntime != nullptr);
+    s_dacFallbackFailure = S_OK;
     HRESULT hr = g_pRuntime->GetClrDataProcess(IRuntime::ClrDataProcessFlags::UseCDac, &g_clrData);
+    HRESULT primaryFailure = hr;
+    bool usedDbgEngFallback = false;
     if (FAILED(hr))
     {
         if (Runtime::GetCDacLoadPolicy() == CDacLoadPolicy::UseCDac)
@@ -3822,6 +3832,7 @@ HRESULT LoadClrDebugDll(void)
         {
             return hr;
         }
+        usedDbgEngFallback = true;
     }
     else
     {
@@ -3832,6 +3843,13 @@ HRESULT LoadClrDebugDll(void)
     if (FAILED(hr))
     {
         g_sos = NULL;
+        g_clrData->Release();
+        g_clrData = NULL;
+        if (usedDbgEngFallback)
+        {
+            s_dacFallbackFailure = hr;
+            return primaryFailure;
+        }
         return hr;
     }
     // Always have an instance of the MethodTable enumerator

src/SOS/Strike/util.h

@@ -1683,6 +1683,7 @@ inline BOOL IsKernelDebugger ()
 
 void    ResetGlobals(void);
 HRESULT LoadClrDebugDll(void);
+HRESULT GetDacFallbackFailure(void);
 
 extern IMetaDataImport* MDImportForModule (DacpModuleData *pModule);
 extern IMetaDataImport* MDImportForModule (DWORD_PTR pModule);

src/tests/SOS.UnitTests/Debuggees/CLRNotificationHandlerFailure/CLRNotificationHandlerFailure.csproj

@@ -0,0 +1,9 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework Condition="'$(BuildProjectFramework)' != ''">$(BuildProjectFramework)</TargetFramework>
+    <TargetFrameworks Condition="'$(BuildProjectFramework)' == ''">$(SupportedSubProcessTargetFrameworks)</TargetFrameworks>
+  </PropertyGroup>
+
+</Project>

src/tests/SOS.UnitTests/Debuggees/CLRNotificationHandlerFailure/Program.cs

@@ -0,0 +1,33 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.Reflection;
+using System.Reflection.Emit;
+using System.Runtime.CompilerServices;
+
+namespace CLRNotificationHandlerFailure
+{
+    internal static class Program
+    {
+        private static void Main()
+        {
+            TriggerNotification();
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        internal static void TriggerNotification()
+        {
+            for (int index = 0; index < 4; index++)
+            {
+                AssemblyBuilder assembly = AssemblyBuilder.DefineDynamicAssembly(
+                    new AssemblyName($"CLRNotificationHandlerFailure.Dynamic{index}"),
+                    AssemblyBuilderAccess.Run);
+                ModuleBuilder module = assembly.DefineDynamicModule($"Dynamic{index}");
+                TypeBuilder type = module.DefineType($"DynamicType{index}");
+                type.CreateType();
+            }
+            Debugger.Break();
+        }
+    }
+}

src/tests/SOS.UnitTests/SOS.cs

@@ -60,6 +60,31 @@ public static IEnumerable<object[]> GetNetCoreConfigurations()
             .Select(c => new[] { c });
     }
 
+    public static IEnumerable<object[]> GetCLRNotificationFailureConfigurations()
+    {
+        if (OS.Kind != OSKind.Windows)
+        {
+            return new[] { new object[] { TestConfiguration.Empty } };
+        }
+
+        List<object[]> configurations = Configurations
+            .Select(args => (TestConfiguration)args[0])
+            .Where(c => c != TestConfiguration.Empty && c.IsNETCore && !c.PublishSingleFile && c.RuntimeFrameworkVersionMajor < 11)
+            .OrderByDescending(c => c.RuntimeFrameworkVersion)
+            .Take(1)
+            .Select(c =>
+            {
+                Dictionary<string, string> settings = new(c.AllSettings)
+                {
+                    ["HostArgs"] = "exec " + c.HostArgs
+                };
+                return new TestConfiguration(settings);
+            })
+            .Select(c => new object[] { c })
+            .ToList();
+        return configurations.Count == 0 ? new[] { new object[] { TestConfiguration.Empty } } : configurations;
+    }
+
     public static IEnumerable<object[]> GetGCConfigurations()
     {
         IEnumerable<TestConfiguration> inputConfigurations = TestRunConfiguration.Instance.Configurations
@@ -670,6 +695,46 @@ await SOSTestHelpers.RunTest(
             },
             Output);
     }
+
+    [SkippableTheory, MemberData(nameof(SOSTestHelpers.GetCLRNotificationFailureConfigurations), MemberType = typeof(SOSTestHelpers))]
+    public async Task CLRNotificationHandlerFailure(TestConfiguration config)
+    {
+        if (config == TestConfiguration.Empty)
+        {
+            throw new SkipTestException("CLR notification failure coverage requires Windows and a pre-.NET 11 runtime");
+        }
+
+        string sourceSosPath = config.SOSPath();
+        string isolatedSosDirectory = Path.Combine(config.LogDirPath, "CLRNotificationHandlerFailure.SOS");
+        // Omitting the bundled cDAC makes the forced cDAC failure independent of the build layout.
+        Directory.CreateDirectory(isolatedSosDirectory);
+        foreach (string sourcePath in Directory.EnumerateFiles(Path.GetDirectoryName(sourceSosPath)))
+        {
+            if (!Path.GetFileName(sourcePath).Equals("mscordaccore_universal.dll", StringComparison.OrdinalIgnoreCase))
+            {
+                File.Copy(sourcePath, Path.Combine(isolatedSosDirectory, Path.GetFileName(sourcePath)), overwrite: true);
+            }
+        }
+        Dictionary<string, string> settings = new(config.AllSettings)
+        {
+            ["SOSPath"] = Path.Combine(isolatedSosDirectory, Path.GetFileName(sourceSosPath))
+        };
+        config = new TestConfiguration(settings);
+
+        await SOSTestHelpers.RunTest(
+            scriptName: "CLRNotificationHandlerFailure.script",
+            new SOSRunner.TestInformation
+            {
+                TestConfiguration = config,
+                TestName = "SOS.CLRNotificationHandlerFailure",
+                DebuggeeName = "CLRNotificationHandlerFailure",
+                TestDump = false,
+                ApplyDacModeOnDemand = true,
+                DacModeOverride = DacMode.CDac,
+                FailOnCLRNotificationStop = true,
+            },
+            Output);
+    }
 }
 
 public class SOSMethodTests