fix(concurrency): drain queue by design before shutdown signals worker exit#469
Merged
Conversation
✅ Dependency Audit
See the Security tab for detailed findings. Workflow: Dependency Audit |
Security Scan Results
Recommendations
Workflow: Security Scanning |
…worker exit The shutdown() method previously set shutdown_requested_ immediately, relying on the workers' exit-condition check (queue.empty()) to drain remaining items — correct but incidental. This commit makes the drain guarantee explicit and by-design: * Add drain_cv_ condition variable to ThreadPool. * Workers notify drain_cv_ under the queue lock immediately after dequeueing the last item. * shutdown() waits on drain_cv_ until the queue is empty before issuing the final condition_.notify_all() that causes workers to exit. * Add GracefulShutdownDrainsQueueExplicitly test that submits a burst of 20 tasks and asserts all complete after shutdown(), with no sleep(). Closes #322. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
drain_cv_condition variable toThreadPoolsoshutdown()explicitly waits for the work queue to empty before waking workers to exit.drain_cv_(under the queue lock) immediately after dequeueing the last item, making the drain observable without any polling.GracefulShutdownDrainsQueueExplicitlytest that submits 20 tasks then callsshutdown()immediately — asserts all 20 complete with nosleep_forreliance.Root cause (issue #322): The previous
shutdown()setshutdown_requested_first and relied on the worker exit-condition check (work_queue_.empty()) to drain remaining items. This was correct but incidental — if the exit condition were ever changed, the drain guarantee would silently vanish. The fix promotes the guarantee to an explicit, observable drain phase.Test plan
GracefulShutdownDrainsQueueExplicitlypasses (new test)GracefulShutdowncontinues to pass (existing test)DestructorShutdowncontinues to pass (existing test)NoWorkAfterShutdowncontinues to pass (existing test)just format-checkCloses #322.
🤖 Generated with Claude Code