Skip to content

Fix concurrent publication of interface GUID cache entries - #134360

Merged
AaronRobinsonMSFT merged 4 commits into
mainfrom
copilot/fix-concurrent-guid-caching
Sep 26, 2026
Merged

AaronRobinsonMSFT merged 4 commits into
mainfrom
copilot/fix-concurrent-guid-caching

Conversation

Copilot AI commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

CoreCLR publishes cached interface GUIDs without ordering payload initialization before concurrent reads. After global ComWrappers marshalling registration on Windows, a reader could pass an incompletely initialized IID to QueryInterface, causing a spurious marshalling exception; this is source-analysis-backed, not dynamically reproduced.

  • Publication ordering: Use VolatileStore in SetGuidInfo and VolatileLoad in live-process GetGuidInfo, ensuring both the GUID and generation classification are visible before consumption.
  • Preserved semantics: Retain the plain DAC read, field layout, single reader snapshot, and existing concurrent-writer allocation/lifetime behavior. No locks, CAS, or reclamation changes.

@azure-pipelines

azure-pipelines Bot commented Sep 21, 2026 •

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 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.

@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.

Co-authored-by: AaronRobinsonMSFT <30635565+AaronRobinsonMSFT@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix sporadic ComWrappers marshalling failures due to GUID caching Fix concurrent publication of interface GUID cache entries Sep 21, 2026
@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

@copilot Please verify in the surrounding code all readers go through GetGuidInfo() and don't directly access m_pGuidInfo. Correct them if they do.

Copilot AI commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please verify in the surrounding code all readers go through GetGuidInfo() and don't directly access m_pGuidInfo. Correct them if they do.

Verified all tracked-file references to m_pGuidInfo, plus source references to GuidInfo/PTR_GuidInfo. No reader bypasses GetGuidInfo():

  • /home/runner/work/runtime/runtime/src/coreclr/vm/methodtable.cpp:6152: DAC reader calls GetGuidInfo().
  • /home/runner/work/runtime/runtime/src/coreclr/vm/methodtable.cpp:6169: live reader calls GetGuidInfo() once, then consumes both payload fields through that local snapshot.
  • /home/runner/work/runtime/runtime/src/coreclr/vm/methodtable.cpp:6303: the sole publisher calls SetGuidInfo() after initializing both fields.

Direct m_pGuidInfo accesses occur only inside the accessors: the intentional plain DAC read, the live VolatileLoad, and VolatileStore. No additional changes are needed. No builds/tests were rerun for this read-only verification.

Comment thread src/coreclr/vm/class.h Outdated
@AaronRobinsonMSFT
AaronRobinsonMSFT marked this pull request as ready for review September 23, 2026 18:16
Copilot AI lite review requested due to automatic review settings September 23, 2026 18:16
@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

/cc @dotnet/interop-contrib

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Add regression coverage for cold concurrent GUID publication and marshalling.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Fixes concurrent publication of CoreCLR interface GUID cache entries using acquire/release ordering.

Changes:

  • Adds VolatileLoad for live readers.
  • Adds VolatileStore for publishers.
  • Preserves DAC behavior and cache ownership semantics.
File Description
src/​coreclr/​vm/​class.h Adds ordered GUID cache publication and consumption.

Comment thread src/coreclr/vm/class.h
@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

@copilot Please investigate the failures on loongarch.

Copilot AI commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Please investigate the failures on loongarch.

Investigated. The loongarch64 failure is unrelated to this PR: build https://dev.azure.com/dnceng-public/cbb18261-c48f-4abb-8651-8cdcb5474649/_build/results?buildId=1609481&view=logs&jobId=b6c7d770-02f1-5c1e-1770-09d092567592 fails during "Build product" while linking the cross-compiled libclrjit_win_x86_loongarch64.so (a JIT component that doesn't include vm/methodtable.cpp or vm/class.h):

ld.lld: error: address assignment did not converge
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

This is an ld.lld linker-convergence bug in the loongarch64 cross-toolchain, not something this PR's VolatileLoad/VolatileStore change to GetGuidInfo/SetGuidInfo could trigger — the JIT binary being linked doesn't reference that code at all. I searched open and recently-closed Known Build Error issues and PRs for this signature ("address assignment did not converge", CoreCLR_Bootstrapped, loongarch64) and found no existing tracker or fix. I don't have a tool available in this session to file a new GitHub issue, so I can't create the KBE myself — flagging here so it can be filed/retried through the usual CI infra process. No code changes are needed in this PR for this failure.

@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

/ba-g Unrelated failure.

@AaronRobinsonMSFT
AaronRobinsonMSFT merged commit e19a215 into main Sep 26, 2026
109 of 111 checks passed
@AaronRobinsonMSFT
AaronRobinsonMSFT deleted the copilot/fix-concurrent-guid-caching branch September 26, 2026 02:18
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.

Concurrent interface GUID caching can cause sporadic ComWrappers marshalling failures

4 participants