From: Steve MacLean Date: Tue, 5 Mar 2019 06:54:24 +0000 (-0500) Subject: Remove dead ContainToAppDomain (#23021) X-Git-Tag: accepted/tizen/unified/20190813.215958~61^2~45 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7d54e6130f0db16b2eaa80272076ae3bd725cf69;p=platform%2Fupstream%2Fcoreclr.git Remove dead ContainToAppDomain (#23021) * Remove dead ContainToAppDomain * Respond to feedback --- diff --git a/src/debug/ee/debugger.cpp b/src/debug/ee/debugger.cpp index 46b8b39..aea9020 100644 --- a/src/debug/ee/debugger.cpp +++ b/src/debug/ee/debugger.cpp @@ -13619,7 +13619,6 @@ void Debugger::UnhandledHijackWorker(CONTEXT * pContext, EXCEPTION_RECORD * pRec // processed this unhandled exception. Thus, we should not call into CLR UEF again if it is the case. if (pThread && (pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException) || - pThread->HasThreadStateNC(Thread::TSNC_AppDomainContainUnhandled) || fSOException)) { diff --git a/src/vm/excep.cpp b/src/vm/excep.cpp index 7db5e90..f2a02a0 100644 --- a/src/vm/excep.cpp +++ b/src/vm/excep.cpp @@ -4840,7 +4840,7 @@ LONG InternalUnhandledExceptionFilter_Worker( // simply return back. See comment in threads.h for details for the flag // below. // - if (pThread && (pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException) || pThread->HasThreadStateNC(Thread::TSNC_AppDomainContainUnhandled))) + if (pThread && pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException)) { // This assert shouldnt be hit in CoreCLR since: // @@ -5241,8 +5241,7 @@ LONG __stdcall COMUnhandledExceptionFilter( // EXCEPTION_CONTINUE_SEARCH or // various runtimes again. // // Thus, check if this UEF has already been invoked in context of this thread and runtime and if so, dont invoke it again. - if (GetThread() && (GetThread()->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException) || - GetThread()->HasThreadStateNC(Thread::TSNC_AppDomainContainUnhandled))) + if (GetThread() && (GetThread()->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException))) { LOG((LF_EH, LL_INFO10, "Exiting COMUnhandledExceptionFilter since we have already done UE processing for this thread!\n")); return retVal; @@ -5471,11 +5470,6 @@ DefaultCatchHandler(PEXCEPTION_POINTERS pExceptionPointers, const int buf_size = 128; WCHAR buf[buf_size] = {0}; - // See detailed explanation of this flag in threads.cpp. But the basic idea is that we already - // reported the exception in the AppDomain where it went unhandled, so we don't need to report - // it at the process level. - // Print the unhandled exception message. - if (!pThread->HasThreadStateNC(Thread::TSNC_AppDomainContainUnhandled)) { EX_TRY { @@ -5610,12 +5604,6 @@ BOOL NotifyAppDomainsOfUnhandledException( return FALSE; } - // See detailed explanation of this flag in threads.cpp. But the basic idea is that we already - // reported the exception in the AppDomain where it went unhandled, so we don't need to report - // it at the process level. - if (pThread->HasThreadStateNC(Thread::TSNC_AppDomainContainUnhandled)) - return FALSE; - ThreadPreventAsyncHolder prevAsync; GCX_COOP(); diff --git a/src/vm/threads.cpp b/src/vm/threads.cpp index d2aab7c..53d87a7 100644 --- a/src/vm/threads.cpp +++ b/src/vm/threads.cpp @@ -7626,80 +7626,11 @@ void Thread::DoExtraWorkForFinalizer() SEH handler from DispatchOuter C++ handler from DispatchMiddle - And if there is an AppDomain transition before we call back to user code, we additionally get: - - AppDomain transition -- contains its own handlers to terminate the first pass - and marshal the exception. - - SEH handler from DispatchOuter - C++ handler from DispatchMiddle - - Regardless of whether or not there is an AppDomain transition, we then have: - User code that obviously can throw. - So if we don't have an AD transition, or we take a fault before we successfully transition the - AppDomain, then the base-most DispatchOuter/Middle will deal with the exception. This may - involve swallowing exceptions or it may involve Watson & debugger attach. It will always - involve notifications to any AppDomain.UnhandledException event listeners. - - But if we did transition the AppDomain, then any Watson, debugger attach and UnhandledException - events will occur in that AppDomain in the initial first pass. So we get a good debugging - experience and we get notifications to the host that show which AppDomain is allowing exceptions - to go unhandled (so perhaps it can be unloaded or otherwise dealt with). - - The trick is that if the exception goes unhandled at the process level, we would normally try - to fire AppDomain events and display the faulting exception on the console from two more - places. These are the base-most DispatchOuter/Middle pair and the hook of the OS unhandled - exception handler at the base of the thread. - - This is redundant and messy. (There's no concern with getting a 2nd Watson because we only - do one of these per process anyway). The solution for the base-most DispatchOuter/Middle is - to use the ManagedThreadCallState.flags to control whether the exception has already been - dealt with or not. These flags cause the ThreadBaseRedirectingFilter to either do normal - "base of the thread" exception handling, or to ignore the exception because it has already - been reported in the AppDomain we transitioned to. - - But turning off the reporting in the OS unhandled exception filter is harder. We don't want - to flip a bit on the Thread to disable this, unless we can be sure we are only disabling - something we already reported, and that this thread will never recover from that situation and - start executing code again. Here's the normal nightmare scenario with SEH: - - 1) exception of type A is thrown - 2) All the filters in the 1st pass say they don't want an A - 3) The exception gets all the way out and is considered unhandled. We report this "fact". - 4) Imagine we then set a bit that says this thread shouldn't report unhandled exceptions. - 5) The 2nd pass starts. - 6) Inside a finally, someone throws an exception of type B. - 7) A new 1st pass starts from the point of the throw, with a type B. - 8) Now a filter says "Yes, I will swallow exception B." - 9) We no longer have an unhandled exception, and execution continues merrily. - - This is an unavoidable consequence of the 2-pass model. If you report unhandled exceptions - in the 1st pass (for good debugging), you might find that this was premature and you don't - have an unhandled exception when you get to the 2nd pass. - - But it would not be optimal if in step 4 we set a bit that says we should suppress normal - notifications and reporting on this thread, believing that the process will terminate. - - The solution is to recognize that the base OS unhandled exception filter runs in two modes. - In the first mode, it operates as today and serves as our backstop. In the second mode - it is fully redundant with the handlers pushed after the AppDomain transition, which are - completely containing the exception to the AD that it occurred in (for purposes of reporting). - So we just need a flag on the thread that says whether or not that set of handlers are pushed - and functioning. That flag enables / disables the base exception reporting and is called - TSNC_AppDomainContainUnhandled - */ -enum ManagedThreadCallStateFlags -{ - MTCSF_NormalBase, - MTCSF_ContainToAppDomain, - MTCSF_SuppressDuplicate, -}; - struct ManagedThreadCallState { ADID pAppDomainId; @@ -7709,34 +7640,31 @@ struct ManagedThreadCallState ADCallBackFcnType pTarget; LPVOID args; UnhandledExceptionLocation filterType; - ManagedThreadCallStateFlags flags; BOOL IsAppDomainEqual(AppDomain* pApp) { LIMITED_METHOD_CONTRACT; return bDomainIsAsID?(pApp->GetId()==pAppDomainId):(pUnsafeAppDomain==pApp); } ManagedThreadCallState(ADID AppDomainId,ADCallBackFcnType Target,LPVOID Args, - UnhandledExceptionLocation FilterType, ManagedThreadCallStateFlags Flags): + UnhandledExceptionLocation FilterType): pAppDomainId(AppDomainId), pUnsafeAppDomain(NULL), bDomainIsAsID(TRUE), pTarget(Target), args(Args), - filterType(FilterType), - flags(Flags) + filterType(FilterType) { LIMITED_METHOD_CONTRACT; }; protected: ManagedThreadCallState(AppDomain* AppDomain,ADCallBackFcnType Target,LPVOID Args, - UnhandledExceptionLocation FilterType, ManagedThreadCallStateFlags Flags): + UnhandledExceptionLocation FilterType): pAppDomainId(ADID(0)), pUnsafeAppDomain(AppDomain), bDomainIsAsID(FALSE), pTarget(Target), args(Args), - filterType(FilterType), - flags(Flags) + filterType(FilterType) { LIMITED_METHOD_CONTRACT; }; @@ -7814,7 +7742,6 @@ static void ManagedThreadBase_DispatchMiddle(ManagedThreadCallState *pCallState) { GCX_COOP(); m_pThread->SetFrame(m_pEntryFrame); - m_pThread->ResetThreadStateNC(Thread::TSNC_AppDomainContainUnhandled); } }; @@ -7881,14 +7808,6 @@ static LONG ThreadBaseRedirectingFilter(PEXCEPTION_POINTERS pExceptionInfo, LPVO TryParam * pRealParam = reinterpret_cast(pParam); ManagedThreadCallState * _pCallState = pRealParam->m_pCallState; - ManagedThreadCallStateFlags flags = _pCallState->flags; - - if (flags == MTCSF_SuppressDuplicate) - { - LOG((LF_EH, LL_INFO100, "ThreadBaseRedirectingFilter: setting TSNC_AppDomainContainUnhandled\n")); - GetThread()->SetThreadStateNC(Thread::TSNC_AppDomainContainUnhandled); - return EXCEPTION_CONTINUE_SEARCH; - } LONG ret = -1; @@ -7913,42 +7832,11 @@ static LONG ThreadBaseRedirectingFilter(PEXCEPTION_POINTERS pExceptionInfo, LPVO NotifyOfCHFFilterWrapper(pExceptionInfo, pRealParam); } - // If we are containing unhandled exceptions to the AppDomain we transitioned into, and the - // exception is coming out, then this exception is going unhandled. We have already done - // Watson and managed events, so suppress all filters below us. Otherwise we are swallowing - // it and returning out of the AppDomain. - if (flags == MTCSF_ContainToAppDomain) - { - if(ret == EXCEPTION_CONTINUE_SEARCH) - { - _pCallState->flags = MTCSF_SuppressDuplicate; - } - else if(ret == EXCEPTION_EXECUTE_HANDLER) - { - _pCallState->flags = MTCSF_NormalBase; - } - // else if( EXCEPTION_CONTINUE_EXECUTION ) do nothing - } - // Get the reference to the current thread.. Thread *pCurThread = GetThread(); _ASSERTE(pCurThread); - if (flags == MTCSF_ContainToAppDomain) { - - if (((ManagedThreadCallState *) _pCallState)->flags == MTCSF_SuppressDuplicate) - { - // Set the flag that we have done unhandled exception processing - // for this managed thread that started in a non-default domain - LOG((LF_EH, LL_INFO100, "ThreadBaseRedirectingFilter: setting TSNC_AppDomainContainUnhandled\n")); - pCurThread->SetThreadStateNC(Thread::TSNC_AppDomainContainUnhandled); - } - } - else - { - _ASSERTE(flags == MTCSF_NormalBase); - LOG((LF_EH, LL_INFO100, "ThreadBaseRedirectingFilter: setting TSNC_ProcessedUnhandledException\n")); // @@ -8086,7 +7974,7 @@ static void ManagedThreadBase_FullTransitionWithAD(ADID pAppDomain, } CONTRACTL_END; - ManagedThreadCallState CallState(pAppDomain, pTarget, args, filterType, MTCSF_NormalBase); + ManagedThreadCallState CallState(pAppDomain, pTarget, args, filterType); ManagedThreadBase_DispatchOuter(&CallState); } @@ -8105,7 +7993,7 @@ void ManagedThreadBase_NoADTransition(ADCallBackFcnType pTarget, AppDomain *pAppDomain = GetAppDomain(); - ManagedThreadCallState CallState(pAppDomain, pTarget, NULL, filterType, MTCSF_NormalBase); + ManagedThreadCallState CallState(pAppDomain, pTarget, NULL, filterType); // self-describing, to create a pTurnAround data for eventual delivery to a subsequent AppDomain // transition. @@ -8147,7 +8035,6 @@ void ManagedThreadBase::FinalizerAppDomain(AppDomain *pAppDomain, { WRAPPER_NO_CONTRACT; pTurnAround->InitForFinalizer(pAppDomain,pTarget,args); - _ASSERTE(pTurnAround->flags == MTCSF_NormalBase); ManagedThreadBase_DispatchInner(pTurnAround); } diff --git a/src/vm/threads.h b/src/vm/threads.h index 90a8931..959ae49 100644 --- a/src/vm/threads.h +++ b/src/vm/threads.h @@ -1242,8 +1242,7 @@ public: TSNC_ADUnloadHelper = 0x00002000, // This thread is AD Unload helper. TSNC_CreatingTypeInitException = 0x00004000, // Thread is trying to create a TypeInitException // unused = 0x00008000, - TSNC_AppDomainContainUnhandled = 0x00010000, // Used to control how unhandled exception reporting occurs. - // See detailed explanation for this bit in threads.cpp + // unused = 0x00010000, TSNC_InRestoringSyncBlock = 0x00020000, // The thread is restoring its SyncBlock for Object.Wait. // After the thread is interrupted once, we turn off interruption // at the beginning of wait.