Uh oh!
There was an error while loading. Please reload this page.
Deduplicate "all managed object wrappers" mapping - #133260
Merged
Merged
Conversation
Prevent duplicate entries in the "all managed object wrappers" table. Fixes#129191 For the case in #129191, there is always only one ComWrappers instance, so this collection will be length 1. I chose to do a simple fix like this for the following reasons: 1. This collection is read from the DAC and cDAC directly to support the `!DumpObj` command. Any significant changes to the structure (such as moving to a `HashSet<ManagedObjectWrapperHolder>`) would require very large changes to enable enumerating the structure from DAC or cDAC code, creating a higher risk change for backporting. 2. In practice, objects are passed to unmanaged code via 1 ComWrappers instance in the vast majority of cases outside of our own tests. In the unlikely case it's passed via more than 1, it would be via 2, the CsWinRT ComWrappers instance and the StrategyBasedComWrappers default instance. 3. I considered adding a parallel HashSet for deduplication. However, that felt like a very large memory cost for the common scenario (.NET object passed to native via only one ComWrappers instance) for a minor savings in a very unlikely scenario (many ComWrappers for the same .NET object).
Contributor
Tagging subscribers to this area: @dotnet/area-system-runtime |
|
Azure Pipelines: Successfully started running 2 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The change is small, localized to diagnostics registration, and avoids unbounded list growth while keeping the existing DAC/cDAC-observable structure intact.
Review tier: Lite
Findings: None
What changed in this PR
Prevents duplicate ManagedObjectWrapperHolder entries from being recorded in the global “all managed object wrappers” ConditionalWeakTable, which is used by diagnostics tooling to enumerate managed object wrappers.
Changes:
- Adds a guarded insert (
Containscheck) when registering a managed object wrapper for diagnostics, preventing repeated additions of the sameManagedObjectWrapperHolderto the per-object list.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.cs | Deduplicates per-object wrapper registration in the diagnostics tracking table to avoid list growth from repeated registrations. |
Uh oh!
There was an error while loading. Please reload this page.
jkoritzinsky
commented
Sep 5, 2026
MemberAuthor
/backport to release/10.0 |
jkoritzinsky
commented
Sep 5, 2026
MemberAuthor
/backport to release/11.0 |
Contributor
Started backporting to |
Contributor
Started backporting to |
This was referenced Sep 5, 2026
AaronRobinsonMSFT
approved these changes
Sep 5, 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 freeto 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.
Prevent duplicate entries in the "all managed object wrappers" table.
Fixes#129191
For the case in #129191, there is always only one ComWrappers instance, so this collection will be length 1.
I chose to do a simple fix like this for the following reasons:
!DumpObjcommand. Any significant changes to the structure (such as moving to aHashSet<ManagedObjectWrapperHolder>) would require very large changes to enable enumerating the structure from DAC or cDAC code, creating a higher risk change for backporting.