Skip to content

Use LifetimeHolder for COM interop holders#126522

Merged
AaronRobinsonMSFT merged 10 commits intodotnet:mainfrom
AaronRobinsonMSFT:clrconfig-lifetimeholder-cleanup-main
Apr 9, 2026
Merged

Use LifetimeHolder for COM interop holders#126522
AaronRobinsonMSFT merged 10 commits intodotnet:mainfrom
AaronRobinsonMSFT:clrconfig-lifetimeholder-cleanup-main

Conversation

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Apr 3, 2026

Summary

This PR continues the CoreCLR holder cleanup by migrating remaining Wrapper<...>-based RAII types to the newer holder abstractions.

  • convert CLRConfig string buffers, CRITSEC allocations, loader/resource helpers, GC/object handles, and COM interop temporaries to LifetimeHolder, ReleaseHolder, or SpecializedWrapper
  • replace bespoke holder classes and no-op helpers with traits-based aliases, trimming boilerplate in holder.h, wrappers.h, and the affected VM headers
  • update transfer-of-ownership sites to use Detach() consistently and remove a few additional unused methods/helpers surfaced by the refactoring
  • keep the refactoring behavior-preserving while tightening the debug check for taking the address of a non-empty LifetimeHolder

Testing

  • validated locally before opening the PR

Note

This PR description was generated with GitHub Copilot.

AaronRobinsonMSFT and others added 5 commits April 3, 2026 10:39
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors CoreCLR VM/interop resource management to use LifetimeHolder/ReleaseHolder/SpecializedWrapper consistently, with ownership transfers standardized via Detach() and redundant holder helpers removed.

Changes:

  • Replaced multiple COM/interop-specific Wrapper-based holder types with LifetimeHolder/ReleaseHolder/SpecializedWrapper aliases (and traits-based frees).
  • Standardized escaping ownership transfers by switching call sites from Extract()/SuppressRelease() patterns to Detach().
  • Simplified/removed legacy “do-nothing” helper functions and inline holder wrappers that were only supporting the older Wrapper patterns.

