[#695] Fix race in TraditionalWorkQueue.isIdle()#688
Open
vharseko wants to merge 1 commit into
Open
Conversation
isIdle() checked that the operation queue is empty and that no worker thread is active, but a worker is invisible to both checks in the window between taking an operation from the queue and assigning it to its operation field. waitUntilIdle() could therefore return immediately while the just-submitted operation was still being handed off to a worker, which made TraditionalWorkQueueTestCase.testWaitUntilIdleSlowOpInProgress flaky. Track the number of accepted-but-not-fully-processed operations in a dedicated counter: incremented before an operation is enqueued (and rolled back if it is rejected), decremented when a worker thread is done with it, and adjusted for operations cancelled while the queue is being resized. isIdle() now simply checks that this counter is zero. Move the four waitUntilIdle tests out of the "slow" group into the default build.
maximthomas
approved these changes
Jul 5, 2026
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
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.
Fixes #695
Problem
TraditionalWorkQueue.isIdle()checked that the operation queue is empty and that no worker thread is active (operation != null). A worker thread is invisible to both checks in the window between taking an operation from the queue (opQueue.poll()insideretryNextOperation) and assigning it to itsoperationfield inTraditionalWorkerThread.run().waitUntilIdle()pollsisIdle()every millisecond, so it could return immediately while a just-submitted operation was still being handed off to a worker.This made
TraditionalWorkQueueTestCase.testWaitUntilIdleSlowOpInProgress(from theslowgroup, excluded from CI) flaky: the test submits an operation delayed by 5 seconds and expectswaitUntilIdle(10000)to block for at least 4 seconds, but it occasionally returned right away.Fix
Track the number of accepted-but-not-fully-processed operations in a dedicated
AtomicInteger:submitOperation()before the operation is enqueued, and rolled back if the operation is rejected (queue full, shutdown, interrupt);operationDone(), called from atry/finallyaround the processing block inTraditionalWorkerThread(covers the normal path, the transaction paths and exceptions);initializeWorkQueue().isIdle()now simply checks that this counter is zero, covering queued operations, in-flight operations and the hand-off window in between.The four
waitUntilIdle*tests are moved out of theslowgroup into the default build (~16s for the class).Verification
TraditionalWorkQueueTestCaserun 5 times in a row under the default CI group filter: 6/6 green each time.