Port more coreclr holders to the LifetimeHolder<Traits> pattern#128103
Open
AaronRobinsonMSFT wants to merge 7 commits into
Open
Port more coreclr holders to the LifetimeHolder<Traits> pattern#128103AaronRobinsonMSFT wants to merge 7 commits into
AaronRobinsonMSFT wants to merge 7 commits into
Conversation
Replaces the legacy Wrapper<>-based HModuleHolder typedef with a
HModuleTraits struct used via LifetimeHolder<>, matching the pattern
already used by HKEYHolder, BSTRHolder, and MapViewHolder.
Call sites updated to use the LifetimeHolder API:
- Assign(v) -> operator=(v)
- Clear() -> Free()
- Extract() / SuppressRelease()+GetValue() -> Detach()
- Copy-init 'HModuleHolder h = expr;' -> direct-init 'HModuleHolder h{ expr };'
(LifetimeHolder's value constructor is explicit)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy SpecializedWrapper-based ResetPointerHolder with a ResetPointerTraits<T> struct used via LifetimeHolder<>. The two ZeroMem<T> / ZeroMem<T*> partial specializations collapse into a single Free() using 'if constexpr (std::is_pointer<T>::value)'. The local VISIBLE macro and detail::ZeroMem helpers are removed. Behavioral note: legacy Wrapper only invoked the release function when the holder was 'acquired' (constructed non-null). LifetimeHolder invokes Free unconditionally on destruction, so the new Free() guards against a null slot pointer. All existing call sites direct-init from a real address, so no call-site changes are required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy SpecializedWrapper-based CoTaskMemHolder typedef (and its DeleteCoTaskMem free-function helper) with a CoTaskMemTraits<T> struct used via LifetimeHolder<>. The new definition is moved to live alongside the other LifetimeHolder-based aliases. Call sites in src/coreclr/debug/di/module.cpp updated: - holder.GetAddr() -> std::addressof(holder) (legacy GetAddr returned 'this' (pointer-to-holder), used as an out-parameter target. LifetimeHolder's operator& is overloaded to return the inner Type*, so std::addressof is required to obtain the address of the holder itself.) - holder.SuppressRelease() -> holder.Detach() - pLocalBuffer->Assign(p) -> *pLocalBuffer = p Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy SpecializedWrapper-based StubHolder typedef with a
StubTraits<T> struct used via LifetimeHolder<>. The StubRelease<T,LOGGER>
function template is preserved verbatim (still supports the optional
LOGGER template parameter used for executable-allocator statistics)
and is now invoked from the trait's Free().
The new declaration is moved to live alongside the other
LifetimeHolder-based aliases in the file. The ExecutableWriterHolderNoLog
and ExecutableAllocator forward declarations travel with it.
Call sites updated:
- stublink.cpp:
- 'StubHolder<Stub> pStub = Stub::NewStub(...);' -> brace direct-init
(LifetimeHolder's value ctor is explicit)
- pStub.Extract() -> pStub.Detach()
- stubcache.cpp (x2):
- 'pstub.SuppressRelease(); RETURN pstub;' -> 'RETURN pstub.Detach();'
(semantically equivalent: both yield the value to the caller and
skip DecRef at scope exit)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaces the legacy SpecializedWrapper-based ExceptionHolder alias with
an ExceptionTraits struct used via LifetimeHolder<>. Also removes the
dead-code '#else' branch (a hand-rolled class ExceptionHolder that had
been preserved behind '#if 1 / #else / #endif'), and the now-unused
Exception__Delete inline helper.
EX_TRY macro internals updated:
- EX_RETHROW: __pException.SuppressRelease() -> __pException.Detach()
- GET_EXCEPTION(): __pException.GetValue() -> static_cast<Exception*>(__pException)
(both ternary branches must yield Exception*)
- EXTRACT_EXCEPTION: __pException.Extract() -> __pException.Detach()
Behavioral note on EX_RETHROW: Detach() differs from legacy
SuppressRelease() in that it also resets m_value to nullptr. Since
control flow immediately PAL_CPP_RETHROWs and unwinds the scope, the
holder's destructor sees nullptr and short-circuits in Free(). Net
effect is identical: the in-flight exception object survives the rethrow.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
Tagging subscribers to this area: @agocke |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the CoreCLR-internal migration from legacy Holder/Wrapper/SpecializedWrapper-based RAII to the newer LifetimeHolder<Traits> pattern, updating relevant call sites and a few exception-handling macros to use Detach()/Free() semantics.
Changes:
- Introduces
HModuleHolder,ResetPointerHolder<T>,CoTaskMemHolder<T>, andStubHolder<T>asLifetimeHolder<Traits>specializations inholder.h. - Ports multiple call sites to the new API shape (notably switching
Extract()/SuppressRelease()patterns toDetach(), andClear()toFree()). - Converts
ExceptionHolderinex.htoLifetimeHolder<ExceptionTraits>and updatesEX_RETHROW/GET_EXCEPTION/EXTRACT_EXCEPTIONusages accordingly.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/threads.cpp | Transfers exception ownership during thread startup using Detach() instead of suppressing release. |
| src/coreclr/vm/stublink.cpp | Updates StubHolder construction and return path to use explicit construction + Detach(). |
| src/coreclr/vm/stubcache.cpp | Updates stub cache refcount/return flow to return raw stubs via Detach() (no RAII release on return). |
| src/coreclr/vm/eetoprofinterfaceimpl.cpp | Moves HModuleHolder ownership into member field via Detach(). |
| src/coreclr/vm/clrex.h | Adjusts GET_EXCEPTION and EX_RETHROW internals to align with LifetimeHolder behavior. |
| src/coreclr/utilcode/util.cpp | Updates DLL lifetime management to use brace-init for HModuleHolder and Detach() for handoff/leak-on-purpose cases. |
| src/coreclr/inc/holder.h | Removes legacy holder aliases/helpers and adds new LifetimeHolder-based traits/aliases for HMODULE, CoTaskMem, stubs, and reset-pointer semantics. |
| src/coreclr/inc/ex.h | Replaces ExceptionHolder specialized wrapper with LifetimeHolder<ExceptionTraits> and updates helper macros to use Detach(). |
| src/coreclr/debug/di/rspriv.h | Updates metadata-copy helper signature to take a VOID** out-parameter. |
| src/coreclr/debug/di/process.cpp | Updates DAC module holder usage to the new assignment/free APIs (= and Free()). |
| src/coreclr/debug/di/module.cpp | Updates metadata-copy call sites and implementation to fill a LifetimeHolder via its overloaded operator&. |
Copilot's findings
- Files reviewed: 11/11 changed files
- Comments generated: 1
jkotas
approved these changes
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues the migration of legacy
Holder/Wrapper/SpecializedWrapper-based resource holders undersrc/coreclr/to the newerLifetimeHolder<Traits>pattern (matches the shape already established byMapViewHolder,LocalAllocHolder,HKEYHolder,BSTRHolder).Each commit ports one holder, in isolation, with the call-site updates required for the new API.
Commits
HModuleHolder—Wrapper<HMODULE, ...>→LifetimeHolder<HModuleTraits>vm/eetoprofinterfaceimpl.cpp,debug/di/rspriv.h/process.cpp,utilcode/util.cpp)ResetPointerHolder—SpecializedWrapper<_TYPE, detail::ZeroMem<_TYPE>::Invoke>→LifetimeHolder<ResetPointerTraits<T>>ZeroMempartial specializations collapse into a singleFree()usingif constexpr (std::is_pointer<T>::value)VISIBLEmacro anddetail::ZeroMemhelpersCoTaskMemHolder—SpecializedWrapper<_TYPE, DeleteCoTaskMem<_TYPE>>→LifetimeHolder<CoTaskMemTraits<T>>DeleteCoTaskMemhelper removeddebug/di/module.cppStubHolder—SpecializedWrapper<_TYPE, StubRelease<_TYPE>>→LifetimeHolder<StubTraits<T>>vm/stublink.cppandvm/stubcache.cppExceptionHolder—SpecializedWrapper<Exception, Exception__Delete>→LifetimeHolder<ExceptionTraits>#if 1 / #else / #endifscaffold and the hand-rolledclass ExceptionHolderin the dead branchException__Deleteinline helperEX_TRYmacro internals ininc/ex.h(EX_RETHROW,GET_EXCEPTION(),EXTRACT_EXCEPTION()) and related call sites invm/clrex.h/vm/threads.cppRecurring API mapping
For reference, the legacy →
LifetimeHoldermappings used throughout these commits:Assign(v)operator=(v)Clear()Free()Extract()Detach()SuppressRelease() + GetValue()Detach()HolderType x = expr;HolderType x{ expr };(value ctor isexplicit)Note
The contents of this pull request were drafted with the assistance of GitHub Copilot.