Skip to content

Refactor loader heap backout adapter to remove multiple inheritance#125867

Open
noahfalk wants to merge 1 commit intodotnet:mainfrom
noahfalk:loaderheap-backout-adapter
Open

Refactor loader heap backout adapter to remove multiple inheritance#125867
noahfalk wants to merge 1 commit intodotnet:mainfrom
noahfalk:loaderheap-backout-adapter

Conversation

@noahfalk
Copy link
Member

A more complete example of what I was talking about in #125129 (comment). We can either merge this PR or close it and incorporate similar changes in 125129

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 20, 2026 21:49
@dotnet-policy-service
Copy link
Contributor

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

Copy link
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 LoaderHeap backout handling to eliminate UnlockedLoaderHeapBase’s multiple inheritance from ILoaderHeapBackout, replacing it with a small composed adapter object that forwards backout calls to the owning heap.

Changes:

  • Remove ILoaderHeapBackout as a base class of UnlockedLoaderHeapBase and introduce a nested LoaderHeapBackout adapter member.
  • Route TaggedMemAllocPtr::m_pHeap assignments through GetBackoutInterface() rather than assigning this.
  • Ensure derived heaps’ RealBackoutMem methods are explicitly marked override to match the new virtual in UnlockedLoaderHeapBase.

Reviewed changes

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

File Description
src/coreclr/utilcode/loaderheap_shared.cpp Initializes the new backout adapter member in UnlockedLoaderHeapBase’s constructor.
src/coreclr/inc/loaderheap.h Replaces multiple inheritance with composition for backout, adds GetBackoutInterface(), and updates allocation tracking to store the adapter interface.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants