Fix race condition in shutdownThread start that can cause server to be unable to stop #8380
Conversation
This file contains hidden or 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
Previously we assign value to shutdownSemaphore after shutdownThread is started, where it is already needed. So we can have situation where shutdownThread instantly leaving due to shutdownSemaphore == nullptr, and we are left with a server that can only be stopped with kill -9.
AlexPeshkoff
approved these changes
Jan 10, 2025
Member
|
Should it be backported? |
Contributor
Author
I think so, this bug appears in at least v4 and v5 (haven't looked at v3 and more older versions). |
dyemanov
pushed a commit
that referenced
this pull request
Jan 13, 2025
Previously we assign value to shutdownSemaphore after shutdownThread is started, where it is already needed. So we can have situation where shutdownThread instantly leaving due to shutdownSemaphore == nullptr, and we are left with a server that can only be stopped with kill -9. Co-authored-by: Artyom Ivanov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Previously we assign value to
shutdownSemaphoreaftershutdownThreadis started, where it was already needed. So we can have situation whereshutdownThreadinstantly leaving due toshutdownSemaphore == nullptr, and we are left with a server that can only be stopped with kill -9.I think this problem was hidden behind
fbguardtimeout and it's really hard to reproduce due to nature of race conditions, but in a synthetic environment usingsleepto emulate high load and process switching in OS it is easily reproducible.