Fix flake in multi-thread handoff test#517
Open
Jawnnypoo wants to merge 1 commit into
Open
Conversation
The test submitted work to two single-thread executors back-to-back and
assumed thread-a would always win the `SynchronizedUpdateQueue` takeOver
CAS. There was no synchronization forcing that ordering, so on ~10-30%
of runs thread-b's executor started up first, took over the queue, and
processed all three events itself — producing:
expected: [(50, thread-a), (10, thread-a), (10, thread-b)]
but was : [(10, thread-b), (50, thread-b), (10, thread-b)]
Add an `awaitTakeOver` option to `MultiThreadRobot.thread()` that blocks
until the named thread has actually claimed the queue. Detection uses an
Inspector — `onStateChanged` fires synchronously inside the listener,
which itself runs inside `takeOver`, so observing the state change with
`pendingEvent.threadName == name` proves that thread owns `threadRunning`.
Once thread-a holds the queue, thread-b's submission is guaranteed to be
enqueued behind it, restoring the deterministic ordering the assertion
expects.
Verified by running the test 20 consecutive times — all pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
JaCoCo Code Coverage 93.83% ✅
Generated by 🚫 Danger |
📊 Benchmark Comparison ReportSummary
🎉 Performance Improvements
No significant changes (3 benchmarks)
Regressions: ±10% with non-overlapping confidence intervals. Improvements: ±10% change only. Generated by 🚫 Danger |
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.
Summary
The test
formula multi-thread handoff to executing threadwas flaky (~10-30% failure rate). It submitted work to two single-thread executors back-to-back and assumedthread-awould always win theSynchronizedUpdateQueuetakeOver CAS — but there was no synchronization forcing that ordering, so sometimesthread-bwon the race instead:This adds an
awaitTakeOveroption toMultiThreadRobot.thread()that blocks until the named thread has actually claimed the queue. Detection uses anInspector—onStateChangedfires synchronously inside the listener, which itself runs insidetakeOver, so observing the state change withpendingEvent.threadName == nameproves that thread ownsthreadRunning. Oncethread-aholds the queue,thread-b's submission is guaranteed to be enqueued behind it.No production code changed — only the test helper and the one assertion that depended on the racy ordering.
Test plan
MultiThreadRobot-based tests (formula multi-threaded events fired at the same time,formula multi-threaded input after termination) — still pass🤖 Generated with Claude Code