Skip to content

[release/9.0-staging] [mono] Missing memory barrier leads to crash in multi-threaded scenarios #113740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 20, 2025

Backport of #113140 to release/9.0-staging

/cc @kg

Customer Impact

  • Customer reported
  • Found internally

In specific multithreaded scenarios, a null pointer is dereferenced by the runtime due to an initialization race condition, causing a crash. A customer observed this on Android. This does not expose uninitialized memory, just zeroed memory that hasn't been filled in yet.

Regression

  • Yes
  • No

47c09fa (.NET 7)

Testing

We are relying on CI and customer validation for this as they are embedding an application with Unreal Engine. It is unclear why the race condition was overlooked originally.

Risk

Low risk. This adds asserts for deterministic early failure (especially on targets where it is legal to dereference null, like wasm) and adds an additional memory barrier in a specific place, so it should not be possible for it to cause an undesirable behavior regression.

The memory barrier could theoretically regress performance, but this is initialization code that runs rarely so the impact should be basically nil.

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

lgtm. please get a code review. we will take for consideration in 9.0.x

@jeffschwMSFT jeffschwMSFT added this to the 9.0.x milestone Mar 25, 2025
@rbhanda rbhanda modified the milestones: 9.0.x, 9.0.5 Mar 25, 2025
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 25, 2025
@steveisok steveisok requested review from lateralusX and BrzVlad March 25, 2025 18:18
Co-authored-by: Aleksey Kliger (λgeek) <alklig@microsoft.com>
@kg kg requested a review from BrzVlad March 26, 2025 16:34
@steveisok steveisok added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 31, 2025
@steveisok steveisok merged commit b41d940 into release/9.0-staging Mar 31, 2025
75 of 80 checks passed
@steveisok steveisok deleted the backport/pr-113140-to-release/9.0-staging branch March 31, 2025 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-VM-meta-mono Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants