Fix use-after-free during consensus shutdown
#5759
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
Signed-off-by: Michal Maslanka <[email protected]>
mmaslankaprv
requested review from
ztlpn,
jcsp,
VadimPlh and
rystsov
as code owners
jcsp
reviewed
Aug 1, 2022
| return _snapshot_writer->close().then( | ||
| [this] { _snapshot_writer.reset(); }); | ||
| }); | ||
| co_await _event_manager.stop(); |
mmaslankaprv
added a commit
to mmaslankaprv/redpanda
that referenced
this pull request
Aug 1, 2022
When raft stops the last step in stop sequence is closing gate. After gate is closed all the background futures are guaranteed to be finished. However it may happen that the fiber is waiting for an `_op_lock` while gate is being closed. We need to make sure that when gate is closed no other fiber can acquire the `_op_lock`. Marking `_op_lock` mutex as broken prevents all waiters for acquiring a mutex and at the same time accessing `consensus` state. Fixes: redpanda-data#5759 Signed-off-by: Michal Maslanka <[email protected]>
jcsp
previously approved these changes
Aug 1, 2022
When raft stops the last step in stop sequence is closing gate. After gate is closed all the background futures are guaranteed to be finished. However it may happen that the fiber is waiting for an `_op_lock` while gate is being closed. We need to make sure that when gate is closed no other fiber can acquire the `_op_lock`. Marking `_op_lock` mutex as broken prevents all waiters for acquiring a mutex and at the same time accessing `consensus` state. Fixes: redpanda-data#5759 Signed-off-by: Michal Maslanka <[email protected]>
Signed-off-by: Michal Maslanka <[email protected]>
jcsp
approved these changes
Aug 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Cover letter
When raft stops the last step in stop sequence is closing gate. After
gate is closed all the background futures are guaranteed to be finished.
However it may happen that the fiber is waiting for an
_op_lockwhilegate is being closed. We need to make sure that when the
_op_lockisacquired the
consensusinstance is still alive.Clearing and marking an oplock as broken after dispatching
gateclose makes it impossible for incoming fibers to access
consensusafter it is deleted.When raft stops the last step in stop sequence is closing gate. After
gate is closed all the background futures are guaranteed to be finished.
However it may happen that the fiber is waiting for an
_op_lockwhilegate is being closed. We need to make sure that when the
_op_lockisacquired the
consensusinstance is still alive.Clearing and marking an oplock as broken after dispatching
gateclose makes it impossible for incoming fibers to access
consensusafter it is deleted.
Fixes #5754
Backport Required
UX changes
Describe in plain language how this PR affects an end-user. What topic flags, configuration flags, command line flags, deprecation policies etc are added/changed.
Release notes