unblockClient: avoid to reset client when the client was shutdown-blocked #10440
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
Member
|
@warriorguo thank you for the report and the fix.
p.s. if you have a fix and you submit a PR, there's no need to also open an issue. |
Contributor
Author
|
@oranagra I did the tests with the unfixed version and notice the test cases truly can not cause crashes, I made some improvements on the test cases then.
and thanks for the info, thought it could be a better practice to track the issue at first lol |
18cb68f to
5c8e826
Compare
oranagra
approved these changes
Mar 20, 2022
Merged
sundb
added a commit
that referenced
this pull request
Oct 15, 2025
…ancellation (#14420) This issue was introduced by #10440 In that PR, we avoided resetting the current user during processCommand, but overlooked the fact that this client might not be the current one, it could be a client that was previously blocked due to shutdown. If we don’t reset these clients, and the shutdown is canceled, then when these clients continue executing other commands, they will trigger an assertion. This PR delays the operation of resetting the client to processUnblockedClients and no longer skips SHUTDOWN_BLOCKED clients.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
fix #10439. see #9872
When executing SHUTDOWN we pause the client so we can un-pause it if the shutdown fails.
this could happen during the timeout, if the shutdown is aborted, but could also happen from withing the initial
call()to shutdown, if the rdb save fails.in that case when we return to
call(), we'll crash ifc->cmdhas been set to NULL.The call stack is:
what's special about SHUTDOWN in that respect is that it can be paused, and then un-paused before the original
call()returns.tests where added for both failed shutdown, and a followup successful one.