Reviewed changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/coreclr/vm/wrappers.h Replaces VARIANT/SAFEARRAY holder wrappers with LifetimeHolder trait-based holders.
src/coreclr/vm/syncblk.cpp Uses new OBJECTHANDLEHolder construction and Detach() for handle ownership transfer.
src/coreclr/vm/runtimecallablewrapper.h Replaces NewRCWHolder class with SpecializedWrapper alias; removes old CtxEntryHolder wrapper.
src/coreclr/vm/runtimecallablewrapper.cpp Introduces local CtxEntryHolder alias via ReleaseHolder and updates usage style.
src/coreclr/vm/profilinghelper.cpp Updates CLRConfigStringHolder initialization to default construction.
src/coreclr/vm/peimage.inl Switches PEImageHolder ownership transfer from Extract() to Detach().
src/coreclr/vm/peimage.h Converts PEImageHolder to a LifetimeHolder with explicit traits.
src/coreclr/vm/peimage.cpp Switches PEImageHolder returns to Detach().
src/coreclr/vm/olevariant.cpp Converts VARIANT/SAFEARRAY holders to LifetimeHolder and updates ownership suppression to Detach().
src/coreclr/vm/nativeimage.cpp Updates PEImageHolder initialization style.
src/coreclr/vm/jitinterface.cpp Updates OBJECTHANDLEHolder handle escaping to use Detach().
src/coreclr/vm/interoputil.cpp Converts DispParamHolder and VARIANT holder usage to LifetimeHolder patterns.
src/coreclr/vm/gchandleutilities.h Replaces OBJECTHANDLEHolder/PinningHandleHolder wrapper typedefs with LifetimeHolder traits.
src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h Updates CLRConfigStringHolder access pattern to work with LifetimeHolder.
src/coreclr/vm/eventing/eventpipe/ds-rt-coreclr.h Updates CLRConfigStringHolder access pattern to work with LifetimeHolder.
src/coreclr/vm/eetoprofinterfaceimpl.cpp Uses Detach() for CRITSEC cookie ownership transfer.
src/coreclr/vm/dispparammarshaler.cpp Updates SAFEARRAY holder initialization and escape to Detach().
src/coreclr/vm/dispatchinfo.cpp Updates SAFEARRAY holder usage to rely on implicit conversions rather than .GetValue().
src/coreclr/vm/debugdebugger.cpp Introduces StrongHandleHolder as a LifetimeHolder with traits.
src/coreclr/vm/coreassemblyspec.cpp Updates PEImageHolder initialization and return to Detach().
src/coreclr/vm/cominterfacemarshaler.cpp Updates NewRCWHolder usage to new alias and construction style.
src/coreclr/vm/comconnectionpoints.h Moves cookie handle ownership into OBJECTHANDLEHolder member and switches cookie holder to NewHolder.
src/coreclr/vm/comconnectionpoints.cpp Updates ownership transfer to cookie via move and adjusts handle access with the new holder member.
src/coreclr/vm/comcallablewrapper.h Replaces CCW-related wrapper holders with ReleaseHolder aliases.
src/coreclr/vm/comcallablewrapper.cpp Converts connection point list holder to LifetimeHolder traits and uses Detach() on successful CAS transfer.
src/coreclr/vm/ceemain.cpp Updates CLRConfigStringHolder initialization to explicit construction with config return.
src/coreclr/vm/ceeload.h Converts module holders to LifetimeHolder traits-based types.
src/coreclr/vm/ceeload.cpp Switches module holder escape to Detach().
src/coreclr/vm/assemblyspec.cpp Updates THROW_BAD_FORMAT argument for new holder usage pattern.
src/coreclr/vm/assemblynative.cpp Updates THROW_BAD_FORMAT arguments for new holder usage pattern.
src/coreclr/vm/appdomain.hpp Converts FileLoadLock::Holder to a LifetimeHolder traits-based holder.
src/coreclr/vm/appdomain.cpp Removes legacy holder helper and updates call sites to new holder conversions and Free().
src/coreclr/nativeaot/Runtime/forward_declarations.h Removes forward declaration for OBJECTHANDLEHolder (no longer a class type).
src/coreclr/inc/holder.h Removes legacy “DoNothing” helpers tied to deleted wrappers; enhances LifetimeHolder (operator& assertion + operator->).
src/coreclr/inc/clrhost.h Converts CRITSEC_AllocationHolder to a LifetimeHolder traits-based type.
src/coreclr/inc/clrconfig.h Converts CLRConfigStringHolder to a LifetimeHolder traits-based type.
src/coreclr/debug/ee/debugger.cpp Updates CLRConfigStringHolder initialization style.

Define RCW/CCW events under FEATURE_COMINTEROP.
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review April 6, 2026 18:11
Copilot AI review requested due to automatic review settings April 6, 2026 18:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/coreclr/nativeaot/Runtime/forward_declarations.h:29

  • RCOBJECTHANDLEHolder appears to have been removed from the VM (it no longer exists under src/coreclr/vm/gchandleutilities.h), but it’s still forward-declared here. If it’s truly unused, consider removing this stale forward declaration as well to avoid confusion and keep the forward-decl list accurate.
FWD_DECL(Module)
FWD_DECL(Object)
FWD_DECL(PageEntry)
FWD_DECL(PAL_EnterHolder)
FWD_DECL(PAL_LeaveHolder)
FWD_DECL(SpinLock)
FWD_DECL(RCOBJECTHANDLEHolder)
FWD_DECL(RuntimeInstance)

Copilot AI review requested due to automatic review settings April 7, 2026 20:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.

@AaronRobinsonMSFT AaronRobinsonMSFT enabled auto-merge (squash) April 8, 2026 23:14
@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 2ae217c into dotnet:main Apr 9, 2026
107 of 111 checks passed
@github-project-automation github-project-automation bot moved this to Done in AppModel Apr 9, 2026
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the clrconfig-lifetimeholder-cleanup-main branch April 9, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants