From ab69225b3b70b0654dba9bcda67585d6f0df22b6 Mon Sep 17 00:00:00 2001 From: Max Charlamb <44248479+max-charlamb@users.noreply.github.com> Date: Wed, 6 May 2026 19:48:56 -0400 Subject: [PATCH] Reuse a per-thread last-thrown-object handle Allocate a single strong GC handle per Thread at construction for the LastThrownObject slot and reuse it for the thread's lifetime, eliminating per-throw handle create/destroy churn. The handle payload is updated in place via StoreObjectInHandle. - Remove SetLastThrownObjectHandle, SafeSetLastThrownObject, and SafeUpdateLastThrownObject (renamed to UpdateLastThrownObject). - Add SetSOForLastThrownObject to clobber the LTO handle with the global preallocated SO handle for cheap SO detection via pointer compare. - Guard ~Thread to skip DestroyStrongHandle when LTO points at the global SO singleton. - Use ObjectHandleIsNull for MODE_ANY-safe null checks in IsLastThrownObjectNull. - Remove IsThreadExceptionNull from EEDbgInterface; callers use Thread::IsThrowableNull directly. - Simplify handle lifetime in excep.cpp, jitinterface.cpp, prestub.cpp, exceptionhandling.cpp, and DAC request/task paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/daccess/request.cpp | 4 +- src/coreclr/debug/daccess/task.cpp | 4 +- src/coreclr/debug/ee/debugger.cpp | 8 +- src/coreclr/vm/eedbginterface.h | 2 - src/coreclr/vm/eedbginterfaceimpl.cpp | 14 +-- src/coreclr/vm/eedbginterfaceimpl.h | 2 - src/coreclr/vm/excep.cpp | 16 ++-- src/coreclr/vm/exceptionhandling.cpp | 2 +- src/coreclr/vm/jitinterface.cpp | 4 +- src/coreclr/vm/prestub.cpp | 10 +- src/coreclr/vm/threads.cpp | 129 +++++--------------------- src/coreclr/vm/threads.h | 27 ++---- 12 files changed, 57 insertions(+), 165 deletions(-) diff --git a/src/coreclr/debug/daccess/request.cpp b/src/coreclr/debug/daccess/request.cpp index 2a1cb9f719617e..d1bd3bb57d3984 100644 --- a/src/coreclr/debug/daccess/request.cpp +++ b/src/coreclr/debug/daccess/request.cpp @@ -900,9 +900,9 @@ HRESULT ClrDataAccess::GetThreadData(CLRDATA_ADDRESS threadAddr, struct DacpThre // which has the same dereference semantics as a real GC handle. { OBJECTHANDLE ohException = thread->GetThrowableAsPseudoHandle(); - if (ohException == (OBJECTHANDLE)NULL) + if (ohException == (OBJECTHANDLE)NULL && !thread->IsLastThrownObjectNull()) { - ohException = thread->m_LastThrownObjectHandle; + ohException = thread->LastThrownObjectHandle(); } threadData->lastThrownObjectHandle = TO_CDADDR(ohException); } diff --git a/src/coreclr/debug/daccess/task.cpp b/src/coreclr/debug/daccess/task.cpp index 4fc7604440aca0..6239af149404f2 100644 --- a/src/coreclr/debug/daccess/task.cpp +++ b/src/coreclr/debug/daccess/task.cpp @@ -521,7 +521,7 @@ ClrDataTask::GetLastExceptionState( EX_TRY { - if (m_thread->m_LastThrownObjectHandle) + if (!m_thread->IsLastThrownObjectNull()) { *exception = new (nothrow) ClrDataExceptionState(m_dac, @@ -529,7 +529,7 @@ ClrDataTask::GetLastExceptionState( m_thread, CLRDATA_EXCEPTION_PARTIAL, NULL, - m_thread->m_LastThrownObjectHandle, + m_thread->LastThrownObjectHandle(), NULL); status = *exception ? S_OK : E_OUTOFMEMORY; } diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 1e853b149fbc7c..0d496e8e505cfb 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -7449,7 +7449,7 @@ HRESULT Debugger::SendException(Thread *pThread, (fFirstChance && (!pExState->GetFlags()->SentDebugFirstChance() || !pExState->GetFlags()->SentDebugUserFirstChance()))); // There must be a managed exception object to send a managed exception event - if (g_pEEInterface->IsThreadExceptionNull(pThread) && (pThread->LastThrownObjectHandle() == NULL)) + if (pThread->IsThrowableNull() && pThread->IsLastThrownObjectNull()) { managedEventNeeded = FALSE; } @@ -7736,7 +7736,7 @@ LONG Debugger::NotifyOfCHFFilter(EXCEPTION_POINTERS* pExceptionPointers, PVOID p { CONTRACTL { - if ((GetThreadNULLOk() == NULL) || g_pEEInterface->IsThreadExceptionNull(GetThread())) + if ((GetThreadNULLOk() == NULL) || GetThread()->IsThrowableNull()) { NOTHROW; GC_NOTRIGGER; @@ -7766,7 +7766,7 @@ LONG Debugger::NotifyOfCHFFilter(EXCEPTION_POINTERS* pExceptionPointers, PVOID p // useful information for the debugger and, in fact, it may be a completely // internally handled runtime exception, so we should do nothing. // - if ((GetThreadNULLOk() == NULL) || g_pEEInterface->IsThreadExceptionNull(GetThread())) + if ((GetThreadNULLOk() == NULL) || GetThread()->IsThrowableNull()) { return EXCEPTION_CONTINUE_SEARCH; } @@ -12336,7 +12336,7 @@ bool Debugger::IsThreadAtSafePlace(Thread *thread) // NOTE: don't check for thread->IsExceptionInProgress(), SO has special handling // that directly sets the last thrown object without ever creating a tracker. // (Tracker is what thread->IsExceptionInProgress() checks for) - if (g_pEEInterface->GetThreadException(thread) == CLRException::GetPreallocatedStackOverflowExceptionHandle()) + if (thread->IsLastThrownObjectStackOverflowException()) { return false; } diff --git a/src/coreclr/vm/eedbginterface.h b/src/coreclr/vm/eedbginterface.h index a6fd0182fd5a19..65b1402c151f2a 100644 --- a/src/coreclr/vm/eedbginterface.h +++ b/src/coreclr/vm/eedbginterface.h @@ -100,8 +100,6 @@ class EEDebugInterface virtual OBJECTHANDLE GetThreadException(Thread *pThread) = 0; - virtual bool IsThreadExceptionNull(Thread *pThread) = 0; - virtual void ClearThreadException(Thread *pThread) = 0; virtual bool StartSuspendForDebug(AppDomain *pAppDomain, diff --git a/src/coreclr/vm/eedbginterfaceimpl.cpp b/src/coreclr/vm/eedbginterfaceimpl.cpp index 2c33b9c94a7fc7..6c83af721386a2 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.cpp +++ b/src/coreclr/vm/eedbginterfaceimpl.cpp @@ -248,20 +248,12 @@ OBJECTHANDLE EEDbgInterfaceImpl::GetThreadException(Thread *pThread) // Return the last thrown object if there's no current throwable. // This logic is similar to UpdateCurrentThrowable(). - return pThread->m_LastThrownObjectHandle; -} - -bool EEDbgInterfaceImpl::IsThreadExceptionNull(Thread *pThread) -{ - CONTRACTL + if (!pThread->IsLastThrownObjectNull()) { - NOTHROW; - GC_NOTRIGGER; - PRECONDITION(CheckPointer(pThread)); + return pThread->LastThrownObjectHandle(); } - CONTRACTL_END; - return pThread->IsThrowableNull(); + return NULL; } void EEDbgInterfaceImpl::ClearThreadException(Thread *pThread) diff --git a/src/coreclr/vm/eedbginterfaceimpl.h b/src/coreclr/vm/eedbginterfaceimpl.h index 3e59d15da56ab3..d16a85cc747873 100644 --- a/src/coreclr/vm/eedbginterfaceimpl.h +++ b/src/coreclr/vm/eedbginterfaceimpl.h @@ -88,8 +88,6 @@ class EEDbgInterfaceImpl : public EEDebugInterface OBJECTHANDLE GetThreadException(Thread *pThread); - bool IsThreadExceptionNull(Thread *pThread); - void ClearThreadException(Thread *pThread); bool StartSuspendForDebug(AppDomain *pAppDomain, diff --git a/src/coreclr/vm/excep.cpp b/src/coreclr/vm/excep.cpp index 5e87f606da0be9..64b646c76142e6 100644 --- a/src/coreclr/vm/excep.cpp +++ b/src/coreclr/vm/excep.cpp @@ -2020,11 +2020,7 @@ VOID DECLSPEC_NORETURN RaiseTheExceptionInternalOnly(OBJECTREF throwable) // Always save the current object in the handle so on rethrow we can reuse it. This is important as it // contains stack trace info. - // - // Note: we use SafeSetLastThrownObject, which will try to set the throwable and if there are any problems, - // it will set the throwable to something appropriate (like OOM exception) and return the new - // exception. Thus, the user's exception object can be replaced here. - throwable = pThread->SafeSetLastThrownObject(throwable); + pThread->SetLastThrownObject(throwable); ULONG_PTR hr = GetHRFromThrowable(throwable); @@ -4007,7 +4003,7 @@ LONG InternalUnhandledExceptionFilter_Worker( OBJECTREF oThrowable = pParam->pThread->GetThrowable(); if ((oThrowable != NULL) && (pParam->pThread->LastThrownObject() != oThrowable)) { - pParam->pThread->SafeSetLastThrownObject(oThrowable); + pParam->pThread->SetLastThrownObject(oThrowable); LOG((LF_EH, LL_INFO100, "InternalUnhandledExceptionFilter_Worker: Resetting the LastThrownObject as it appears to have changed.\n")); } @@ -8194,10 +8190,10 @@ void SetupInitialThrowBucketDetails(UINT_PTR adjustedIp) if (fAreBucketingDetailsPresent) { #ifdef _DEBUG - // Under OOM scenarios, its possible that when we are raising a threadabort, - // the throwable may get converted to preallocated OOM object when RaiseTheExceptionInternalOnly - // invokes Thread::SafeSetLastThrownObject. We check if this is the current case and use it in - // our validation below. + // It's possible that the throwable is the preallocated OOM object even when the + // bucket tracker captured the buckets for a thread abort (e.g., OOM was thrown + // directly while a thread abort was in flight). Account for that case in our + // validation below. BOOL fIsPreallocatedOOMExceptionForTA = FALSE; if ((!fIsThreadAbortException) && pUEWatsonBucketTracker->CapturedForThreadAbort()) { diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 6473f4d069958c..aaea3688c50eb1 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -3186,7 +3186,7 @@ void CallCatchFunclet(OBJECTREF throwable, BYTE* pHandlerIP, REGDISPLAY* pvRegDi if (!pThread->GetExceptionState()->IsExceptionInProgress()) { - pThread->SafeSetLastThrownObject(NULL); + pThread->SetLastThrownObject(NULL); } // Sync managed exception state, for the managed thread, based upon any active exception tracker diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index cb30fa87c3aaec..be621e3046c2e4 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10543,9 +10543,7 @@ void CEEInfo::HandleException(struct _EXCEPTION_POINTERS *pExceptionPointers) if (_gc.oCurrentThrowable != _gc.oLastThrownObject) { // Update the LTO. - // - // Note: Incase of OOM, this will get set to OOM instance. - pCurThread->SafeSetLastThrownObject(_gc.oCurrentThrowable); + pCurThread->SetLastThrownObject(_gc.oCurrentThrowable); } GCPROTECT_END(); diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index d5e7ee2d2893b9..6b0dcf7371a73c 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -1933,9 +1933,8 @@ extern "C" PCODE STDCALL PreStubWorker(TransitionBlock* pTransitionBlock, Method } EX_CATCH { - OBJECTHANDLE ohThrowable = CURRENT_THREAD->LastThrownObjectHandle(); - _ASSERTE(ohThrowable); - StackTraceInfo::AppendElement(ObjectFromHandle(ohThrowable), 0, (UINT_PTR)pTransitionBlock, pMD, NULL); + _ASSERTE(!CURRENT_THREAD->IsLastThrownObjectNull()); + StackTraceInfo::AppendElement(CURRENT_THREAD->LastThrownObject(), 0, (UINT_PTR)pTransitionBlock, pMD, NULL); EX_RETHROW; } EX_END_CATCH @@ -2136,11 +2135,10 @@ void ExecuteInterpretedMethodWithArgs_PortableEntryPoint_Complex(PCODE portableE } EX_CATCH { - OBJECTHANDLE ohThrowable = CURRENT_THREAD->LastThrownObjectHandle(); - _ASSERTE(ohThrowable); + _ASSERTE(!CURRENT_THREAD->IsLastThrownObjectNull()); if (finishedPrestubPortion) { - StackTraceInfo::AppendElement(ObjectFromHandle(ohThrowable), 0, (UINT_PTR)block, pMethod, NULL); + StackTraceInfo::AppendElement(CURRENT_THREAD->LastThrownObject(), 0, (UINT_PTR)block, pMethod, NULL); } EX_RETHROW; } diff --git a/src/coreclr/vm/threads.cpp b/src/coreclr/vm/threads.cpp index 5e1f114f56d5a4..c73d93b8dbbb82 100644 --- a/src/coreclr/vm/threads.cpp +++ b/src/coreclr/vm/threads.cpp @@ -1242,7 +1242,9 @@ Thread::Thread() m_StrongHndToExposedObject = CreateGlobalStrongHandle(NULL); GlobalStrongHandleHolder strongHndToExposedObjectHolder(m_StrongHndToExposedObject); - m_LastThrownObjectHandle = NULL; + m_LastThrownObjectHandle = CreateGlobalStrongHandle(NULL); + GlobalStrongHandleHolder lastThrownObjectHandleHolder(m_LastThrownObjectHandle); + m_ltoIsUnhandled = FALSE; m_debuggerFilterContext = NULL; @@ -1364,6 +1366,7 @@ Thread::Thread() exposedObjectHolder.SuppressRelease(); strongHndToExposedObjectHolder.SuppressRelease(); contextHolder.SuppressRelease(); + lastThrownObjectHandleHolder.SuppressRelease(); #ifdef FEATURE_COMINTEROP m_uliInitializeSpyCookie.QuadPart = 0ul; @@ -2270,11 +2273,13 @@ Thread::~Thread() if (!IsAtProcessExit()) { - // Destroy any handles that we're using to hold onto exception objects - SafeSetThrowables(NULL); - DestroyShortWeakHandle(m_ExposedObject); DestroyStrongHandle(m_StrongHndToExposedObject); + + if (m_LastThrownObjectHandle != g_pPreallocatedStackOverflowException) + { + DestroyStrongHandle(m_LastThrownObjectHandle); + } } g_pThinLockThreadIdDispenser->DisposeId(GetThreadId()); @@ -3111,7 +3116,7 @@ void Thread::SetLastThrownObject(OBJECTREF throwable, BOOL isUnhandled) { CONTRACTL { - if ((throwable == NULL) || CLRException::IsPreallocatedExceptionObject(throwable)) NOTHROW; else THROWS; // From CreateHandle + NOTHROW; GC_NOTRIGGER; if (throwable == NULL) MODE_ANY; else MODE_COOPERATIVE; } @@ -3125,44 +3130,22 @@ void Thread::SetLastThrownObject(OBJECTREF throwable, BOOL isUnhandled) // you can't have a NULL unhandled exception _ASSERTE(!(throwable == NULL && isUnhandled)); - if (m_LastThrownObjectHandle != NULL) - { - // We'll sometimes use a handle for a preallocated exception object. We should never, ever destroy one of - // these handles... they'll be destroyed when the Runtime shuts down. - if (!CLRException::IsPreallocatedExceptionHandle(m_LastThrownObjectHandle)) - { - DestroyHandle(m_LastThrownObjectHandle); - } - - m_LastThrownObjectHandle = NULL; // Make sure to set this to NULL here just in case we throw trying to make - // a new handle below. - } + // Each Thread owns a per-thread strong handle for the LastThrownObject slot + // (allocated in the Thread ctor, destroyed in ~Thread). Setting LTO is just + // a payload write; no handle allocation or destruction is needed. + _ASSERTE(m_LastThrownObjectHandle != NULL); + _ASSERTE(throwable == NULL || this == GetThread()); + _ASSERTE(throwable == NULL || IsException(throwable->GetMethodTable())); if (throwable != NULL) { - _ASSERTE(this == GetThread()); - - // Non-compliant exceptions are always wrapped. - // The use of the ExceptionNative:: helper here (rather than the global ::IsException helper) - // is hokey, but we need a GC_NOTRIGGER version and it's only for an ASSERT. - _ASSERTE(IsException(throwable->GetMethodTable())); - - // If we're tracking one of the preallocated exception objects, then just use the global handle that - // matches it rather than creating a new one. - if (CLRException::IsPreallocatedExceptionObject(throwable)) - { - m_LastThrownObjectHandle = CLRException::GetPreallocatedHandleForObject(throwable); - } - else - { - m_LastThrownObjectHandle = AppDomain::GetCurrentDomain()->CreateHandle(throwable); - } - - _ASSERTE(m_LastThrownObjectHandle != NULL); + StoreObjectInHandle(m_LastThrownObjectHandle, throwable); m_ltoIsUnhandled = isUnhandled; + } else { + ResetOBJECTHANDLE(m_LastThrownObjectHandle); m_ltoIsUnhandled = FALSE; } } @@ -3185,41 +3168,6 @@ void Thread::SetSOForLastThrownObject() m_LastThrownObjectHandle = CLRException::GetPreallocatedStackOverflowExceptionHandle(); } -// -// This is a nice wrapper for SetLastThrownObject which catches any exceptions caused by not being able to create -// the handle for the throwable, and setting the last thrown object to the preallocated out of memory exception -// instead. -// -OBJECTREF Thread::SafeSetLastThrownObject(OBJECTREF throwable) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - if (throwable == NULL) MODE_ANY; else MODE_COOPERATIVE; - } - CONTRACTL_END; - - // We return the original throwable if nothing goes wrong. - OBJECTREF ret = throwable; - - EX_TRY - { - // Try to set the throwable. - SetLastThrownObject(throwable); - } - EX_CATCH - { - // If it didn't work, then set the last thrown object to the preallocated OOM exception, and return that - // object instead of the original throwable. - ret = CLRException::GetPreallocatedOutOfMemoryException(); - SetLastThrownObject(ret); - } - EX_END_CATCH - - return ret; -} - // // This is a nice wrapper for updating the last thrown object handle, which catches any exceptions caused by not // being able to create the handle for the throwable, and falls back to the preallocated out of memory exception @@ -3283,34 +3231,15 @@ void Thread::SyncManagedExceptionState(bool fIsDebuggerThread) GCX_COOP(); // Syncup the LastThrownObject on the managed thread - SafeUpdateLastThrownObject(); + UpdateLastThrownObject(); } } -void Thread::SetLastThrownObjectHandle(OBJECTHANDLE h) -{ - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } - CONTRACTL_END; - - if (m_LastThrownObjectHandle != NULL && - !CLRException::IsPreallocatedExceptionHandle(m_LastThrownObjectHandle)) - { - DestroyHandle(m_LastThrownObjectHandle); - } - - m_LastThrownObjectHandle = h; -} - // // Create a duplicate handle of the current throwable and set the last thrown object to that. This ensures that the // last thrown object and the current throwable have handles that are in the same app domain. // -void Thread::SafeUpdateLastThrownObject(void) +void Thread::UpdateLastThrownObject(void) { CONTRACTL { @@ -3324,16 +3253,7 @@ void Thread::SafeUpdateLastThrownObject(void) if (throwable != NULL) { - EX_TRY - { - SetLastThrownObject(throwable); - } - EX_CATCH - { - // If we can't create a handle, set the last thrown object to the preallocated OOM exception. - SafeSetThrowables(CLRException::GetPreallocatedOutOfMemoryException()); - } - EX_END_CATCH + SetLastThrownObject(throwable); } } @@ -6690,9 +6610,8 @@ extern "C" InterpThreadContext* STDCALL GetInterpThreadContextWithPossiblyMissin } EX_CATCH { - OBJECTHANDLE ohThrowable = CURRENT_THREAD->LastThrownObjectHandle(); - _ASSERTE(ohThrowable); - StackTraceInfo::AppendElement(ObjectFromHandle(ohThrowable), 0, (UINT_PTR)pTransitionBlock, pByteCodeStart->Method->methodHnd, NULL); + _ASSERTE(!CURRENT_THREAD->IsLastThrownObjectNull()); + StackTraceInfo::AppendElement(CURRENT_THREAD->LastThrownObject(), 0, (UINT_PTR)pTransitionBlock, pByteCodeStart->Method->methodHnd, NULL); EX_RETHROW; } EX_END_CATCH diff --git a/src/coreclr/vm/threads.h b/src/coreclr/vm/threads.h index 195a01434d8ed3..4e705c7da13b1a 100644 --- a/src/coreclr/vm/threads.h +++ b/src/coreclr/vm/threads.h @@ -2619,7 +2619,7 @@ class Thread // Differs from m_pThrowable in that it doesn't stack on nested exceptions. OBJECTHANDLE m_LastThrownObjectHandle; // Unsafe to use directly. Use accessors instead. - // Indicates that the throwable in m_lastThrownObjectHandle should be treated as + // Indicates that the throwable in m_LastThrownObjectHandle should be treated as // unhandled. This occurs during fatal error and a few other early error conditions // before EH is fully set up. BOOL m_ltoIsUnhandled; @@ -2628,22 +2628,18 @@ class Thread public: - BOOL IsLastThrownObjectNull() { WRAPPER_NO_CONTRACT; return (m_LastThrownObjectHandle == (OBJECTHANDLE)0); } + BOOL IsLastThrownObjectNull() + { + WRAPPER_NO_CONTRACT; + _ASSERTE(m_LastThrownObjectHandle != NULL); + return ObjectHandleIsNull(m_LastThrownObjectHandle); + } OBJECTREF LastThrownObject() { WRAPPER_NO_CONTRACT; - - if (m_LastThrownObjectHandle == (OBJECTHANDLE)0) - { - return NULL; - } - else - { - // We only have a handle if we have an object to keep in it. - _ASSERTE(ObjectFromHandle(m_LastThrownObjectHandle) != NULL); - return ObjectFromHandle(m_LastThrownObjectHandle); - } + _ASSERTE(m_LastThrownObjectHandle != NULL); + return ObjectFromHandle(m_LastThrownObjectHandle); } OBJECTHANDLE LastThrownObjectHandle() @@ -2655,7 +2651,6 @@ class Thread void SetLastThrownObject(OBJECTREF throwable, BOOL isUnhandled = FALSE); void SetSOForLastThrownObject(); - OBJECTREF SafeSetLastThrownObject(OBJECTREF throwable); // Inidcates that the last thrown object is now treated as unhandled void MarkLastThrownObjectUnhandled() @@ -2671,7 +2666,7 @@ class Thread return m_ltoIsUnhandled; } - void SafeUpdateLastThrownObject(void); + void UpdateLastThrownObject(void); OBJECTREF SafeSetThrowables(OBJECTREF pThrowable, BOOL isUnhandled = FALSE); @@ -2693,8 +2688,6 @@ class Thread void ClearThreadCurrNotification(); private: - void SetLastThrownObjectHandle(OBJECTHANDLE h); - ThreadExceptionState m_ExceptionState; private: