Skip to content

Add acquire barrier to populate bit reading#117523

Merged
AaronRobinsonMSFT merged 1 commit into
dotnet:mainfrom
AaronRobinsonMSFT:runtime_112565
Jul 11, 2025
Merged

Add acquire barrier to populate bit reading#117523
AaronRobinsonMSFT merged 1 commit into
dotnet:mainfrom
AaronRobinsonMSFT:runtime_112565

Conversation

@AaronRobinsonMSFT

@AaronRobinsonMSFT AaronRobinsonMSFT commented Jul 10, 2025

Copy link
Copy Markdown
Member

The setting of the NDirectMethodDesc::kNDirectPopulated bit is done via an interlocked operation. This bit is used to indicate if the NDirectMethodDesc has been populated with the needed information to load the P/Invoke. The reading of this bit however was missing an acquire barrier and thus setting of fields between the check and set weren't being observed correctly.

Fixes #112565

This should be considered to backport to .NET 9.0.

The setting of the NDirectMethodDesc::kNDirectPopulated bit
is done via an interlocked operation. This bit is used to indicate
if the NDirectMethodDesc has been populated with the needed
information to load the P/Invoke. The reading of this bit however
was missing an acquire barrier and thus setting of fields between
the check and set weren't being observed correctly.

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.

Pull Request Overview

Adds an acquire barrier to the IsPopulated check so that any writes performed before setting the kNDirectPopulated bit are correctly observed by readers.

  • Updated IsPopulated to use VolatileLoad when reading m_wFlags

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

@jkoritzinsky jkoritzinsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This must have been an absolute pain to debug and find. LGTM!

@elinor-fung

Copy link
Copy Markdown
Member

Definitely not for this change, but should we have something similar for kDefaultDllImportSearchPathsIsCached (indication of populating another field on NDirectMethodDesc)?

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2025
@AaronRobinsonMSFT

Copy link
Copy Markdown
Member Author

/backport to release/9.0-staging

@github-actions github-actions Bot unlocked this conversation Sep 22, 2025
@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/17923699227

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Segfault in minipal_resolve_dllimport

5 participants