Skip to content

Use LifetimeHolder for COM interop holders#126522

Draft
AaronRobinsonMSFT wants to merge 6 commits intodotnet:mainfrom
AaronRobinsonMSFT:clrconfig-lifetimeholder-cleanup-main
Draft

Use LifetimeHolder for COM interop holders#126522
AaronRobinsonMSFT wants to merge 6 commits intodotnet:mainfrom
AaronRobinsonMSFT:clrconfig-lifetimeholder-cleanup-main

Conversation

@AaronRobinsonMSFT
Copy link
Copy Markdown
Member

Summary

  • replace COM interop-specific Wrapper holders with LifetimeHolder, ReleaseHolder, and SpecializedWrapper aliases
  • update ownership transfers to use Detach() consistently where values escape
  • remove redundant do-nothing helpers from the affected CoreCLR interop code

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.

// 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);
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Avoid the C-style cast here. PEImageHolder is implicitly convertible to PEImage* (and now also supports operator->), so passing pILImage directly (or using static_cast<PEImage*>(pILImage)) is clearer and avoids hiding unintended conversions.

Suggested change
THROW_BAD_FORMAT(BFA_BAD_IL, (PEImage*)pILImage);
THROW_BAD_FORMAT(BFA_BAD_IL, pILImage);

Copilot uses AI. Check for mistakes.
Comment on lines 197 to 206
// 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);
}
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

Avoid the C-style cast here. PEImageHolder is implicitly convertible to PEImage* (and supports operator->), so prefer passing pILImage directly (or static_cast<PEImage*>(pILImage)) to keep the intent explicit and avoid C-style cast pitfalls.

Copilot uses AI. Check for mistakes.


//-----------------------------------------------------------------------------
// Holder/Wrapper are the simplest way to define holders - they synthesizes a base class out of
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

This comment still has a couple of grammatical issues after the edit (e.g., missing space after the period and “im” -> “I'm”). Consider cleaning it up while touching this line to avoid leaving a partially-corrected comment.

Suggested change
// 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

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants