diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index ade4037d4a0d46..df084afe163d43 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -864,7 +864,7 @@ UINT_PTR ExceptionTracker::FinishSecondPass( void CleanUpForSecondPass(Thread* pThread, bool fIsSO, LPVOID MemoryStackFpForFrameChain, LPVOID MemoryStackFp); -static void PopExplicitFrames(Thread *pThread, void *targetSp, void *targetCallerSp) +static void PopExplicitFrames(Thread *pThread, void *targetSp, void *targetCallerSp, bool popGCFrames = true) { Frame* pFrame = pThread->GetFrame(); while (pFrame < targetSp) @@ -907,11 +907,14 @@ static void PopExplicitFrames(Thread *pThread, void *targetSp, void *targetCalle } } - GCFrame* pGCFrame = pThread->GetGCFrame(); - while (pGCFrame && pGCFrame < targetSp) + if (popGCFrames) { - pGCFrame->Pop(); - pGCFrame = pThread->GetGCFrame(); + GCFrame* pGCFrame = pThread->GetGCFrame(); + while (pGCFrame && pGCFrame < targetSp) + { + pGCFrame->Pop(); + pGCFrame = pThread->GetGCFrame(); + } } } @@ -934,11 +937,43 @@ ProcessCLRExceptionNew(IN PEXCEPTION_RECORD pExceptionRecord, // Skip native frames of asm helpers that have the ProcessCLRException set as their personality routine. // There is nothing to do for those with the new exception handling. + if (!ExecutionManager::IsManagedCode((PCODE)pDispatcherContext->ControlPc)) + { + return ExceptionContinueSearch; + } + + if (pThread->HasThreadStateNC(Thread::TSNC_UnhandledException2ndPass) && !(pExceptionRecord->ExceptionFlags & EXCEPTION_UNWINDING)) + { + // We are in the 1st pass of exception handling, but the thread mark says that it has already executed 2nd pass + // of unhandled exception handling. That means that some external native code on top of the stack has caught the + // exception that runtime considered to be unhandled, and a new native exception was thrown on the current thread. + // We need to reset the flags below so that we no longer block exception handling for the managed frames. + pThread->ResetThreadStateNC(Thread::TSNC_UnhandledException2ndPass); + pThread->ResetThreadStateNC(Thread::TSNC_ProcessedUnhandledException); + } + // Also skip all frames when processing unhandled exceptions. That allows them to reach the host app // level and let 3rd party the chance to handle them. - if (!ExecutionManager::IsManagedCode((PCODE)pDispatcherContext->ControlPc) || - pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException)) + if (pThread->HasThreadStateNC(Thread::TSNC_ProcessedUnhandledException)) { + if (pExceptionRecord->ExceptionFlags & EXCEPTION_UNWINDING) + { + if (!pThread->HasThreadStateNC(Thread::TSNC_UnhandledException2ndPass)) + { + pThread->SetThreadStateNC(Thread::TSNC_UnhandledException2ndPass); + GCX_COOP(); + // The 3rd argument passes to PopExplicitFrame is normally the parent SP to correctly handle InlinedCallFrame embbeded + // in parent managed frame. But at this point there are no further managed frames are on the stack, so we can pass NULL. + // Also don't pop the GC frames, their destructor will pop them as the exception propagates. + // NOTE: this needs to be popped in the 2nd pass to ensure that crash dumps and Watson get the dump with these still + // present. + ExInfo *pExInfo = (ExInfo*)pThread->GetExceptionState()->GetCurrentExceptionTracker(); + void *sp = (void*)GetRegdisplaySP(pExInfo->m_frameIter.m_crawl.GetRegisterSet()); + PopExplicitFrames(pThread, sp, NULL /* targetCallerSp */, false /* popGCFrames */); + ExInfo::PopExInfos(pThread, sp); + } + } + return ExceptionContinueSearch; } @@ -8438,6 +8473,8 @@ extern "C" CLR_BOOL QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStack Thread* pThread = GET_THREAD(); ExInfo* pExInfo = (ExInfo*)pThread->GetExceptionState()->GetCurrentExceptionTracker(); + pThread->ResetThreadStateNC(Thread::TSNC_ProcessedUnhandledException); + BEGIN_QCALL; Frame* pFrame = pThread->GetFrame(); @@ -8554,7 +8591,7 @@ extern "C" CLR_BOOL QCALLTYPE SfiInit(StackFrameIterator* pThis, CONTEXT* pStack #ifdef HOST_WINDOWS CreateCrashDumpIfEnabled(/* fSOException */ FALSE); GetThread()->SetThreadStateNC(Thread::TSNC_ProcessedUnhandledException); - RaiseException(pExInfo->m_ExceptionCode, EXCEPTION_NONCONTINUABLE_EXCEPTION, pExInfo->m_ptrs.ExceptionRecord->NumberParameters, pExInfo->m_ptrs.ExceptionRecord->ExceptionInformation); + RaiseException(pExInfo->m_ExceptionCode, EXCEPTION_NONCONTINUABLE, pExInfo->m_ptrs.ExceptionRecord->NumberParameters, pExInfo->m_ptrs.ExceptionRecord->ExceptionInformation); #else CrashDumpAndTerminateProcess(pExInfo->m_ExceptionCode); #endif @@ -8691,7 +8728,7 @@ extern "C" CLR_BOOL QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollid { #ifdef HOST_WINDOWS GetThread()->SetThreadStateNC(Thread::TSNC_ProcessedUnhandledException); - RaiseException(pTopExInfo->m_ExceptionCode, EXCEPTION_NONCONTINUABLE_EXCEPTION, pTopExInfo->m_ptrs.ExceptionRecord->NumberParameters, pTopExInfo->m_ptrs.ExceptionRecord->ExceptionInformation); + RaiseException(pTopExInfo->m_ExceptionCode, EXCEPTION_NONCONTINUABLE, pTopExInfo->m_ptrs.ExceptionRecord->NumberParameters, pTopExInfo->m_ptrs.ExceptionRecord->ExceptionInformation); #else CrashDumpAndTerminateProcess(pTopExInfo->m_ExceptionCode); #endif diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 423b26b691a257..42d7f7ec29954e 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -655,7 +655,7 @@ class Thread // effort. // // Once we are completely independent of the OS UEF, we could remove this. - // unused = 0x02000000, + TSNC_UnhandledException2ndPass = 0x02000000, // The unhandled exception propagation is in the 2nd pass TSNC_DebuggerSleepWaitJoin = 0x04000000, // Indicates to the debugger that this thread is in a sleep wait or join state // This almost mirrors the TS_Interruptible state however that flag can change // during GC-preemptive mode whereas this one cannot. diff --git a/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.cs b/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.cs index d01d867efcb972..9327c775f1a15d 100644 --- a/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.cs +++ b/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.cs @@ -15,6 +15,12 @@ public class ForeignThreadExceptionsTest [DllImport("ForeignThreadExceptionsNative")] public static extern void InvokeCallbackOnNewThread(MyCallback callback); + [DllImport("ForeignThreadExceptionsNative")] + public static extern void InvokeCallbackAndCatchTwiceOnNewThread(MyCallback callback); + + [DllImport("ForeignThreadExceptionsNative")] + public static extern void ThrowException(); + public static void MethodThatThrows() { throw new Exception("This is MethodThatThrows."); @@ -56,6 +62,28 @@ public static void RunTest() Console.WriteLine("Caught hardware exception in a delegate called through Reverse PInvoke on a foreign thread."); } }); + + if (OperatingSystem.IsWindows() && !TestLibrary.Utilities.IsNativeAot && !TestLibrary.Utilities.IsMonoRuntime) + { + InvokeCallbackAndCatchTwiceOnNewThread(() => { + throw new Exception("Exception unhandled in any managed code"); + }); + + int finallyCallsCount = 0; + InvokeCallbackAndCatchTwiceOnNewThread(() => { + try + { + // Throw native exception that is not handled in any managed code + ThrowException(); + } + finally + { + finallyCallsCount++; + } + }); + + Assert.Equal(2, finallyCallsCount); + } } [Fact] diff --git a/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.csproj b/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.csproj index 73b58709c91401..9606ae19e6f9a2 100644 --- a/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.csproj +++ b/src/tests/Exceptions/ForeignThread/ForeignThreadExceptions.csproj @@ -6,6 +6,7 @@ + diff --git a/src/tests/Exceptions/ForeignThread/ForeignThreadExceptionsNative.cpp b/src/tests/Exceptions/ForeignThread/ForeignThreadExceptionsNative.cpp index 09fb4cf97c556b..b1c1598090b548 100644 --- a/src/tests/Exceptions/ForeignThread/ForeignThreadExceptionsNative.cpp +++ b/src/tests/Exceptions/ForeignThread/ForeignThreadExceptionsNative.cpp @@ -16,6 +16,44 @@ #include typedef void (*PFNACTION1)(); + +#ifdef _WIN32 +extern "C" DLL_EXPORT void STDMETHODCALLTYPE ThrowException() +{ + throw std::exception(); +} + +extern "C" DLL_EXPORT void InvokeCallbackAndCatchTwice(PFNACTION1 callback) +{ + try + { + callback(); + } + catch (...) + { + printf("Caught exception once\n"); + } + + try + { + // Put garbage on the stack that was possibly used by the previous callback to catch + // cases when the explicit frames or ExInfos were not cleaned up properly when the + // exception is not handled in the managed code and reaches this native caller. + unsigned int *p = (unsigned int *)alloca(16384); + for (int i = 0; i < 16384 / sizeof(unsigned int); i++) + { + *p++ = 0xbaadf00d; + } + callback(); + } + catch (...) + { + printf("Caught exception again\n"); + } +} + +#endif // _WIN32 + extern "C" DLL_EXPORT void InvokeCallback(PFNACTION1 callback) { callback(); @@ -32,12 +70,21 @@ void* InvokeCallbackUnix(void* callback) #endif // !_WIN32 -extern "C" DLL_EXPORT void InvokeCallbackOnNewThread(PFNACTION1 callback) -{ #ifdef _WIN32 - std::thread t1(InvokeCallback, callback); +void InvokeCallbackOnNewThreadCommon(PFNACTION1 callback, void (*startRoutine)(PFNACTION1)) +{ + std::thread t1(startRoutine, callback); t1.join(); +} + +extern "C" DLL_EXPORT void InvokeCallbackAndCatchTwiceOnNewThread(PFNACTION1 callback) +{ + InvokeCallbackOnNewThreadCommon(callback, InvokeCallbackAndCatchTwice); +} + #else // _WIN32 +void InvokeCallbackOnNewThreadCommon(PFNACTION1 callback, void *(*startRoutine)(void*)) +{ // For Unix, we need to use pthreads to create the thread so that we can set its stack size. // We need to set the stack size due to the very small (80kB) default stack size on MUSL // based Linux distros. @@ -50,10 +97,19 @@ extern "C" DLL_EXPORT void InvokeCallbackOnNewThread(PFNACTION1 callback) AbortIfFail(st); pthread_t t; - st = pthread_create(&t, &attr, InvokeCallbackUnix, (void*)callback); + st = pthread_create(&t, &attr, startRoutine, (void*)callback); AbortIfFail(st); st = pthread_join(t, NULL); AbortIfFail(st); +} #endif // _WIN32 + +extern "C" DLL_EXPORT void InvokeCallbackOnNewThread(PFNACTION1 callback) +{ +#ifdef _WIN32 + InvokeCallbackOnNewThreadCommon(callback, InvokeCallback); +#else // _WIN32 + InvokeCallbackOnNewThreadCommon(callback, InvokeCallbackUnix); +#endif }