Fix[bmqc]: first clean and block queues, then unset d_started - #1669
Open
678098 wants to merge 1 commit into
Open
Fix[bmqc]: first clean and block queues, then unset d_started#1669678098 wants to merge 1 commit into
678098 wants to merge 1 commit into
Conversation
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
678098
changed the title
chrisbeard
reviewed
Jul 30, 2026
| { | ||
| BSLS_ASSERT_SAFE(isStarted() && "MQTP has not been started"); | ||
|
|
||
| d_started = false; |
Contributor
There was a problem hiding this comment.
It feels like this is a logical bug because we use d_started to indicate the started and stopping states. If the stopping state were distinct, we would be able to test that condition and avoid asserting?
Collaborator
Author
There was a problem hiding this comment.
If we have one variable d_state = {stopped, starting, stopping}, it will still assert, unless we change the condition to BSLS_ASSERT(d_state != stopped) everywhere.
If we have a second independent bool flag d_stopped that we set to true during stop() without changing d_started, the assertion will be gone; however, d_stopped flag is useless on its own.
We can achieve the same behavior without introducing a new variable or state.
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
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.
MQTP::stopsetsd_started = falsefirst before stopping each dispatcher queue.This leads to a thread race where dispatcher events being processed in its thread might try to enqueue another event through a dispatcher that is going to close. Since we've set
d_started = false, enqueue attempt fails on assertion.The fix is to set
d_startedflag only after we've disabled push to each dispatcher queue and cleared its contents. During stop, we just drop any events in queues, so disabling push is equivalent to just dropping a dispatcher event, which is fine at shutdown.