This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Fix BufferBlock race condition between completion and consumption #7246
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
A failed test in CI highlighted a rare race condition in the BufferBlock implementation. When a bounded buffer block has room become available and postponed messages waiting to be consumed, it’ll queue a task to consume those messages from the relevant sources. It should stop consuming messages after it’s been marked as complete, but if the consuming task is already running when the buffer is marked as completed, it may end up still consuming messages due to missing a check for having been marked as completed. The fix is simply to add in that check before dequeueing the next postponed message. Prior to the fix, I ran the test 1,000,000 times and was able to repro this in 22 of them. After the fix, I could no longer repro it. (There is still an even more rare but expected race that can occur if the block is in the process of consuming the item from one of its sources and the block being marked as completed, but that's no different than a race between it being marked as completed and another thread adding to the block and is the nature of using these components concurrently.)
|
LGTM |
picenka21
pushed a commit
to picenka21/runtime
that referenced
this pull request
Feb 18, 2022
Fix BufferBlock race condition between completion and consumption Commit migrated from dotnet/corefx@fc38379
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.
A failed test in CI highlighted a rare race condition in the BufferBlock implementation. When a bounded buffer block has postponed messages waiting to be consumed and room becomes available, it’ll queue a task to consume those messages from the relevant sources. It should stop consuming messages after it’s been marked as complete, but if the consuming task is already running when the buffer is marked as complete, it may end up still consuming as many messages as have been postponed due to missing a check in that consuming task for having been marked as completed. The fix is simply to add in that check before dequeueing the next postponed message.
Prior to the fix, I ran the test 1,000,000 times and was able to repro this in 22 of them. After the fix, I could no longer repro it.
(There is still an even more rare but expected race that can occur if the block is in the process of consuming the item from one of its sources and the block being marked as completed, but that's no different than a race between it being marked as completed and another thread adding to the block and is the nature of using these components concurrently.)
Fixes https://github.com/dotnet/corefx/issues/7244
cc: @mellinoe, @AlfredoMS