Skip to content

Fix a series of possible race conditions in thread static variable initialization#127843

Merged
davidwrighton merged 7 commits into
dotnet:mainfrom
davidwrighton:fix_thread_statics_race_conditions
May 13, 2026
Merged

Fix a series of possible race conditions in thread static variable initialization#127843
davidwrighton merged 7 commits into
dotnet:mainfrom
davidwrighton:fix_thread_statics_race_conditions

Conversation

@davidwrighton
Copy link
Copy Markdown
Member

  • The two critical ones are the one just after the memcpy in the TLSIndexToMethodTableMap, as well as the VolatileStore when setting pIndex.

Fixes #127776

…itialization

- The two critical ones are the one just after the memcpy in the TLSIndexToMethodTableMap, as well as the VolatileStore when setting pIndex.

Fixes dotnet#127776
@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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR targets a set of suspected race conditions in CoreCLR thread-static infrastructure by strengthening publication/visibility guarantees when updating TLS index metadata and the TLSIndex→MethodTable map. This aligns with the reported intermittent macOS arm64 crash in GetThreadLocalStaticBase by ensuring readers can’t observe partially-published state during concurrent thread-static access.

Changes:

  • Use VolatileStore when publishing a resized TLSIndexToMethodTableMap (pMap and m_maxIndex) to ensure the copied/initialized map contents are visible before other threads observe the updated pointers/sizes.
  • Use VolatileStore when writing/clearing individual map entries to prevent reordering and improve cross-thread visibility of per-index updates.
  • Use VolatileStore when publishing the newly allocated TLSIndex (*pIndex) so other threads that observe the allocated index have the intended ordering/visibility for preceding writes on the allocation path.

Copilot AI review requested due to automatic review settings May 7, 2026 21:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/coreclr/vm/threadstatics.h Outdated
Comment thread src/coreclr/vm/threadstatics.cpp
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 7, 2026 23:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread src/coreclr/vm/threadstatics.h
Comment thread src/coreclr/vm/threadstatics.cpp
@davidwrighton davidwrighton requested review from VSadov and jkotas May 11, 2026 16:35
@jkotas
Copy link
Copy Markdown
Member

jkotas commented May 11, 2026

Do we need VolatileLoads to pair with these VolatileWrites? Or are there some hidden invariants that make VolatileLoads unnecessary in these cases?

Comment thread src/coreclr/vm/threadstatics.cpp Outdated
Comment thread src/coreclr/vm/threadstatics.cpp Outdated
Comment thread src/coreclr/vm/threadstatics.cpp Outdated
Comment thread src/coreclr/vm/threadstatics.cpp
Copy link
Copy Markdown
Member

@VSadov VSadov left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks.

Copilot AI review requested due to automatic review settings May 12, 2026 00:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comment thread src/coreclr/vm/threadstatics.h Outdated
Comment thread src/coreclr/vm/threadstatics.cpp
Comment thread src/coreclr/vm/threadstatics.cpp
@davidwrighton
Copy link
Copy Markdown
Member Author

/ba-g machine timeouts unrelated to this change.

@davidwrighton davidwrighton merged commit c3be9c8 into dotnet:main May 13, 2026
105 of 110 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Intermittent SIGSEGV in GetThreadLocalStaticBase on macOS arm64

4 participants