Don't ignore exceptions thrown from handlers of some events (dotnet/coreclr#10502)

Fixes dotnet/corefxdotnet/coreclr#14747:
- Events include: AssemblyLoadContext.Unloading, AppDomain.ProcessExit
- Made the same change for AppDomain.DomainUnload for consistency, but it's not raised

Commit migrated from https://github.com/dotnet/coreclr/commit/54b2c88f5a8c4623a4a465218a9cef94b92642bf

diff --git a/src/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp
index c50d776..8e79018 100644 (file)
--- a/src/coreclr/src/vm/appdomain.cpp
+++ b/src/coreclr/src/vm/appdomain.cpp
@@ -7475,7 +7475,7 @@ void AppDomain::ProcessUnloadDomainEventOnFinalizeThread()
 {
     CONTRACTL
     {
-        NOTHROW;
+        THROWS;
         GC_TRIGGERS;
         MODE_COOPERATIVE;
     }
@@ -7512,45 +7512,37 @@ void AppDomain::RaiseUnloadDomainEvent()
 {
     CONTRACTL
     {
-        NOTHROW;
+        THROWS;
         MODE_COOPERATIVE;
         GC_TRIGGERS;
         SO_INTOLERANT;
     }
     CONTRACTL_END;
 
-    EX_TRY
+    Thread *pThread = GetThread();
+    if (this != pThread->GetDomain())
     {
-        Thread *pThread = GetThread();
-        if (this != pThread->GetDomain())
+        pThread->DoADCallBack(this, AppDomain::RaiseUnloadDomainEvent_Wrapper, this,ADV_FINALIZER|ADV_COMPILATION);
+    }
+    else
+    {
+        struct _gc
         {
-            pThread->DoADCallBack(this, AppDomain::RaiseUnloadDomainEvent_Wrapper, this,ADV_FINALIZER|ADV_COMPILATION);
-        }
-        else
+            APPDOMAINREF Domain;
+            OBJECTREF    Delegate;
+        } gc;
+        ZeroMemory(&gc, sizeof(gc));
+
+        GCPROTECT_BEGIN(gc);
+        gc.Domain = (APPDOMAINREF) GetRawExposedObject();
+        if (gc.Domain != NULL)
         {
-            struct _gc
-            {
-                APPDOMAINREF Domain;
-                OBJECTREF    Delegate;
-            } gc;
-            ZeroMemory(&gc, sizeof(gc));
-
-            GCPROTECT_BEGIN(gc);
-            gc.Domain = (APPDOMAINREF) GetRawExposedObject();
-            if (gc.Domain != NULL)
-            {
-                gc.Delegate = gc.Domain->m_pDomainUnloadEventHandler;
-                if (gc.Delegate != NULL)
-                    DistributeEventReliably(&gc.Delegate, (OBJECTREF *) &gc.Domain);
-            }
-            GCPROTECT_END();
+            gc.Delegate = gc.Domain->m_pDomainUnloadEventHandler;
+            if (gc.Delegate != NULL)
+                DistributeEvent(&gc.Delegate, (OBJECTREF *) &gc.Domain);
         }
+        GCPROTECT_END();
     }
-    EX_CATCH
-    {
-        //@TODO call a MDA here
-    }
-    EX_END_CATCH(SwallowAllExceptions);
 }
 
 void AppDomain::RaiseLoadingAssemblyEvent(DomainAssembly *pAssembly)
@@ -7669,20 +7661,15 @@ void AppDomain::RaiseOneExitProcessEvent()
     } gc;
     ZeroMemory(&gc, sizeof(gc));
 
