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/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/inc/holder.h b/src/coreclr/inc/holder.h index bd6b3de64fdfce..df1d7b365c05f5 100644 --- a/src/coreclr/inc/holder.h +++ b/src/coreclr/inc/holder.h @@ -618,54 +618,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 -// 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) -{ -} - -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 +// Holder/Wrapper are the simplest way to define holders - they synthesize a base class from // function pointers //----------------------------------------------------------------------------- @@ -1145,7 +1099,24 @@ 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 + { + STATIC_CONTRACT_LEAF; + _ASSERTE(m_value != NULL); + return m_value; + } void Free() { diff --git a/src/coreclr/nativeaot/Runtime/forward_declarations.h b/src/coreclr/nativeaot/Runtime/forward_declarations.h index f821945f886d7b..c228266b6c2804 100644 --- a/src/coreclr/nativeaot/Runtime/forward_declarations.h +++ b/src/coreclr/nativeaot/Runtime/forward_declarations.h @@ -21,12 +21,10 @@ 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) FWD_DECL(SpinLock) -FWD_DECL(RCOBJECTHANDLEHolder) FWD_DECL(RuntimeInstance) FWD_DECL(StackFrameIterator) FWD_DECL(SyncClean) 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..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, pILImage.GetValue()); + 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, pILImage.GetValue()); + 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 eec10de149a933..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, pILImage.GetValue()); + THROW_BAD_FORMAT(BFA_BAD_IL, static_cast(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 a33ffa588586ad..cb08ac75c78d95 100644 --- a/src/coreclr/vm/ceeload.cpp +++ b/src/coreclr/vm/ceeload.cpp @@ -623,7 +623,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() @@ -3824,7 +3824,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/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/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.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..97311c4ade04f6 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,37 +63,16 @@ 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) -{ - 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/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/debugdebugger.cpp b/src/coreclr/vm/debugdebugger.cpp index 0343b2307e43ef..9b11ac63280df3 100644 --- a/src/coreclr/vm/debugdebugger.cpp +++ b/src/coreclr/vm/debugdebugger.cpp @@ -846,8 +846,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 @@ -868,7 +879,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/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/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; 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/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..d8b395941218f8 100644 --- a/src/coreclr/vm/eventtrace_gcheap.cpp +++ b/src/coreclr/vm/eventtrace_gcheap.cpp @@ -61,7 +61,6 @@ BOOL ETW::GCLog::ShouldTrackMovementForEtw() BOOL ETW::GCLog::ShouldWalkStaticsAndCOMForEtw() { LIMITED_METHOD_CONTRACT; - return s_forcedGCInProgress && ETW_TRACING_CATEGORY_ENABLED(MICROSOFT_WINDOWS_DOTNETRUNTIME_PROVIDER_DOTNET_Context, TRACE_LEVEL_INFORMATION, @@ -505,7 +504,6 @@ HRESULT ETW::GCLog::ForceGCForDiagnostics() //--------------------------------------------------------------------------------------- // WalkStaticsAndCOMForETW walks both CCW/RCW objects and static variables. //--------------------------------------------------------------------------------------- - VOID ETW::GCLog::WalkStaticsAndCOMForETW() { CONTRACTL @@ -519,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); @@ -529,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(); } 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. 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/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/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 8041129865a390..fea5d904ff2407 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/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/olevariant.cpp b/src/coreclr/vm/olevariant.cpp index 26fe57e76d8be7..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(); } // @@ -3586,7 +3571,7 @@ void OleVariant::MarshalSafeArrayForArrayRef(BASEARRAYREF *pArrayRef, else { { - PinningHandleHolder handle = GetAppDomain()->CreatePinningHandle((OBJECTREF)Array); + PinningHandleHolder handle(GetAppDomain()->CreatePinningHandle((OBJECTREF)Array)); if (bArrayOfInterfaceWrappers) { @@ -3684,7 +3669,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); } @@ -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/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(); 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); 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/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); 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: