From 5bd1ff9cb5ba7207d8aba35e29f9f07a039e00e8 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 1 Apr 2026 13:18:17 -0700 Subject: [PATCH 1/8] Use LifetimeHolder for CLRConfig strings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/debug/ee/debugger.cpp | 2 +- src/coreclr/inc/clrconfig.h | 9 ++++++++- src/coreclr/vm/ceemain.cpp | 2 +- src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h | 2 +- src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h | 4 ++-- src/coreclr/vm/profilinghelper.cpp | 2 +- 6 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index b4912f6dce92de..ffd41afb34570a 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/src/coreclr/debug/ee/debugger.cpp @@ -1050,7 +1050,7 @@ void Debugger::InitDebugEventCounting() memset(&g_iDbgDebuggerCounter, 0, DBG_DEBUGGER_MAX*sizeof(int)); // retrieve the possible counter for break point - CLRConfigStringHolder wstrValue = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_DebuggerBreakPoint); + CLRConfigStringHolder wstrValue(CLRConfig::GetConfigValue(CLRConfig::INTERNAL_DebuggerBreakPoint)); // The string value is of the following format // =Count;=Count;....; // The string must end with ; diff --git a/src/coreclr/inc/clrconfig.h b/src/coreclr/inc/clrconfig.h index 7ca8e654a54bb6..213f231cffda3e 100644 --- a/src/coreclr/inc/clrconfig.h +++ b/src/coreclr/inc/clrconfig.h @@ -150,6 +150,13 @@ inline CLRConfig::LookupOptions operator&(CLRConfig::LookupOptions lhs, CLRConfi return static_cast(static_cast(lhs) & static_cast(rhs)); } -typedef Wrapper CLRConfigStringHolder; +struct CLRConfigStringTraits final +{ + using Type = LPWSTR; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { CLRConfig::FreeConfigString(value); } +}; + +using CLRConfigStringHolder = LifetimeHolder; #endif //__CLRConfig_h__ diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index 289f91e1828504..3180399f9ce2cd 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -709,7 +709,7 @@ void EEStartupHelper() unsigned level = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_LogLevel, LL_INFO1000); unsigned bytesPerThread = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_StressLogSize, STRESSLOG_CHUNK_SIZE * 4); unsigned totalBytes = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_TotalStressLogSize, STRESSLOG_CHUNK_SIZE * 1024); - CLRConfigStringHolder logFilename = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_StressLogFilename); + CLRConfigStringHolder logFilename(CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_StressLogFilename)); StressLog::Initialize(facilities, level, bytesPerThread, totalBytes, GetClrModuleBase(), logFilename); g_pStressLog = &StressLog::theLog; } diff --git a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h index 3f412558055446..2d49b8f78e35ff 100644 --- a/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h +++ b/src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h @@ -167,7 +167,7 @@ ds_rt_config_value_get_ports (void) STATIC_CONTRACT_NOTHROW; CLRConfigStringHolder value(CLRConfig::GetConfigValue (CLRConfig::EXTERNAL_DOTNET_DiagnosticPorts)); - return ep_rt_utf16_to_utf8_string (reinterpret_cast(value.GetValue ())); + return ep_rt_utf16_to_utf8_string (reinterpret_cast(static_cast(value))); } static diff --git a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h index 3d3b13d91183f7..97991823eaff97 100644 --- a/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h +++ b/src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h @@ -532,7 +532,7 @@ ep_rt_config_value_get_config (void) { STATIC_CONTRACT_NOTHROW; CLRConfigStringHolder value(CLRConfig::GetConfigValue (CLRConfig::INTERNAL_EventPipeConfig)); - return ep_rt_utf16_to_utf8_string (reinterpret_cast(value.GetValue ())); + return ep_rt_utf16_to_utf8_string (reinterpret_cast(static_cast(value))); } static @@ -542,7 +542,7 @@ ep_rt_config_value_get_output_path (void) { STATIC_CONTRACT_NOTHROW; CLRConfigStringHolder value(CLRConfig::GetConfigValue (CLRConfig::INTERNAL_EventPipeOutputPath)); - return ep_rt_utf16_to_utf8_string (reinterpret_cast(value.GetValue ())); + return ep_rt_utf16_to_utf8_string (reinterpret_cast(static_cast(value))); } static diff --git a/src/coreclr/vm/profilinghelper.cpp b/src/coreclr/vm/profilinghelper.cpp index c7f7d2bc457925..1e51c091ff1d75 100644 --- a/src/coreclr/vm/profilinghelper.cpp +++ b/src/coreclr/vm/profilinghelper.cpp @@ -756,7 +756,7 @@ HRESULT ProfilingAPIUtility::AttemptLoadDelayedStartupProfilers() HRESULT ProfilingAPIUtility::AttemptLoadProfilerList() { HRESULT hr = S_OK; - CLRConfigStringHolder wszProfilerList(NULL); + CLRConfigStringHolder wszProfilerList; #if defined(TARGET_ARM64) CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_CORECLR_NOTIFICATION_PROFILERS_ARM64, &wszProfilerList); From 18829196d86f2341bcbe4d17df4eb58e06834c61 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Wed, 1 Apr 2026 14:32:52 -0700 Subject: [PATCH 2/8] Use LifetimeHolder for CRITSEC allocation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/inc/clrhost.h | 15 +++++++++++++-- src/coreclr/vm/eetoprofinterfaceimpl.cpp | 3 +-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/coreclr/inc/clrhost.h b/src/coreclr/inc/clrhost.h index 5971ca2b4356e3..a74075f8e152a8 100644 --- a/src/coreclr/inc/clrhost.h +++ b/src/coreclr/inc/clrhost.h @@ -89,8 +89,19 @@ DWORD ClrSleepEx(DWORD dwMilliseconds, BOOL bAlertable); typedef Holder CRITSEC_Holder; // Use this holder to manage CRITSEC_COOKIE allocation to ensure it will be released if anything goes wrong -FORCEINLINE void VoidClrDeleteCriticalSection(CRITSEC_COOKIE cs) { if (cs != NULL) ClrDeleteCriticalSection(cs); } -typedef Wrapper, VoidClrDeleteCriticalSection, 0> CRITSEC_AllocationHolder; +struct CRITSECCookieAllocationTraits final +{ + using Type = CRITSEC_COOKIE; + static constexpr Type Default() { return NULL; } + static void Free(Type cs) + { + STATIC_CONTRACT_WRAPPER; + if (cs != NULL) + ClrDeleteCriticalSection(cs); + } +}; + +using CRITSEC_AllocationHolder = LifetimeHolder; #ifndef DACCESS_COMPILE // Suspend/resume APIs that fail-fast on errors diff --git a/src/coreclr/vm/eetoprofinterfaceimpl.cpp b/src/coreclr/vm/eetoprofinterfaceimpl.cpp index 8b819864b7aef7..c91dfbadf13fc8 100644 --- a/src/coreclr/vm/eetoprofinterfaceimpl.cpp +++ b/src/coreclr/vm/eetoprofinterfaceimpl.cpp @@ -599,8 +599,7 @@ HRESULT EEToProfInterfaceImpl::Init( m_pProfToEE = pProfToEE; - m_csGCRefDataFreeList = csGCRefDataFreeList.Extract(); - csGCRefDataFreeList = NULL; + m_csGCRefDataFreeList = csGCRefDataFreeList.Detach(); m_pFunctionIDHashTable = pFunctionIDHashTable.Extract(); pFunctionIDHashTable = NULL; From f3d0d252a4bb8c2e18f03b77f9e75990cec5dfe8 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 3 Apr 2026 10:36:02 -0700 Subject: [PATCH 3/8] Use LifetimeHolder for loader-related holders Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/inc/holder.h | 8 +++++++ src/coreclr/vm/appdomain.cpp | 10 ++------- src/coreclr/vm/appdomain.hpp | 16 +++++++++++--- src/coreclr/vm/assemblynative.cpp | 4 ++-- src/coreclr/vm/assemblyspec.cpp | 2 +- src/coreclr/vm/ceeload.cpp | 4 ++-- src/coreclr/vm/ceeload.h | 34 +++++++++++++++++++---------- src/coreclr/vm/coreassemblyspec.cpp | 4 ++-- src/coreclr/vm/nativeimage.cpp | 2 +- src/coreclr/vm/peimage.cpp | 4 ++-- src/coreclr/vm/peimage.h | 16 +++++++++----- src/coreclr/vm/peimage.inl | 4 ++-- 12 files changed, 68 insertions(+), 40 deletions(-) diff --git a/src/coreclr/inc/holder.h b/src/coreclr/inc/holder.h index bd6b3de64fdfce..ca6d404932b90e 100644 --- a/src/coreclr/inc/holder.h +++ b/src/coreclr/inc/holder.h @@ -1147,6 +1147,14 @@ class LifetimeHolder final operator Type() const { STATIC_CONTRACT_LEAF; return m_value; } Type* operator&() { STATIC_CONTRACT_LEAF; _ASSERTE(m_value == T::Default()); return &m_value; } + template ::value>> + U operator->() const + { + STATIC_CONTRACT_LEAF; + _ASSERTE(m_value != NULL); + return m_value; + } + void Free() { STATIC_CONTRACT_WRAPPER; diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 63f78b3f757e86..5c6c47ac11611f 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -2137,12 +2137,6 @@ FileLoadLock::FileLoadLock(PEFileListLock* pLock, PEAssembly* pPEAssembly) pPEAssembly->AddRef(); } -void FileLoadLock::HolderLeave(FileLoadLock *pThis) -{ - LIMITED_METHOD_CONTRACT; - pThis->Leave(); -} - // // Assembly loading: @@ -2649,7 +2643,7 @@ void AppDomain::TryIncrementalLoad(FileLoadLevel workLevel, FileLoadLockHolder& // This is factored out so we don't call EX_TRY in a loop (EX_TRY can _alloca) BOOL released = FALSE; - FileLoadLock* pLoadLock = lockHolder.GetValue(); + FileLoadLock* pLoadLock = lockHolder; Assembly* pAssembly = pLoadLock->GetAssembly(); EX_TRY @@ -2690,7 +2684,7 @@ void AppDomain::TryIncrementalLoad(FileLoadLevel workLevel, FileLoadLockHolder& if (pLoadLock->CompleteLoadLevel(workLevel, success) && pLoadLock->GetLoadLevel()==FILE_LOAD_DELIVER_EVENTS) { - lockHolder.Release(); + lockHolder.Free(); released = TRUE; pAssembly->DeliverAsyncEvents(); }; diff --git a/src/coreclr/vm/appdomain.hpp b/src/coreclr/vm/appdomain.hpp index 23f0e689da57cb..a64118a9fdaee4 100644 --- a/src/coreclr/vm/appdomain.hpp +++ b/src/coreclr/vm/appdomain.hpp @@ -333,10 +333,20 @@ class FileLoadLock : public ListLockEntry FileLoadLock(PEFileListLock* pLock, PEAssembly* pPEAssembly); - static void HolderLeave(FileLoadLock *pThis); - public: - typedef Wrapper Holder; + struct HolderTraits final + { + using Type = FileLoadLock*; + static constexpr Type Default() { return NULL; } + static void Free(Type pThis) + { + LIMITED_METHOD_CONTRACT; + if (pThis != NULL) + pThis->Leave(); + } + }; + + using Holder = LifetimeHolder; }; diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp index a4d71202470ed7..498a15f65c03c3 100644 --- a/src/coreclr/vm/assemblynative.cpp +++ b/src/coreclr/vm/assemblynative.cpp @@ -196,13 +196,13 @@ extern "C" void QCALLTYPE AssemblyNative_LoadFromPath(INT_PTR ptrNativeAssemblyB // Need to verify that this is a valid CLR assembly. if (!pILImage->CheckILFormat()) - THROW_BAD_FORMAT(BFA_BAD_IL, pILImage.GetValue()); + THROW_BAD_FORMAT(BFA_BAD_IL, (PEImage*)pILImage); LoaderAllocator* pLoaderAllocator = pBinder->GetLoaderAllocator(); if (pLoaderAllocator && pLoaderAllocator->IsCollectible() && !pILImage->IsILOnly()) { // Loading IJW assemblies into a collectible AssemblyLoadContext is not allowed - THROW_BAD_FORMAT(BFA_IJW_IN_COLLECTIBLE_ALC, pILImage.GetValue()); + THROW_BAD_FORMAT(BFA_IJW_IN_COLLECTIBLE_ALC, (PEImage*)pILImage); } } diff --git a/src/coreclr/vm/assemblyspec.cpp b/src/coreclr/vm/assemblyspec.cpp index eec10de149a933..75bc09d1902ed2 100644 --- a/src/coreclr/vm/assemblyspec.cpp +++ b/src/coreclr/vm/assemblyspec.cpp @@ -431,7 +431,7 @@ Assembly *AssemblySpec::LoadAssembly(LPCWSTR pFilePath) // Need to verify that this is a valid CLR assembly. if (!pILImage->CheckILFormat()) - THROW_BAD_FORMAT(BFA_BAD_IL, pILImage.GetValue()); + THROW_BAD_FORMAT(BFA_BAD_IL, (PEImage*)pILImage); RETURN AssemblyNative::LoadFromPEImage(AppDomain::GetCurrentDomain()->GetDefaultBinder(), pILImage, true /* excludeAppPaths */); } diff --git a/src/coreclr/vm/ceeload.cpp b/src/coreclr/vm/ceeload.cpp index 8eb81e38a329af..6efce51612f19b 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -620,7 +620,7 @@ Module *Module::Create(Assembly *pAssembly, PEAssembly *pPEAssembly, AllocMemTra ModuleHolder pModuleSafe(pModule); pModuleSafe->DoInit(pamTracker, NULL); - RETURN pModuleSafe.Extract(); + RETURN pModuleSafe.Detach(); } void Module::ApplyMetaData() @@ -3821,7 +3821,7 @@ ReflectionModule *ReflectionModule::Create(Assembly *pAssembly, PEAssembly *pPEA pModule->DoInit(pamTracker, szName); pModule->SetIsRuntimeWrapExceptionsCached_ForReflectionEmitModules(); - RETURN pModule.Extract(); + RETURN pModule.Detach(); } diff --git a/src/coreclr/vm/ceeload.h b/src/coreclr/vm/ceeload.h index 1bcc3b2bda95e1..9d39750334544e 100644 --- a/src/coreclr/vm/ceeload.h +++ b/src/coreclr/vm/ceeload.h @@ -1790,27 +1790,37 @@ class ReflectionModule : public Module void CaptureModuleMetaDataToMemory(); }; -// Module holders -FORCEINLINE void VoidModuleDestruct(Module *pModule) +struct ModuleHolderTraits final { + using Type = Module*; + static constexpr Type Default() { return NULL; } + static void Free(Type pModule) + { + STATIC_CONTRACT_WRAPPER; #ifndef DACCESS_COMPILE - if (g_fEEStarted) - pModule->Destruct(); + if (g_fEEStarted && pModule != NULL) + pModule->Destruct(); #endif -} - -typedef Wrapper ModuleHolder; - + } +}; +using ModuleHolder = LifetimeHolder; -FORCEINLINE void VoidReflectionModuleDestruct(ReflectionModule *pModule) +struct ReflectionModuleHolderTraits final { + using Type = ReflectionModule*; + static constexpr Type Default() { return NULL; } + static void Free(Type pModule) + { + STATIC_CONTRACT_WRAPPER; #ifndef DACCESS_COMPILE - pModule->Destruct(); + if (pModule != NULL) + pModule->Destruct(); #endif -} + } +}; -typedef Wrapper ReflectionModuleHolder; +using ReflectionModuleHolder = LifetimeHolder; diff --git a/src/coreclr/vm/coreassemblyspec.cpp b/src/coreclr/vm/coreassemblyspec.cpp index 8e98f6729088e3..65f386a1e400d1 100644 --- a/src/coreclr/vm/coreassemblyspec.cpp +++ b/src/coreclr/vm/coreassemblyspec.cpp @@ -86,7 +86,7 @@ STDAPI BinderAcquirePEImage(LPCWSTR wszAssemblyPath, EX_TRY { - PEImageHolder pImage = PEImage::OpenImage(wszAssemblyPath, MDInternalImport_Default, probeExtensionResult); + PEImageHolder pImage(PEImage::OpenImage(wszAssemblyPath, MDInternalImport_Default, probeExtensionResult)); // Make sure that the IL image can be opened. if (pImage->IsFile()) @@ -99,7 +99,7 @@ STDAPI BinderAcquirePEImage(LPCWSTR wszAssemblyPath, } if (pImage) - *ppPEImage = pImage.Extract(); + *ppPEImage = pImage.Detach(); } EX_CATCH_HRESULT(hr); diff --git a/src/coreclr/vm/nativeimage.cpp b/src/coreclr/vm/nativeimage.cpp index c831257e23278c..5f44ce17e0be38 100644 --- a/src/coreclr/vm/nativeimage.cpp +++ b/src/coreclr/vm/nativeimage.cpp @@ -138,7 +138,7 @@ namespace // No need to use cache for this PE image. // Composite r2r PE image is not a part of anyone's identity. // We only need it to obtain the native image, which will be cached at AppDomain level. - PEImageHolder pImage = PEImage::OpenImage(fullPath, MDInternalImport_NoCache, probeExtensionResult); + PEImageHolder pImage(PEImage::OpenImage(fullPath, MDInternalImport_NoCache, probeExtensionResult)); PEImageLayout* loaded = pImage->GetOrCreateLayout(PEImageLayout::LAYOUT_LOADED); // We will let pImage instance be freed after exiting this scope, but we will keep the layout, // thus the layout needs an AddRef, or it will be gone together with pImage. diff --git a/src/coreclr/vm/peimage.cpp b/src/coreclr/vm/peimage.cpp index aee07068633a65..21240cefcd33a2 100644 --- a/src/coreclr/vm/peimage.cpp +++ b/src/coreclr/vm/peimage.cpp @@ -723,7 +723,7 @@ PTR_PEImage PEImage::CreateFromByteArray(const BYTE* array, COUNT_T size) SimpleWriteLockHolder lock(pImage->m_pLayoutLock); pImage->SetLayout(IMAGE_FLAT,pLayout); - RETURN dac_cast(pImage.Extract()); + RETURN dac_cast(pImage.Detach()); } #ifndef TARGET_UNIX @@ -756,7 +756,7 @@ PTR_PEImage PEImage::CreateFromHMODULE(HMODULE hMod) } _ASSERTE(pImage->m_pLayouts[IMAGE_FLAT] != NULL); - RETURN dac_cast(pImage.Extract()); + RETURN dac_cast(pImage.Detach()); } #endif // !TARGET_UNIX diff --git a/src/coreclr/vm/peimage.h b/src/coreclr/vm/peimage.h index c43a7881e03d33..ea56c0d8d594c7 100644 --- a/src/coreclr/vm/peimage.h +++ b/src/coreclr/vm/peimage.h @@ -331,13 +331,19 @@ struct cdac_data static constexpr size_t ProbeExtensionResult = offsetof(PEImage, m_probeExtensionResult); }; -FORCEINLINE void PEImageRelease(PEImage *i) +struct PEImageHolderTraits final { - WRAPPER_NO_CONTRACT; - i->Release(); -} + using Type = PEImage*; + static constexpr Type Default() { return NULL; } + static void Free(Type i) + { + WRAPPER_NO_CONTRACT; + if (i != NULL) + i->Release(); + } +}; -typedef Wrapper PEImageHolder; +using PEImageHolder = LifetimeHolder; // ================================================================================ // Inline definitions diff --git a/src/coreclr/vm/peimage.inl b/src/coreclr/vm/peimage.inl index 1ca6d5ab3ba054..f735b72c49f494 100644 --- a/src/coreclr/vm/peimage.inl +++ b/src/coreclr/vm/peimage.inl @@ -356,7 +356,7 @@ inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags { PEImageHolder pImage(new PEImage{pPath}); pImage->Init(probeExtensionResult); - return dac_cast(pImage.Extract()); + return dac_cast(pImage.Detach()); } CrstHolder holder(&s_hashLock); @@ -374,7 +374,7 @@ inline PTR_PEImage PEImage::OpenImage(LPCWSTR pPath, MDInternalImportFlags flags pImage->Init(probeExtensionResult); pImage->AddToHashMap(); - return dac_cast(pImage.Extract()); + return dac_cast(pImage.Detach()); } found->AddRef(); From bc0a9948739b00ad1be7c9a89fb55a9815741e76 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 3 Apr 2026 13:06:46 -0700 Subject: [PATCH 4/8] Use LifetimeHolder for coreclr object handles Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../nativeaot/Runtime/forward_declarations.h | 1 - src/coreclr/vm/comconnectionpoints.cpp | 11 ++-- src/coreclr/vm/comconnectionpoints.h | 21 ++++---- src/coreclr/vm/debugdebugger.cpp | 17 +++++-- src/coreclr/vm/gchandleutilities.h | 51 ++++++++----------- src/coreclr/vm/jitinterface.cpp | 5 +- src/coreclr/vm/olevariant.cpp | 4 +- src/coreclr/vm/syncblk.cpp | 6 +-- 8 files changed, 56 insertions(+), 60 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/forward_declarations.h b/src/coreclr/nativeaot/Runtime/forward_declarations.h index f821945f886d7b..40acdc77b9831e 100644 --- a/src/coreclr/nativeaot/Runtime/forward_declarations.h +++ b/src/coreclr/nativeaot/Runtime/forward_declarations.h @@ -21,7 +21,6 @@ FWD_DECL(EEThreadId) FWD_DECL(MethodInfo) FWD_DECL(Module) FWD_DECL(Object) -FWD_DECL(OBJECTHANDLEHolder) FWD_DECL(PageEntry) FWD_DECL(PAL_EnterHolder) FWD_DECL(PAL_LeaveHolder) diff --git a/src/coreclr/vm/comconnectionpoints.cpp b/src/coreclr/vm/comconnectionpoints.cpp index cfc089c2651afd..1f28d1ff6e8ee3 100644 --- a/src/coreclr/vm/comconnectionpoints.cpp +++ b/src/coreclr/vm/comconnectionpoints.cpp @@ -344,11 +344,8 @@ void ConnectionPoint::AdviseWorker(IUnknown *pUnk, DWORD *pdwCookie) } // Allocate the object handle and the connection cookie. - OBJECTHANDLEHolder phndEventItfObj = GetAppDomain()->CreateHandle((OBJECTREF)pEventItfObj); - ConnectionCookieHolder pConCookie = ConnectionCookie::CreateConnectionCookie(phndEventItfObj); - - // pConCookie owns the handle now and will destroy it on exception - phndEventItfObj.SuppressRelease(); + OBJECTHANDLEHolder phndEventItfObj(GetAppDomain()->CreateHandle((OBJECTREF)pEventItfObj)); + ConnectionCookieHolder pConCookie = ConnectionCookie::CreateConnectionCookie(std::move(phndEventItfObj)); // Add the connection cookie to the list. InsertWithLock(pConCookie); @@ -1115,14 +1112,14 @@ HRESULT __stdcall ConnectionEnum::Next(ULONG cConnections, CONNECTDATA* rgcd, UL ConnectionPoint::LockHolder lh(m_pConnectionPoint); { - // Switch to cooperative GC mode before we manipulate OBJCETREF's. + // Switch to cooperative GC mode before we manipulate OBJECTREF's. GCX_COOP(); for (cFetched = 0; cFetched < cConnections && m_CurrCookie; cFetched++) { { CONTRACT_VIOLATION(ThrowsViolation); - rgcd[cFetched].pUnk = GetComIPFromObjectRef((OBJECTREF*)m_CurrCookie->m_hndEventProvObj, ComIpType_Unknown, NULL); + rgcd[cFetched].pUnk = GetComIPFromObjectRef((OBJECTREF*)(OBJECTHANDLE)m_CurrCookie->m_hndEventProvObj, ComIpType_Unknown, NULL); rgcd[cFetched].dwCookie = m_CurrCookie->m_id; } m_CurrCookie = pConnectionList->GetNext(m_CurrCookie); diff --git a/src/coreclr/vm/comconnectionpoints.h b/src/coreclr/vm/comconnectionpoints.h index f4b90a16d36c4b..173eb0b0870a01 100644 --- a/src/coreclr/vm/comconnectionpoints.h +++ b/src/coreclr/vm/comconnectionpoints.h @@ -35,26 +35,23 @@ struct EventMethodInfo // Structure passed out as a cookie when Advise is called. struct ConnectionCookie { - ConnectionCookie(OBJECTHANDLE hndEventProvObj) : m_hndEventProvObj(hndEventProvObj) + ConnectionCookie(OBJECTHANDLEHolder hndEventProvObj) : m_hndEventProvObj(std::move(hndEventProvObj)) { CONTRACTL { NOTHROW; GC_NOTRIGGER; MODE_ANY; - PRECONDITION(NULL != hndEventProvObj); + PRECONDITION(NULL != m_hndEventProvObj); } CONTRACTL_END; } - ~ConnectionCookie() - { - WRAPPER_NO_CONTRACT; - DestroyHandle(m_hndEventProvObj); - } +public: + ~ConnectionCookie() = default; // Currently called only from Cooperative mode. - static ConnectionCookie* CreateConnectionCookie(OBJECTHANDLE hndEventProvObj) + static ConnectionCookie* CreateConnectionCookie(OBJECTHANDLEHolder hndEventProvObj) { CONTRACT (ConnectionCookie*) { @@ -66,12 +63,12 @@ struct ConnectionCookie } CONTRACT_END; - RETURN (new ConnectionCookie(hndEventProvObj)); + RETURN (new ConnectionCookie(std::move(hndEventProvObj))); } - SLink m_Link; - OBJECTHANDLE m_hndEventProvObj; - DWORD m_id; + SLink m_Link; + OBJECTHANDLEHolder m_hndEventProvObj; + DWORD m_id; }; FORCEINLINE void ConnectionCookieRelease(ConnectionCookie* p) diff --git a/src/coreclr/vm/debugdebugger.cpp b/src/coreclr/vm/debugdebugger.cpp index 30fe96e56f2736..348dfb9f971f79 100644 --- a/src/coreclr/vm/debugdebugger.cpp +++ b/src/coreclr/vm/debugdebugger.cpp @@ -839,8 +839,19 @@ extern "C" MethodDesc* QCALLTYPE StackFrame_GetMethodDescFromNativeIP(LPVOID ip) return pResult; } -FORCEINLINE void HolderDestroyStrongHandle(OBJECTHANDLE h) { if (h != NULL) DestroyStrongHandle(h); } -typedef Wrapper, HolderDestroyStrongHandle, 0> StrongHandleHolder; +struct StrongHandleHolderTraits final +{ + using Type = OBJECTHANDLE; + static constexpr Type Default() { return NULL; } + static void Free(Type handle) + { + WRAPPER_NO_CONTRACT; + if (handle != NULL) + DestroyStrongHandle(handle); + } +}; + +using StrongHandleHolder = LifetimeHolder; // receives a custom notification object from the target and sends it to the RS via // code:Debugger::SendCustomDebuggerNotification @@ -861,7 +872,7 @@ extern "C" void QCALLTYPE DebugDebugger_CustomNotification(QCall::ObjectHandleOn Thread * pThread = GetThread(); AppDomain * pAppDomain = AppDomain::GetCurrentDomain(); - StrongHandleHolder objHandle = pAppDomain->CreateStrongHandle(data.Get()); + StrongHandleHolder objHandle(pAppDomain->CreateStrongHandle(data.Get())); MethodTable* pMT = data.Get()->GetGCSafeMethodTable(); Module* pModule = pMT->GetModule(); DomainAssembly* pDomainAssembly = pModule->GetDomainAssembly(); diff --git a/src/coreclr/vm/gchandleutilities.h b/src/coreclr/vm/gchandleutilities.h index f6f08499222206..ef5fb28a986c2e 100644 --- a/src/coreclr/vm/gchandleutilities.h +++ b/src/coreclr/vm/gchandleutilities.h @@ -350,47 +350,40 @@ inline void DestroyTypedHandle(OBJECTHANDLE handle) // Handle holders/wrappers #ifndef FEATURE_NATIVEAOT -typedef Wrapper, DestroyHandle> OHWrapper; -typedef Wrapper, DestroyPinningHandle, 0> PinningHandleHolder; -typedef Wrapper, DestroyAsyncPinningHandle, 0> AsyncPinningHandleHolder; -typedef Wrapper, DestroyRefcountedHandle> RefCountedOHWrapper; - -typedef Holder, DestroyLongWeakHandle> LongWeakHandleHolder; -typedef Holder, DestroyGlobalStrongHandle> GlobalStrongHandleHolder; -typedef Holder, DestroyGlobalShortWeakHandle> GlobalShortWeakHandleHolder; -typedef Holder, DestroyWeakInteriorHandle> WeakInteriorHandleHolder; -typedef Holder, ResetOBJECTHANDLE> ObjectInHandleHolder; - -class RCOBJECTHANDLEHolder : public RefCountedOHWrapper +struct OBJECTHANDLEHolderTraits final { -public: - FORCEINLINE RCOBJECTHANDLEHolder(OBJECTHANDLE p = NULL) : RefCountedOHWrapper(p) - { - LIMITED_METHOD_CONTRACT; - } - FORCEINLINE void operator=(OBJECTHANDLE p) + using Type = OBJECTHANDLE; + static constexpr Type Default() { return NULL; } + static void Free(Type handle) { WRAPPER_NO_CONTRACT; - - RefCountedOHWrapper::operator=(p); + if (handle != NULL) + DestroyHandle(handle); } }; -class OBJECTHANDLEHolder : public OHWrapper +using OBJECTHANDLEHolder = LifetimeHolder; + +struct PinningHandleHolderTraits final { -public: - FORCEINLINE OBJECTHANDLEHolder(OBJECTHANDLE p = NULL) : OHWrapper(p) - { - LIMITED_METHOD_CONTRACT; - } - FORCEINLINE void operator=(OBJECTHANDLE p) + using Type = OBJECTHANDLE; + static constexpr Type Default() { return NULL; } + static void Free(Type handle) { WRAPPER_NO_CONTRACT; - - OHWrapper::operator=(p); + if (handle != NULL) + DestroyPinningHandle(handle); } }; +using PinningHandleHolder = LifetimeHolder; + +typedef Holder, DestroyLongWeakHandle> LongWeakHandleHolder; +typedef Holder, DestroyGlobalStrongHandle> GlobalStrongHandleHolder; +typedef Holder, DestroyGlobalShortWeakHandle> GlobalShortWeakHandleHolder; +typedef Holder, DestroyWeakInteriorHandle> WeakInteriorHandleHolder; +typedef Holder, ResetOBJECTHANDLE> ObjectInHandleHolder; + #endif // !FEATURE_NATIVEAOT #endif // !DACCESS_COMPILE diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 48e6c2ea8c14c0..82ea65298482e7 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -2879,13 +2879,12 @@ CORINFO_OBJECT_HANDLE CEEInfo::getJitHandleForObject(OBJECTREF objref, bool know m_pJitHandles = new SArray(); } - OBJECTHANDLEHolder handle = AppDomain::GetCurrentDomain()->CreateHandle(objref); + OBJECTHANDLEHolder handle(AppDomain::GetCurrentDomain()->CreateHandle(objref)); m_pJitHandles->Append(handle); - handle.SuppressRelease(); // We know that handle is aligned so we use the lowest bit as a marker // "this is a handle, not a frozen object". - return (CORINFO_OBJECT_HANDLE)((size_t)handle.GetValue() | 1); + return (CORINFO_OBJECT_HANDLE)((size_t)handle.Detach() | 1); } OBJECTREF CEEInfo::getObjectFromJitHandle(CORINFO_OBJECT_HANDLE handle) diff --git a/src/coreclr/vm/olevariant.cpp b/src/coreclr/vm/olevariant.cpp index 26fe57e76d8be7..84da7d78ecbb81 100644 --- a/src/coreclr/vm/olevariant.cpp +++ b/src/coreclr/vm/olevariant.cpp @@ -3586,7 +3586,7 @@ void OleVariant::MarshalSafeArrayForArrayRef(BASEARRAYREF *pArrayRef, else { { - PinningHandleHolder handle = GetAppDomain()->CreatePinningHandle((OBJECTREF)Array); + PinningHandleHolder handle(GetAppDomain()->CreatePinningHandle((OBJECTREF)Array)); if (bArrayOfInterfaceWrappers) { @@ -3684,7 +3684,7 @@ void OleVariant::MarshalArrayRefForSafeArray(SAFEARRAY *pSafeArray, pSrcData = (BYTE*)pSafeArray->pvData; } - PinningHandleHolder handle = GetAppDomain()->CreatePinningHandle((OBJECTREF)*pArrayRef); + PinningHandleHolder handle(GetAppDomain()->CreatePinningHandle((OBJECTREF)*pArrayRef)); marshal->OleToComArray(pSrcData, pArrayRef, pInterfaceMT); } diff --git a/src/coreclr/vm/syncblk.cpp b/src/coreclr/vm/syncblk.cpp index bffd9d2d69c318..d81973bf4e2980 100644 --- a/src/coreclr/vm/syncblk.cpp +++ b/src/coreclr/vm/syncblk.cpp @@ -1735,12 +1735,12 @@ OBJECTHANDLE SyncBlock::GetOrCreateLock(OBJECTREF lockObj) // We'll likely need to put this lock object into the sync block. // Create the handle here. - OBJECTHANDLEHolder lockHandle = GetAppDomain()->CreateHandle(lockObj); + OBJECTHANDLEHolder lockHandle(GetAppDomain()->CreateHandle(lockObj)); - if (TryUpgradeThinLockToFullLock(lockHandle.GetValue())) + if (TryUpgradeThinLockToFullLock(lockHandle)) { // Our lock instance is the one in the sync block now. - return lockHandle.Extract(); + return lockHandle.Detach(); } return VolatileLoad(&m_Lock); From 701e4b6fdc4c35998f14b791720e4e281ca01102 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 3 Apr 2026 16:10:57 -0700 Subject: [PATCH 5/8] Use LifetimeHolder for COM interop holders Replace the COM interop-specific Wrapper-based holder classes with LifetimeHolder, ReleaseHolder, and SpecializedWrapper aliases, and update ownership transfers to use Detach consistently. This removes redundant DoNothing helpers and keeps the holder cleanup aligned with the rest of the ongoing refactoring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/inc/holder.h | 47 ++---- src/coreclr/vm/comcallablewrapper.cpp | 50 +++---- src/coreclr/vm/comcallablewrapper.h | 37 +---- src/coreclr/vm/comconnectionpoints.h | 23 +-- src/coreclr/vm/cominterfacemarshaler.cpp | 3 +- src/coreclr/vm/dispatchinfo.cpp | 4 +- src/coreclr/vm/dispparammarshaler.cpp | 7 +- src/coreclr/vm/interoputil.cpp | 56 +++----- src/coreclr/vm/olevariant.cpp | 69 ++++----- src/coreclr/vm/runtimecallablewrapper.cpp | 4 +- src/coreclr/vm/runtimecallablewrapper.h | 50 +------ src/coreclr/vm/wrappers.h | 165 +++++----------------- 12 files changed, 127 insertions(+), 388 deletions(-) diff --git a/src/coreclr/inc/holder.h b/src/coreclr/inc/holder.h index ca6d404932b90e..d264e03e09ecd0 100644 --- a/src/coreclr/inc/holder.h +++ b/src/coreclr/inc/holder.h @@ -619,26 +619,8 @@ FORCEINLINE void DoNothing() } // Prefast stuff.We should have DoNothing in the holder declaration, but currently -// prefast doesnt support, it, so im stuffing all these here so if we need to change the template you can change +// prefast doesn't support, it, so im stuffing all these here so if we need to change the template you can change // everything here. When prefast works, remove the following functions -struct ConnectionCookie; -FORCEINLINE void ConnectionCookieDoNothing(ConnectionCookie* p) -{ -} - -class ComCallWrapper; -FORCEINLINE void CCWHolderDoNothing(ComCallWrapper* p) -{ -} - - -FORCEINLINE void DispParamHolderDoNothing(VARIANT* p) -{ -} - -FORCEINLINE void VariantPtrDoNothing(VARIANT* p) -{ -} FORCEINLINE void VariantDoNothing(VARIANT) { @@ -648,22 +630,6 @@ FORCEINLINE void ZeroDoNothing(VOID* p) { } -class CtxEntry; -FORCEINLINE void CtxEntryDoNothing(CtxEntry* p) -{ -} - -struct RCW; -FORCEINLINE void NewRCWHolderDoNothing(RCW*) -{ -} - -// Prefast stuff.We should have DoNothing in the holder declaration -FORCEINLINE void SafeArrayDoNothing(SAFEARRAY* p) -{ -} - - //----------------------------------------------------------------------------- // Holder/Wrapper are the simplest way to define holders - they synthesizes a base class out of // function pointers @@ -1145,7 +1111,16 @@ class LifetimeHolder final } operator Type() const { STATIC_CONTRACT_LEAF; return m_value; } - Type* operator&() { STATIC_CONTRACT_LEAF; _ASSERTE(m_value == T::Default()); return &m_value; } + + Type* operator&() + { + STATIC_CONTRACT_LEAF; +#ifdef _DEBUG + Type tmp = T::Default(); + _ASSERTE(memcmp(&m_value, &tmp, sizeof(Type)) == 0 && "Taking address of non-empty holder"); +#endif // _DEBUG + return &m_value; + } template ::value>> U operator->() const diff --git a/src/coreclr/vm/comcallablewrapper.cpp b/src/coreclr/vm/comcallablewrapper.cpp index b0f1341a4632eb..518e8c9996583c 100644 --- a/src/coreclr/vm/comcallablewrapper.cpp +++ b/src/coreclr/vm/comcallablewrapper.cpp @@ -338,44 +338,30 @@ extern "C" PCODE ComPreStubWorker(UMEntryThunkData* pEntryThunk) return pStub; } -FORCEINLINE void CPListRelease(CQuickArray* value) -{ - WRAPPER_NO_CONTRACT; - - if (value) - { - // Delete all the connection points. - for (UINT i = 0; i < value->Size(); i++) - delete (*value)[i]; - - // Delete the list itself. - delete value; - } -} - typedef CQuickArray CPArray; -FORCEINLINE void CPListDoNothing(CPArray*) +struct CPListHolderTraits final { - LIMITED_METHOD_CONTRACT; -} - -class CPListHolder : public Wrapper -{ -public: - CPListHolder(CPArray* p = NULL) - : Wrapper(p) + using Type = CPArray*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { WRAPPER_NO_CONTRACT; - } - FORCEINLINE void operator=(CPArray* p) - { - WRAPPER_NO_CONTRACT; - Wrapper::operator=(p); + if (value != NULL) + { + // Delete all the connection points. + for (UINT i = 0; i < value->Size(); i++) + delete (*value)[i]; + + // Delete the list itself. + delete value; + } } }; +using CPListHolder = LifetimeHolder; + NOINLINE void LogCCWRefCountChange_BREAKPOINT(ComCallWrapper *pCCW) { LIMITED_METHOD_CONTRACT; @@ -816,7 +802,7 @@ void SimpleComCallWrapper::SetUpCPListHelper(MethodTable **apSrcItfMTs, int cSrc } CONTRACTL_END; - CPListHolder pCPList = NULL; + CPListHolder pCPList; ComCallWrapper *pWrap = GetMainWrapper(); int NumCPs = 0; @@ -844,8 +830,8 @@ void SimpleComCallWrapper::SetUpCPListHelper(MethodTable **apSrcItfMTs, int cSrc // Finally, we set the connection point list in the simple wrapper. If // no other thread already set it, we set pCPList to NULL to indicate // that ownership has been transferred to the simple wrapper. - if (InterlockedCompareExchangeT(&m_pCPList, pCPList.GetValue(), NULL) == NULL) - pCPList.SuppressRelease(); + if (InterlockedCompareExchangeT(&m_pCPList, static_cast(pCPList), NULL) == NULL) + pCPList.Detach(); } ConnectionPoint *SimpleComCallWrapper::TryCreateConnectionPoint(ComCallWrapper *pWrap, MethodTable *pEventMT) diff --git a/src/coreclr/vm/comcallablewrapper.h b/src/coreclr/vm/comcallablewrapper.h index 163ad53f53b279..85ba9971adfae1 100644 --- a/src/coreclr/vm/comcallablewrapper.h +++ b/src/coreclr/vm/comcallablewrapper.h @@ -330,18 +330,7 @@ class ComCallWrapperTemplate SLOT* m_rgpIPtr[1]; }; -inline void ComCallWrapperTemplateRelease(ComCallWrapperTemplate *value) -{ - WRAPPER_NO_CONTRACT; - - if (value) - { - value->Release(); - } -} - -typedef Wrapper, ComCallWrapperTemplateRelease, 0> ComCallWrapperTemplateHolder; - +using ComCallWrapperTemplateHolder = ReleaseHolder; //-------------------------------------------------------------------------------- // Header on top of Vtables that we create for COM callable interfaces @@ -1054,29 +1043,7 @@ struct cdac_data static constexpr uintptr_t ThisMask = (uintptr_t)ComCallWrapper::enum_ThisMask; }; -FORCEINLINE void CCWRelease(ComCallWrapper* p) -{ - WRAPPER_NO_CONTRACT; - - p->Release(); -} - -class CCWHolder : public Wrapper -{ -public: - CCWHolder(ComCallWrapper* p = NULL) - : Wrapper(p) - { - WRAPPER_NO_CONTRACT; - } - - FORCEINLINE void operator=(ComCallWrapper* p) - { - WRAPPER_NO_CONTRACT; - - Wrapper::operator=(p); - } -}; +using CCWHolder = ReleaseHolder; // // Uncommonly used data on Simple CCW // Created on-demand diff --git a/src/coreclr/vm/comconnectionpoints.h b/src/coreclr/vm/comconnectionpoints.h index 173eb0b0870a01..97311c4ade04f6 100644 --- a/src/coreclr/vm/comconnectionpoints.h +++ b/src/coreclr/vm/comconnectionpoints.h @@ -71,29 +71,8 @@ struct ConnectionCookie DWORD m_id; }; -FORCEINLINE void ConnectionCookieRelease(ConnectionCookie* p) -{ - WRAPPER_NO_CONTRACT; - - delete p; -} - // Connection cookie holder used to ensure the cookies are deleted when required. -class ConnectionCookieHolder : public Wrapper -{ -public: - ConnectionCookieHolder(ConnectionCookie* p = NULL) - : Wrapper(p) - { - WRAPPER_NO_CONTRACT; - } - - FORCEINLINE void operator=(ConnectionCookie* p) - { - WRAPPER_NO_CONTRACT; - Wrapper::operator=(p); - } -}; +using ConnectionCookieHolder = NewHolder; // List of connection cookies. typedef SList CONNECTIONCOOKIELIST; diff --git a/src/coreclr/vm/cominterfacemarshaler.cpp b/src/coreclr/vm/cominterfacemarshaler.cpp index 63cf04b1af630a..12ee7d005cb3ca 100644 --- a/src/coreclr/vm/cominterfacemarshaler.cpp +++ b/src/coreclr/vm/cominterfacemarshaler.cpp @@ -129,8 +129,7 @@ void COMInterfaceMarshaler::CreateObjectRef(BOOL fDuplicate, OBJECTREF *pComObj, pSB->SetPrecious(); DWORD dwSyncBlockIndex = pSB->GetSyncBlockIndex(); - NewRCWHolder pNewRCW; - pNewRCW = RCW::CreateRCW(m_pUnknown, dwSyncBlockIndex, m_flags, m_typeHandle.GetMethodTable()); + NewRCWHolder pNewRCW(RCW::CreateRCW(m_pUnknown, dwSyncBlockIndex, m_flags, m_typeHandle.GetMethodTable())); if (fDuplicate) { diff --git a/src/coreclr/vm/dispatchinfo.cpp b/src/coreclr/vm/dispatchinfo.cpp index 21ecf93f664a45..68aed360cd9b8a 100644 --- a/src/coreclr/vm/dispatchinfo.cpp +++ b/src/coreclr/vm/dispatchinfo.cpp @@ -1134,7 +1134,7 @@ void DispatchInfo::InvokeMemberWorker(DispatchMemberInfo* pDispMemberInfo, Thread* pThread = GetThread(); AppDomain* pAppDomain = AppDomain::GetCurrentDomain(); - SafeArrayPtrHolder pSA = NULL; + SafeArrayPtrHolder pSA; VARIANT safeArrayVar; HRESULT hr; @@ -1331,7 +1331,7 @@ void DispatchInfo::InvokeMemberWorker(DispatchMemberInfo* pDispMemberInfo, LONG lSafeArrayArg = 0; bByRefArg = FALSE; pSA = SafeArrayCreateVector(VT_VARIANT, 0, iSrcArg - NumNamedArgs + 1); - if (pSA.GetValue() == NULL) + if (pSA == NULL) COMPlusThrowHR(E_OUTOFMEMORY); V_VT(&safeArrayVar) = VT_VARIANT | VT_ARRAY; V_ARRAY(&safeArrayVar) = pSA; diff --git a/src/coreclr/vm/dispparammarshaler.cpp b/src/coreclr/vm/dispparammarshaler.cpp index b7d7923d86e88a..1c9c9a22d41a4a 100644 --- a/src/coreclr/vm/dispparammarshaler.cpp +++ b/src/coreclr/vm/dispparammarshaler.cpp @@ -265,7 +265,7 @@ void DispParamArrayMarshaler::MarshalManagedToNative(OBJECTREF *pSrcObj, VARIANT } CONTRACTL_END; - SafeArrayPtrHolder pSafeArray = NULL; + SafeArrayPtrHolder pSafeArray; VARTYPE vt = m_ElementVT; MethodTable *pElemMT = m_pElementMT; @@ -294,11 +294,8 @@ void DispParamArrayMarshaler::MarshalManagedToNative(OBJECTREF *pSrcObj, VARIANT } // Store the resulting SAFEARRAY in the destination VARIANT. - V_ARRAY(pDestVar) = pSafeArray; + V_ARRAY(pDestVar) = pSafeArray.Detach(); V_VT(pDestVar) = VT_ARRAY | vt; - - // Don't destroy the safearray. - pSafeArray.SuppressRelease(); } void DispParamArrayMarshaler::MarshalManagedToNativeRef(OBJECTREF *pSrcObj, VARIANT *pRefVar) diff --git a/src/coreclr/vm/interoputil.cpp b/src/coreclr/vm/interoputil.cpp index e3de11420a3c09..6e60e690da19d1 100644 --- a/src/coreclr/vm/interoputil.cpp +++ b/src/coreclr/vm/interoputil.cpp @@ -2957,45 +2957,35 @@ static void DoIUInvokeDispMethod(IDispatchEx* pDispEx, IDispatch* pDisp, DISPID GCPROTECT_END(); } - -FORCEINLINE void DispParamHolderRelease(VARIANT* value) +struct DispParamHolderTraits final { - CONTRACTL + using Type = VARIANT*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { - THROWS; - GC_TRIGGERS; - MODE_ANY; - } - CONTRACTL_END; - - if (value) - { - if (V_VT(value) & VT_BYREF) - { - VariantHolder TmpVar; - OleVariant::ExtractContentsFromByrefVariant(value, &TmpVar); - } - - SafeVariantClear(value); - } -} + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_ANY; + } + CONTRACTL_END; -class DispParamHolder : public Wrapper -{ -public: - DispParamHolder(VARIANT* p = NULL) - : Wrapper(p) - { - WRAPPER_NO_CONTRACT; - } + if (value) + { + if (V_VT(value) & VT_BYREF) + { + VariantHolder TmpVar; + OleVariant::ExtractContentsFromByrefVariant(value, &TmpVar); + } - FORCEINLINE void operator=(VARIANT* p) - { - WRAPPER_NO_CONTRACT; - Wrapper::operator=(p); + SafeVariantClear(value); + } } }; +using DispParamHolder = LifetimeHolder; + //-------------------------------------------------------------------------------- // This methods converts an IEnumVARIANT to a managed IEnumerator. static OBJECTREF ConvertEnumVariantToMngEnum(IEnumVARIANT* pNativeEnum) @@ -3058,7 +3048,7 @@ void IUInvokeDispMethod( SafeComHolder pUnk = NULL; SafeComHolder pDisp = NULL; SafeComHolder pDispEx = NULL; - VariantPtrHolder pVarResult = NULL; + VariantPtrHolder pVarResult; NewArrayHolder params = NULL; // diff --git a/src/coreclr/vm/olevariant.cpp b/src/coreclr/vm/olevariant.cpp index 84da7d78ecbb81..f082e8b2f1ae40 100644 --- a/src/coreclr/vm/olevariant.cpp +++ b/src/coreclr/vm/olevariant.cpp @@ -898,51 +898,38 @@ void SafeVariantClear(VARIANT* pVar) } } -class VariantEmptyHolder : public Wrapper, SafeVariantClear, 0> +struct VariantEmptyHolderTraits final { -public: - VariantEmptyHolder(VARIANT* p = NULL) : - Wrapper, SafeVariantClear, 0>(p) + using Type = VARIANT*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { WRAPPER_NO_CONTRACT; - } - - FORCEINLINE void operator=(VARIANT* p) - { - WRAPPER_NO_CONTRACT; - - Wrapper, SafeVariantClear, 0>::operator=(p); + SafeVariantClear(value); } }; -FORCEINLINE void RecordVariantRelease(VARIANT* value) -{ - if (value) - { - WRAPPER_NO_CONTRACT; +using VariantEmptyHolder = LifetimeHolder; - if (V_RECORD(value)) - V_RECORDINFO(value)->RecordDestroy(V_RECORD(value)); - if (V_RECORDINFO(value)) - V_RECORDINFO(value)->Release(); - } -} - -class RecordVariantHolder : public Wrapper, RecordVariantRelease, 0> +struct RecordVariantHolderTraits final { -public: - RecordVariantHolder(VARIANT* p = NULL) - : Wrapper, RecordVariantRelease, 0>(p) + using Type = VARIANT*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { - WRAPPER_NO_CONTRACT; - } + LIMITED_METHOD_CONTRACT; - FORCEINLINE void operator=(VARIANT* p) - { - WRAPPER_NO_CONTRACT; - Wrapper, RecordVariantRelease, 0>::operator=(p); + if (value != NULL) + { + if (V_RECORD(value)) + V_RECORDINFO(value)->RecordDestroy(V_RECORD(value)); + if (V_RECORDINFO(value)) + V_RECORDINFO(value)->Release(); + } } }; + +using RecordVariantHolder = LifetimeHolder; #endif // FEATURE_COMINTEROP /* ------------------------------------------------------------------------- * @@ -2762,7 +2749,7 @@ void OleVariant::MarshalOleVariantForObjectUncommon(OBJECTREF * const & pObj, VA convertObjectToVariant.InvokeThrowing(pObj, pOle); } - veh.SuppressRelease(); + veh.Detach(); } void OleVariant::MarshalInterfaceArrayComToOleHelper(BASEARRAYREF *pComArray, void *oleArray, @@ -3210,7 +3197,7 @@ void OleVariant::MarshalArrayVariantObjectToOle(OBJECTREF * const & pObj, } CONTRACTL_END; - SafeArrayPtrHolder pSafeArray = NULL; + SafeArrayPtrHolder pSafeArray; BASEARRAYREF *pArrayRef = (BASEARRAYREF *) pObj; MethodTable *pElemMT = NULL; @@ -3227,8 +3214,7 @@ void OleVariant::MarshalArrayVariantObjectToOle(OBJECTREF * const & pObj, pSafeArray = CreateSafeArrayForArrayRef(pArrayRef, vt, pElemMT); MarshalSafeArrayForArrayRef(pArrayRef, pSafeArray, vt, pElemMT); } - V_ARRAY(pOleVariant) = pSafeArray; - pSafeArray.SuppressRelease(); + V_ARRAY(pOleVariant) = pSafeArray.Detach(); } void OleVariant::MarshalArrayVariantOleRefToObject(const VARIANT *pOleVariant, @@ -3299,7 +3285,7 @@ SAFEARRAY *OleVariant::CreateSafeArrayDescriptorForArrayRef(BASEARRAYREF *pArray ULONG nElem = (*pArrayRef)->GetNumComponents(); ULONG nRank = (*pArrayRef)->GetRank(); - SafeArrayPtrHolder pSafeArray = NULL; + SafeArrayPtrHolder pSafeArray; IfFailThrow(SafeArrayAllocDescriptorEx(vt, nRank, &pSafeArray)); @@ -3384,8 +3370,7 @@ SAFEARRAY *OleVariant::CreateSafeArrayDescriptorForArrayRef(BASEARRAYREF *pArray IfFailThrow(SafeArraySetRecordInfo(pSafeArray, pRecInfo)); } - pSafeArray.SuppressRelease(); - RETURN pSafeArray; + RETURN pSafeArray.Detach(); } // @@ -3705,7 +3690,7 @@ void OleVariant::ConvertValueClassToVariant(OBJECTREF *pBoxedValueClass, VARIANT HRESULT hr = S_OK; SafeComHolder pTypeInfo = NULL; - RecordVariantHolder pRecHolder = pOleVariant; + RecordVariantHolder pRecHolder(pOleVariant); // Initialize the OLE variant's VT_RECORD fields to NULL. V_RECORDINFO(pRecHolder) = NULL; @@ -3758,7 +3743,7 @@ void OleVariant::ConvertValueClassToVariant(OBJECTREF *pBoxedValueClass, VARIANT V_RECORD(pRecHolder)); } - pRecHolder.SuppressRelease(); + pRecHolder.Detach(); } void OleVariant::TransposeArrayData(BYTE *pDestData, BYTE *pSrcData, SIZE_T dwNumComponents, SIZE_T dwComponentSize, SAFEARRAY *pSafeArray, BOOL bSafeArrayToMngArray) diff --git a/src/coreclr/vm/runtimecallablewrapper.cpp b/src/coreclr/vm/runtimecallablewrapper.cpp index 6c028b2108c14c..f705d67b5c98da 100644 --- a/src/coreclr/vm/runtimecallablewrapper.cpp +++ b/src/coreclr/vm/runtimecallablewrapper.cpp @@ -2041,6 +2041,8 @@ BOOL RCW::AllowEagerSTACleanup() return m_Flags.m_fAllowEagerSTACleanup; } +using CtxEntryHolder = ReleaseHolder; + HRESULT RCW::EnterContext(PFNCTXCALLBACK pCallbackFunc, LPVOID pData) { CONTRACTL @@ -2053,7 +2055,7 @@ HRESULT RCW::EnterContext(PFNCTXCALLBACK pCallbackFunc, LPVOID pData) } CONTRACTL_END; - CtxEntryHolder pCtxEntry = GetWrapperCtxEntry(); + CtxEntryHolder pCtxEntry(GetWrapperCtxEntry()); return pCtxEntry->EnterContext(pCallbackFunc, pData); } diff --git a/src/coreclr/vm/runtimecallablewrapper.h b/src/coreclr/vm/runtimecallablewrapper.h index 9a0f4bcf8e5f34..fec2211911b14e 100644 --- a/src/coreclr/vm/runtimecallablewrapper.h +++ b/src/coreclr/vm/runtimecallablewrapper.h @@ -757,21 +757,7 @@ FORCEINLINE void NewRCWHolderRelease(RCW* p) } }; -class NewRCWHolder : public Wrapper -{ -public: - NewRCWHolder(RCW* p = NULL) - : Wrapper(p) - { - WRAPPER_NO_CONTRACT; - } - - FORCEINLINE void operator=(RCW* p) - { - WRAPPER_NO_CONTRACT; - Wrapper::operator=(p); - } -}; +using NewRCWHolder = SpecializedWrapper; #ifndef DACCESS_COMPILE class RCWHolder @@ -1384,38 +1370,4 @@ struct cdac_data static constexpr size_t FirstBucket = offsetof(RCWCleanupList, m_pFirstBucket); }; -FORCEINLINE void CtxEntryHolderRelease(CtxEntry *p) -{ - CONTRACTL - { - NOTHROW; - GC_TRIGGERS; - MODE_ANY; - } - CONTRACTL_END; - - if (p != NULL) - { - p->Release(); - } -} - -class CtxEntryHolder : public Wrapper -{ -public: - CtxEntryHolder(CtxEntry *p = NULL) - : Wrapper(p) - { - WRAPPER_NO_CONTRACT; - } - - FORCEINLINE void operator=(CtxEntry *p) - { - WRAPPER_NO_CONTRACT; - - Wrapper::operator=(p); - } - -}; - #endif // _RUNTIMECALLABLEWRAPPER_H diff --git a/src/coreclr/vm/wrappers.h b/src/coreclr/vm/wrappers.h index ff032765684a4c..16f26f8439fb7a 100644 --- a/src/coreclr/vm/wrappers.h +++ b/src/coreclr/vm/wrappers.h @@ -51,37 +51,6 @@ class MDEnumHolder IMDInternalImport* m_IMDII; }; - -//-------------------------------------------------------------------------------- -// safe variant helper -void SafeVariantClear(_Inout_ VARIANT* pVar); - -class VariantHolder -{ -public: - inline VariantHolder() - { - LIMITED_METHOD_CONTRACT; - memset(&m_var, 0, sizeof(VARIANT)); - } - - inline ~VariantHolder() - { - WRAPPER_NO_CONTRACT; - SafeVariantClear(&m_var); - } - - inline VARIANT* operator&() - { - LIMITED_METHOD_CONTRACT; - return static_cast(&m_var); - } - -private: - VARIANT m_var; -}; - - template inline void SafeComRelease(TYPE *value) { @@ -113,26 +82,9 @@ using SafeComHolder = SpecializedWrapper<_TYPE, SafeComRelease<_TYPE>>; template using SafeComHolderPreemp = SpecializedWrapper<_TYPE, SafeComReleasePreemp<_TYPE>>; -//----------------------------------------------------------------------------- -// NewPreempHolder : New'ed memory holder, deletes in preemp mode. -// -// { -// NewPreempHolder foo = new Foo (); -// } // delete foo on out of scope in preemp mode. -//----------------------------------------------------------------------------- - -template -void DeletePreemp(TYPE *value) -{ - WRAPPER_NO_CONTRACT; - - GCX_PREEMP(); - delete value; -} - -template -using NewPreempHolder = SpecializedWrapper<_TYPE, DeletePreemp<_TYPE>>; - +//-------------------------------------------------------------------------------- +// safe variant helper +void SafeVariantClear(_Inout_ VARIANT* pVar); //----------------------------------------------------------------------------- // VariantPtrHolder : Variant holder, Calls VariantClear on scope exit. @@ -142,33 +94,35 @@ using NewPreempHolder = SpecializedWrapper<_TYPE, DeletePreemp<_TYPE>>; // } // Call SafeVariantClear on out of scope. //----------------------------------------------------------------------------- -FORCEINLINE void VariantPtrRelease(VARIANT* value) +struct VariantHolderTraits final { - WRAPPER_NO_CONTRACT; - - if (value) + using Type = VARIANT; + static constexpr Type Default() { return {}; } + static void Free(Type& value) { - SafeVariantClear(value); + WRAPPER_NO_CONTRACT; + SafeVariantClear(&value); } -} +}; -class VariantPtrHolder : public Wrapper -{ -public: - VariantPtrHolder(VARIANT* p = NULL) - : Wrapper(p) - { - LIMITED_METHOD_CONTRACT; - } +using VariantHolder = LifetimeHolder; - FORCEINLINE void operator=(VARIANT* p) +struct VariantPtrHolderTraits final +{ + using Type = VARIANT*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { WRAPPER_NO_CONTRACT; - - Wrapper::operator=(p); + if (value != NULL) + { + SafeVariantClear(value); + } } }; +using VariantPtrHolder = LifetimeHolder; + #ifdef FEATURE_COMINTEROP //----------------------------------------------------------------------------- // SafeArrayPtrHolder : SafeArray holder, Calls SafeArrayDestroy on scope exit. @@ -179,75 +133,28 @@ class VariantPtrHolder : public Wrapper -{ -public: - SafeArrayPtrHolder(SAFEARRAY* p = NULL) - : Wrapper(p) - { - LIMITED_METHOD_CONTRACT; - } - - FORCEINLINE void operator=(SAFEARRAY* p) + using Type = SAFEARRAY*; + static constexpr Type Default() { return NULL; } + static void Free(Type value) { WRAPPER_NO_CONTRACT; - Wrapper::operator=(p); - } -}; - -#endif // FEATURE_COMINTEROP - -//----------------------------------------------------------------------------- -// ZeroHolder : Sets value to zero on context exit. -// -// { -// ZeroHolder foo = &data; -// } // set data to zero on context exit -//----------------------------------------------------------------------------- - -FORCEINLINE void ZeroRelease(VOID* value) -{ - LIMITED_METHOD_CONTRACT; - if (value) - { - (*(size_t*)value) = 0; - } -} - -class ZeroHolder : public Wrapper -{ -public: - ZeroHolder(VOID* p = NULL) - : Wrapper(p) - { - LIMITED_METHOD_CONTRACT; - } - - FORCEINLINE void operator=(VOID* p) - { - WRAPPER_NO_CONTRACT; + if (value != NULL) + { + // SafeArrayDestroy may block and may also call back to MODE_PREEMPTIVE + // runtime functions like e.g. code:Unknown_Release_Internal + GCX_PREEMP(); - Wrapper::operator=(p); + HRESULT hr; hr = SafeArrayDestroy(value); + _ASSERTE(SUCCEEDED(hr)); + } } }; -#ifdef FEATURE_COMINTEROP +using SafeArrayPtrHolder = LifetimeHolder; + class TYPEATTRHolder { public: From f941d6160492f41170d7a2abb9a9f2d291b141d3 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Fri, 3 Apr 2026 16:16:48 -0700 Subject: [PATCH 6/8] Remove additional unused methods. --- src/coreclr/inc/holder.h | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/coreclr/inc/holder.h b/src/coreclr/inc/holder.h index d264e03e09ecd0..2496f4b5a5754f 100644 --- a/src/coreclr/inc/holder.h +++ b/src/coreclr/inc/holder.h @@ -618,18 +618,6 @@ FORCEINLINE void DoNothing() { } -// Prefast stuff.We should have DoNothing in the holder declaration, but currently -// prefast doesn't support, it, so im stuffing all these here so if we need to change the template you can change -// everything here. When prefast works, remove the following functions - -FORCEINLINE void VariantDoNothing(VARIANT) -{ -} - -FORCEINLINE void ZeroDoNothing(VOID* p) -{ -} - //----------------------------------------------------------------------------- // Holder/Wrapper are the simplest way to define holders - they synthesizes a base class out of // function pointers From f508ea117c6b8fa04d5fd0d49b28a5e8c47e56f6 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 6 Apr 2026 11:10:35 -0700 Subject: [PATCH 7/8] Copilot feedback. Define RCW/CCW events under FEATURE_COMINTEROP. --- src/coreclr/inc/holder.h | 2 +- src/coreclr/vm/assemblynative.cpp | 4 ++-- src/coreclr/vm/assemblyspec.cpp | 2 +- src/coreclr/vm/eventtrace_bulktype.cpp | 19 ++----------------- src/coreclr/vm/eventtrace_gcheap.cpp | 8 ++++++-- src/coreclr/vm/eventtracepriv.h | 3 ++- 6 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/coreclr/inc/holder.h b/src/coreclr/inc/holder.h index 2496f4b5a5754f..df1d7b365c05f5 100644 --- a/src/coreclr/inc/holder.h +++ b/src/coreclr/inc/holder.h @@ -619,7 +619,7 @@ FORCEINLINE void DoNothing() } //----------------------------------------------------------------------------- -// Holder/Wrapper are the simplest way to define holders - they synthesizes a base class out of +// Holder/Wrapper are the simplest way to define holders - they synthesize a base class from // function pointers //----------------------------------------------------------------------------- diff --git a/src/coreclr/vm/assemblynative.cpp b/src/coreclr/vm/assemblynative.cpp index 498a15f65c03c3..aaadceeaf00732 100644 --- a/src/coreclr/vm/assemblynative.cpp +++ b/src/coreclr/vm/assemblynative.cpp @@ -196,13 +196,13 @@ extern "C" void QCALLTYPE AssemblyNative_LoadFromPath(INT_PTR ptrNativeAssemblyB // Need to verify that this is a valid CLR assembly. if (!pILImage->CheckILFormat()) - THROW_BAD_FORMAT(BFA_BAD_IL, (PEImage*)pILImage); + THROW_BAD_FORMAT(BFA_BAD_IL, static_cast(pILImage)); LoaderAllocator* pLoaderAllocator = pBinder->GetLoaderAllocator(); if (pLoaderAllocator && pLoaderAllocator->IsCollectible() && !pILImage->IsILOnly()) { // Loading IJW assemblies into a collectible AssemblyLoadContext is not allowed - THROW_BAD_FORMAT(BFA_IJW_IN_COLLECTIBLE_ALC, (PEImage*)pILImage); + THROW_BAD_FORMAT(BFA_IJW_IN_COLLECTIBLE_ALC, static_cast(pILImage)); } } diff --git a/src/coreclr/vm/assemblyspec.cpp b/src/coreclr/vm/assemblyspec.cpp index 75bc09d1902ed2..e938a6a62eaaa0 100644 --- a/src/coreclr/vm/assemblyspec.cpp +++ b/src/coreclr/vm/assemblyspec.cpp @@ -431,7 +431,7 @@ Assembly *AssemblySpec::LoadAssembly(LPCWSTR pFilePath) // Need to verify that this is a valid CLR assembly. if (!pILImage->CheckILFormat()) - THROW_BAD_FORMAT(BFA_BAD_IL, (PEImage*)pILImage); + THROW_BAD_FORMAT(BFA_BAD_IL, static_cast(pILImage)); RETURN AssemblyNative::LoadFromPEImage(AppDomain::GetCurrentDomain()->GetDefaultBinder(), pILImage, true /* excludeAppPaths */); } diff --git a/src/coreclr/vm/eventtrace_bulktype.cpp b/src/coreclr/vm/eventtrace_bulktype.cpp index 44dcdb902fa8f5..c4c1d3cb635746 100644 --- a/src/coreclr/vm/eventtrace_bulktype.cpp +++ b/src/coreclr/vm/eventtrace_bulktype.cpp @@ -19,6 +19,7 @@ #include "eventtracepriv.h" +#ifdef FEATURE_COMINTEROP //--------------------------------------------------------------------------------------- // BulkComLogger: Batches up and logs RCW and CCW //--------------------------------------------------------------------------------------- @@ -90,7 +91,6 @@ void BulkComLogger::WriteRcw(RCW *pRcw, Object *obj) _ASSERTE(m_currRcw < kMaxRcwCount); -#ifdef FEATURE_COMINTEROP TypeHandle typeHandle = obj->GetGCSafeTypeHandleIfPossible(); if (typeHandle == NULL) { @@ -106,7 +106,6 @@ void BulkComLogger::WriteRcw(RCW *pRcw, Object *obj) if (++m_currRcw >= kMaxRcwCount) FlushRcw(); -#endif } void BulkComLogger::FlushRcw() @@ -132,16 +131,12 @@ void BulkComLogger::FlushRcw() unsigned short instance = GetClrInstanceId(); -#if !defined(HOST_UNIX) EVENT_DATA_DESCRIPTOR eventData[3]; EventDataDescCreate(&eventData[0], &m_currRcw, sizeof(const unsigned int)); EventDataDescCreate(&eventData[1], &instance, sizeof(const unsigned short)); EventDataDescCreate(&eventData[2], m_etwRcwData, sizeof(EventRCWEntry) * m_currRcw); ULONG result = EventWrite(Microsoft_Windows_DotNETRuntimeHandle, &GCBulkRCW, ARRAY_SIZE(eventData), eventData); -#else - ULONG result = FireEtXplatGCBulkRCW(m_currRcw, instance, sizeof(EventRCWEntry) * m_currRcw, m_etwRcwData); -#endif // !defined(HOST_UNIX) result |= EventPipeWriteEventGCBulkRCW(m_currRcw, instance, sizeof(EventRCWEntry) * m_currRcw, m_etwRcwData); _ASSERTE(result == ERROR_SUCCESS); @@ -163,7 +158,6 @@ void BulkComLogger::WriteCcw(ComCallWrapper *pCcw, Object **handle, Object *obj) _ASSERTE(m_currCcw < kMaxCcwCount); -#ifdef FEATURE_COMINTEROP IUnknown *iUnk = NULL; int refCount = 0; ULONG flags = 0; @@ -197,7 +191,6 @@ void BulkComLogger::WriteCcw(ComCallWrapper *pCcw, Object **handle, Object *obj) if (m_currCcw >= kMaxCcwCount) FlushCcw(); -#endif } void BulkComLogger::FlushCcw() @@ -223,16 +216,12 @@ void BulkComLogger::FlushCcw() unsigned short instance = GetClrInstanceId(); -#if !defined(HOST_UNIX) EVENT_DATA_DESCRIPTOR eventData[3]; EventDataDescCreate(&eventData[0], &m_currCcw, sizeof(const unsigned int)); EventDataDescCreate(&eventData[1], &instance, sizeof(const unsigned short)); EventDataDescCreate(&eventData[2], m_etwCcwData, sizeof(EventCCWEntry) * m_currCcw); ULONG result = EventWrite(Microsoft_Windows_DotNETRuntimeHandle, &GCBulkRootCCW, ARRAY_SIZE(eventData), eventData); -#else - ULONG result = FireEtXplatGCBulkRootCCW(m_currCcw, instance, sizeof(EventCCWEntry) * m_currCcw, m_etwCcwData); -#endif //!defined(HOST_UNIX) result |= EventPipeWriteEventGCBulkRootCCW(m_currCcw, instance, sizeof(EventCCWEntry) * m_currCcw, m_etwCcwData); _ASSERTE(result == ERROR_SUCCESS); @@ -366,16 +355,12 @@ void BulkComLogger::AddCcwHandle(Object **handle) curr->Handles[curr->Count++] = handle; } - - - +#endif // FEATURE_COMINTEROP //--------------------------------------------------------------------------------------- // BulkStaticsLogger: Batches up and logs static variable roots //--------------------------------------------------------------------------------------- - - #include "domainassembly.h" BulkStaticsLogger::BulkStaticsLogger(BulkTypeEventLogger *typeLogger) diff --git a/src/coreclr/vm/eventtrace_gcheap.cpp b/src/coreclr/vm/eventtrace_gcheap.cpp index 4a54052e386143..9801a76e26ae2f 100644 --- a/src/coreclr/vm/eventtrace_gcheap.cpp +++ b/src/coreclr/vm/eventtrace_gcheap.cpp @@ -61,11 +61,14 @@ BOOL ETW::GCLog::ShouldTrackMovementForEtw() BOOL ETW::GCLog::ShouldWalkStaticsAndCOMForEtw() { LIMITED_METHOD_CONTRACT; - +#ifdef FEATURE_COMINTEROP return s_forcedGCInProgress && ETW_TRACING_CATEGORY_ENABLED(MICROSOFT_WINDOWS_DOTNETRUNTIME_PROVIDER_DOTNET_Context, TRACE_LEVEL_INFORMATION, CLR_GCHEAPDUMP_KEYWORD); +#else + return FALSE; +#endif // FEATURE_COMINTEROP } // Batches the list of moved/surviving references for the GCBulkMovedObjectRanges / @@ -505,9 +508,9 @@ HRESULT ETW::GCLog::ForceGCForDiagnostics() //--------------------------------------------------------------------------------------- // WalkStaticsAndCOMForETW walks both CCW/RCW objects and static variables. //--------------------------------------------------------------------------------------- - VOID ETW::GCLog::WalkStaticsAndCOMForETW() { +#ifdef FEATURE_COMINTEROP CONTRACTL { NOTHROW; @@ -537,6 +540,7 @@ VOID ETW::GCLog::WalkStaticsAndCOMForETW() { } EX_END_CATCH +#endif // FEATURE_COMINTEROP } diff --git a/src/coreclr/vm/eventtracepriv.h b/src/coreclr/vm/eventtracepriv.h index b262a4fd787da5..a1772c8a4fdef6 100644 --- a/src/coreclr/vm/eventtracepriv.h +++ b/src/coreclr/vm/eventtracepriv.h @@ -334,7 +334,7 @@ class BulkTypeEventLogger void FireBulkTypeEvent(); }; - +#ifdef FEATURE_COMINTEROP // Does all logging for RCWs and CCWs in the process. We walk RCWs by enumerating all syncblocks in // the process and seeing if they have associated interop information. We enumerate all CCWs in the // process from the RefCount handles on the handle table. @@ -398,6 +398,7 @@ class BulkComLogger CCWEnumerationEntry *m_enumResult; }; +#endif // FEATURE_COMINTEROP // Does bulk static variable ETW logging. From 2497b67e084b39bf5cbd8f24d7ae7c443709b889 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Mon, 6 Apr 2026 17:12:36 -0700 Subject: [PATCH 8/8] Remove unused RCOBJECTHANDLEHolder forward declaration and clean up COM interop related code --- src/coreclr/nativeaot/Runtime/forward_declarations.h | 1 - src/coreclr/vm/eventtrace_gcheap.cpp | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/forward_declarations.h b/src/coreclr/nativeaot/Runtime/forward_declarations.h index 40acdc77b9831e..c228266b6c2804 100644 --- a/src/coreclr/nativeaot/Runtime/forward_declarations.h +++ b/src/coreclr/nativeaot/Runtime/forward_declarations.h @@ -25,7 +25,6 @@ FWD_DECL(PageEntry) FWD_DECL(PAL_EnterHolder) FWD_DECL(PAL_LeaveHolder) FWD_DECL(SpinLock) -FWD_DECL(RCOBJECTHANDLEHolder) FWD_DECL(RuntimeInstance) FWD_DECL(StackFrameIterator) FWD_DECL(SyncClean) diff --git a/src/coreclr/vm/eventtrace_gcheap.cpp b/src/coreclr/vm/eventtrace_gcheap.cpp index 9801a76e26ae2f..d8b395941218f8 100644 --- a/src/coreclr/vm/eventtrace_gcheap.cpp +++ b/src/coreclr/vm/eventtrace_gcheap.cpp @@ -61,14 +61,10 @@ BOOL ETW::GCLog::ShouldTrackMovementForEtw() BOOL ETW::GCLog::ShouldWalkStaticsAndCOMForEtw() { LIMITED_METHOD_CONTRACT; -#ifdef FEATURE_COMINTEROP return s_forcedGCInProgress && ETW_TRACING_CATEGORY_ENABLED(MICROSOFT_WINDOWS_DOTNETRUNTIME_PROVIDER_DOTNET_Context, TRACE_LEVEL_INFORMATION, CLR_GCHEAPDUMP_KEYWORD); -#else - return FALSE; -#endif // FEATURE_COMINTEROP } // Batches the list of moved/surviving references for the GCBulkMovedObjectRanges / @@ -510,7 +506,6 @@ HRESULT ETW::GCLog::ForceGCForDiagnostics() //--------------------------------------------------------------------------------------- VOID ETW::GCLog::WalkStaticsAndCOMForETW() { -#ifdef FEATURE_COMINTEROP CONTRACTL { NOTHROW; @@ -522,9 +517,11 @@ VOID ETW::GCLog::WalkStaticsAndCOMForETW() { BulkTypeEventLogger typeLogger; +#ifdef FEATURE_COMINTEROP // Walk RCWs/CCWs BulkComLogger comLogger(&typeLogger); comLogger.LogAllComObjects(); +#endif // FEATURE_COMINTEROP // Walk static variables BulkStaticsLogger staticLogger(&typeLogger); @@ -532,7 +529,9 @@ VOID ETW::GCLog::WalkStaticsAndCOMForETW() // Ensure all loggers have written all events, fire type logger last to batch events // (FireBulkComEvent or FireBulkStaticsEvent may queue up additional types). +#ifdef FEATURE_COMINTEROP comLogger.FireBulkComEvent(); +#endif // FEATURE_COMINTEROP staticLogger.FireBulkStaticsEvent(); typeLogger.FireBulkTypeEvent(); } @@ -540,7 +539,6 @@ VOID ETW::GCLog::WalkStaticsAndCOMForETW() { } EX_END_CATCH -#endif // FEATURE_COMINTEROP }