[release/7.0] Fix pthread_cond_wait race on macOS #82893
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The native runtime event implementations for nativeaot and GC use pthread_cond_wait to wait for the event and pthread_cond_broadcast to signal that the event was set. While the usage of the pthread_cond_broadcast conforms with the documentation, it turns out that glibc before 2.25 had a race in the implementation that can cause the pthread_cond_broadcast to be unnoticed and the wait waiting forever. It turns out that macOS implementation has the same issue. The fix for the issue is to call pthread_cond_broadcast while the related mutex is taken. This change fixes intermittent crossgen2 hangs with nativeaot build of crossgen2 reported in #81570. I was able to repro the hang locally in tens of thousands of iterations of running crossgen2 without any arguments (the hang occurs when server GC creates threads). With this fix, it ran without problems over the weekend, passing 5.5 million iterations.
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
|
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas Issue DetailsBackport of #82709 to release/7.0 /cc @janvorli Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
jeffschwMSFT
approved these changes
Mar 2, 2023
jkotas
approved these changes
Mar 2, 2023
rbhanda
added
Servicing-approved
|
Approved by Tactics. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #82709 to release/7.0
/cc @janvorli
Customer Impact
Applications compiled with NativeAOT can hang intermittently at startup on macOS. This was occurring with our own crossgen2 in the CI.
The problem is caused by the implementation of
pthread_cond_broadcastnot adhering to the documentation in a race condition case. There is a tiny window of opportunity within which the relatedpthread_cond_waitisn't woken by thepthread_cond_broadcastwhen the latter is not invoked with the related mutex taken.Testing
Stress testing running of crossgen2 compiled with NativeAOT on macOS without any arguments. Without the fix, it hanged in tens or hundreds of thousands of iterations. With the fix, it was running ok for 5.5 million of iterations.
Risk
Very low, the change just moves
pthread_cond_broadcastinside of a mutex and the doc for that method says it should not matter whether it is called inside of the mutex or not.