From e5aeb82c0f2b15e039851c3570755f9127c667e8 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Mon, 27 Mar 2017 22:38:53 -0700 Subject: [PATCH] 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 --- src/coreclr/src/vm/appdomain.cpp | 87 ++++++++++------------- src/coreclr/src/vm/comdelegate.cpp | 139 +++++++++++-------------------------- src/coreclr/src/vm/comdelegate.h | 4 +- 3 files changed, 76 insertions(+), 154 deletions(-) diff --git a/src/coreclr/src/vm/appdomain.cpp b/src/coreclr/src/vm/appdomain.cpp index c50d776..8e79018 100644 --- 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 --- 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, ¶m) - { - 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(invocationCount); + _ASSERTE(FitsInU4(invocationCount)); + DWORD cnt = static_cast(invocationCount); - _ASSERTE(cnt <= gc.Array->GetNumComponents()); + _ASSERTE(cnt <= gc.Array->GetNumComponents()); - for (DWORD i=0; im_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 --- 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, -- 2.7.4