-    EX_TRY {
-
-        GCPROTECT_BEGIN(gc);
-        gc.Domain = (APPDOMAINREF) SystemDomain::GetCurrentDomain()->GetRawExposedObject();
-        if (gc.Domain != NULL)
-        {
-            gc.Delegate = gc.Domain->m_pProcessExitEventHandler;
-            if (gc.Delegate != NULL)
-                DistributeEventReliably(&gc.Delegate, (OBJECTREF *) &gc.Domain);
-        }
-        GCPROTECT_END();
-
-    } EX_CATCH {
-    } EX_END_CATCH(SwallowAllExceptions);
+    GCPROTECT_BEGIN(gc);
+    gc.Domain = (APPDOMAINREF) SystemDomain::GetCurrentDomain()->GetRawExposedObject();
+    if (gc.Domain != NULL)
+    {
+        gc.Delegate = gc.Domain->m_pProcessExitEventHandler;
+        if (gc.Delegate != NULL)
+            DistributeEvent(&gc.Delegate, (OBJECTREF *) &gc.Domain);
+    }
+    GCPROTECT_END();
 }
 
 // Local wrapper used in AppDomain::RaiseExitProcessEvent,
@@ -7691,17 +7678,13 @@ void AppDomain::RaiseOneExitProcessEvent()
 // because it calls private RaiseOneExitProcessEvent
 /*static*/ void AppDomain::RaiseOneExitProcessEvent_Wrapper(AppDomainIterator* pi)
 {
-
     STATIC_CONTRACT_MODE_COOPERATIVE;
-    STATIC_CONTRACT_NOTHROW;
+    STATIC_CONTRACT_THROWS;
     STATIC_CONTRACT_GC_TRIGGERS;
 
-    EX_TRY {
-        ENTER_DOMAIN_PTR(pi->GetDomain(),ADV_ITERATOR)
-        AppDomain::RaiseOneExitProcessEvent();
-        END_DOMAIN_TRANSITION;
-    } EX_CATCH {
-    } EX_END_CATCH(SwallowAllExceptions);
+    ENTER_DOMAIN_PTR(pi->GetDomain(), ADV_ITERATOR)
+    AppDomain::RaiseOneExitProcessEvent();
+    END_DOMAIN_TRANSITION;
 }
 
 static LONG s_ProcessedExitProcessEventCount = 0;
@@ -7718,7 +7701,7 @@ void AppDomain::RaiseExitProcessEvent()
         return;
 
     STATIC_CONTRACT_MODE_COOPERATIVE;
-    STATIC_CONTRACT_NOTHROW;
+    STATIC_CONTRACT_THROWS;
     STATIC_CONTRACT_GC_TRIGGERS;
 
     // Only finalizer thread during shutdown can call this function.

diff --git a/src/coreclr/src/vm/comdelegate.cpp b/src/coreclr/src/vm/comdelegate.cpp
index 075c473..20ae695 100644 (file)
--- a/src/coreclr/src/vm/comdelegate.cpp
+++ b/src/coreclr/src/vm/comdelegate.cpp
@@ -3960,55 +3960,12 @@ static void InvokeUnhandledSwallowing(OBJECTREF *pDelegate,
 }
 
 
