Skip to content
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

Fix access to m_pDynamicStaticsInfo #97353

Merged

Conversation

davidwrighton
Copy link
Member

  • Remove race condition where it is possible that an updated dynamic statics info pointer is published without ensuring that the data is also considered written.
  • Use VolatileLoadWithoutBarrier at the read site (where locks are not taken) to ensure that there are no difficult to examine reads of this pointer.

- Remove race condition where it is possible that an updated dynamic statics info pointer is published without ensuring that the data is also considered written.
- Use VolatileLoadWithoutBarrier at the read site (where locks are not taken) to ensure that there are no difficult to examine reads of this pointer.
@jkotas
Copy link
Member

jkotas commented Jan 23, 2024

Did you see a crash caused by this issue?

@davidwrighton
Copy link
Member Author

@jkotas No, no crashes caused by this issue have been observed that I have been able to identify.

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.

Thanks

@davidwrighton
Copy link
Member Author

@jkotas I'm currently trying to figure out if I should push this for backport inside of the #97352 PR or if I should just fix this for .NET 9. I'm currently undecided. It's clearly low risk, but I have no evidence its actually happening in practice. Do you have an opinion?

@jkotas
Copy link
Member

jkotas commented Jan 23, 2024

I would include this fix in the servicing change. The C/C++ compilers can do very surprising reordering.

@davidwrighton davidwrighton merged commit 85e9e7b into dotnet:main Jan 23, 2024
111 checks passed
davidwrighton added a commit that referenced this pull request Jan 23, 2024
- Remove race condition where it is possible that an updated dynamic statics info pointer is published without ensuring that the data is also considered written.
- Use VolatileLoadWithoutBarrier at the read site (where locks are not taken) to ensure that there are no difficult to examine reads of this pointer.
jeffschwMSFT pushed a commit that referenced this pull request Feb 9, 2024
…97352)

* Move a lock to protect m_pDynamicStaticsInfo

* apply feedback

* cast to LONG

* Fix access to m_pDynamicStaticsInfo (#97353)

- Remove race condition where it is possible that an updated dynamic statics info pointer is published without ensuring that the data is also considered written.
- Use VolatileLoadWithoutBarrier at the read site (where locks are not taken) to ensure that there are no difficult to examine reads of this pointer.

---------

Co-authored-by: Hyungju Lee <leee.lee@samsung.com>
Co-authored-by: David Wrighton <davidwr@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Feb 23, 2024
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.

2 participants