-// cannot combine SEH & C++ exceptions in one method.  Split out from InvokeNotify.
-static void InvokeNotifyInner(OBJECTREF *pDelegate, OBJECTREF *pDomain)
-{
-    // static contract, since we use SEH.
-    STATIC_CONTRACT_GC_TRIGGERS;
-    STATIC_CONTRACT_THROWS;
-    STATIC_CONTRACT_MODE_COOPERATIVE;
-
-    _ASSERTE(pDelegate  != NULL && IsProtectedByGCFrame(pDelegate));
-    _ASSERTE(pDomain    != NULL && IsProtectedByGCFrame(pDomain));
-
-    struct Param : ThreadBaseExceptionFilterParam
-    {
-        OBJECTREF *pDelegate;
-        OBJECTREF *pDomain;
-    } param;
-    param.location = SystemNotification;
-    param.pDelegate = pDelegate;
-    param.pDomain = pDomain;
-
-    PAL_TRY(Param *, pParam, &param)
-    {
-        PREPARE_NONVIRTUAL_CALLSITE_USING_CODE(DELEGATEREF(*pParam->pDelegate)->GetMethodPtr());
-
-        DECLARE_ARGHOLDER_ARRAY(args, 3);
-
-        args[ARGNUM_0] = OBJECTREF_TO_ARGHOLDER(DELEGATEREF(*pParam->pDelegate)->GetTarget());
-        args[ARGNUM_1] = OBJECTREF_TO_ARGHOLDER(*pParam->pDomain);
-        args[ARGNUM_2] = NULL;
-
-        CALL_MANAGED_METHOD_NORET(args);
-    }
-    PAL_EXCEPT_FILTER(ThreadBaseExceptionFilter)
-    {
-        _ASSERTE(!"ThreadBaseExceptionFilter returned EXECUTE_HANDLER.");
-    }
-    PAL_ENDTRY;
-}
-
-
-
-// Helper to dispatch a single event notification.  If anything goes wrong, we cause
-// an unhandled exception notification to occur out of our first pass, and then we
-// swallow and continue.
+// Helper to dispatch a single event notification.
 static void InvokeNotify(OBJECTREF *pDelegate, OBJECTREF *pDomain)
 {
     CONTRACTL
     {
-        NOTHROW;
+        THROWS;
         GC_TRIGGERS;
         MODE_COOPERATIVE;
     }
@@ -4030,19 +3987,15 @@ static void InvokeNotify(OBJECTREF *pDelegate, OBJECTREF *pDomain)
     _ASSERTE(!pThread->HasCriticalRegion());
     _ASSERTE(!pThread->HasThreadAffinity());
 
-    EX_TRY
-    {
-        InvokeNotifyInner(pDelegate, pDomain);
-    }
-    EX_CATCH
-    {
-        // It's not even worth asserting, because these aren't our bugs.  At
-        // some point, a MDA may be warranted.
-        // This is an early check for condition that we assert in Thread::InternalReset called from DoOneFinalization later.
-        _ASSERTE(!pThread->HasCriticalRegion());
-        _ASSERTE(!pThread->HasThreadAffinity());
-    }
-    EX_END_CATCH(SwallowAllExceptions)
+    PREPARE_NONVIRTUAL_CALLSITE_USING_CODE(DELEGATEREF(*pDelegate)->GetMethodPtr());
+
+    DECLARE_ARGHOLDER_ARRAY(args, 3);
+
+    args[ARGNUM_0] = OBJECTREF_TO_ARGHOLDER(DELEGATEREF(*pDelegate)->GetTarget());
+    args[ARGNUM_1] = OBJECTREF_TO_ARGHOLDER(*pDomain);
+    args[ARGNUM_2] = NULL;
+
+    CALL_MANAGED_METHOD_NORET(args);
 
     // This is an early check for condition that we assert in Thread::InternalReset called from DoOneFinalization later.
     _ASSERTE(!pThread->HasCriticalRegion());
@@ -4050,16 +4003,11 @@ static void InvokeNotify(OBJECTREF *pDelegate, OBJECTREF *pDomain)
 }
 
 
-// For critical system events, ensure that each handler gets a notification --
-// even if prior handlers in the chain have thrown an exception.  Also, try
-// to deliver an unhandled exception event if we ever swallow an exception
-// out of a reliable notification.  Note that the add_ event handers are
-// responsible for any reliable preparation of the target, like eager JITting.
-void DistributeEventReliably(OBJECTREF *pDelegate, OBJECTREF *pDomain)
+void DistributeEvent(OBJECTREF *pDelegate, OBJECTREF *pDomain)
 {
     CONTRACTL
     {
-        NOTHROW;  
+        THROWS;
         GC_TRIGGERS;
         MODE_COOPERATIVE;
     }
@@ -4070,51 +4018,42 @@ void DistributeEventReliably(OBJECTREF *pDelegate, OBJECTREF *pDomain)
 
     Thread *pThread = GetThread();
 
-    EX_TRY
+    struct _gc
     {
-        struct _gc
-        {
-            PTRARRAYREF Array;
-            OBJECTREF   InnerDelegate;
-        } gc;
-        ZeroMemory(&gc, sizeof(gc));
+        PTRARRAYREF Array;
+        OBJECTREF   InnerDelegate;
+    } gc;
+    ZeroMemory(&gc, sizeof(gc));
 
-        GCPROTECT_BEGIN(gc);
+    GCPROTECT_BEGIN(gc);
 
-        gc.Array = (PTRARRAYREF) ((DELEGATEREF)(*pDelegate))->GetInvocationList();
-        if (gc.Array == NULL || !gc.Array->GetMethodTable()->IsArray())
-        {
-            InvokeNotify(pDelegate, pDomain);
-        }
-        else
-        {
-            // The _invocationCount could be less than the array size, if we are sharing
-            // immutable arrays cleverly.
-            INT_PTR invocationCount = ((DELEGATEREF)(*pDelegate))->GetInvocationCount();
+    gc.Array = (PTRARRAYREF) ((DELEGATEREF)(*pDelegate))->GetInvocationList();
+    if (gc.Array == NULL || !gc.Array->GetMethodTable()->IsArray())
+    {
+        InvokeNotify(pDelegate, pDomain);
+    }
+    else
+    {
+        // The _invocationCount could be less than the array size, if we are sharing
+        // immutable arrays cleverly.
+        INT_PTR invocationCount = ((DELEGATEREF)(*pDelegate))->GetInvocationCount();
             
-            _ASSERTE(FitsInU4(invocationCount));
-            DWORD cnt = static_cast<DWORD>(invocationCount);
+        _ASSERTE(FitsInU4(invocationCount));
+        DWORD cnt = static_cast<DWORD>(invocationCount);
 
-            _ASSERTE(cnt <= gc.Array->GetNumComponents());
+        _ASSERTE(cnt <= gc.Array->GetNumComponents());
 
-            for (DWORD i=0; i<cnt; i++)
+        for (DWORD i=0; i<cnt; i++)
+        {
+            gc.InnerDelegate = gc.Array->m_Array[i];
+            InvokeNotify(&gc.InnerDelegate, pDomain);
+            if (pThread->IsAbortRequested())
             {
-                gc.InnerDelegate = gc.Array->m_Array[i];
-                InvokeNotify(&gc.InnerDelegate, pDomain);
-                if (pThread->IsAbortRequested())
-                {
-                    pThread->UnmarkThreadForAbort(Thread::TAR_Thread);
-                }
+                pThread->UnmarkThreadForAbort(Thread::TAR_Thread);
             }
         }
-        GCPROTECT_END();
     }
-    EX_CATCH
-    {
-        // It's not even worth asserting, because these aren't our bugs.  At
-        // some point, a MDA may be warranted.
-    }
-    EX_END_CATCH(SwallowAllExceptions)
+    GCPROTECT_END();
 }
 
 // The unhandled exception event is a little easier to distribute, because

diff --git a/src/coreclr/src/vm/comdelegate.h b/src/coreclr/src/vm/comdelegate.h
index c8ebf37..f1bed43 100644 (file)
--- a/src/coreclr/src/vm/comdelegate.h
+++ b/src/coreclr/src/vm/comdelegate.h
@@ -180,8 +180,8 @@ enum DelegateBindingFlags
     DBF_RelaxedSignature    =   0x00000080, // Allow relaxed signature matching (co/contra variance)
 };
 
-void DistributeEventReliably(OBJECTREF *pDelegate,
-                             OBJECTREF *pDomain);
+void DistributeEvent(OBJECTREF *pDelegate,
+                     OBJECTREF *pDomain);
 
 void DistributeUnhandledExceptionReliably(OBJECTREF *pDelegate,
                                           OBJECTREF *pDomain,