fix(tests): deterministically gate the DoHealthChecksAsync reentrancy-guard test#3331
Merged
Merged
Conversation
…-guard test reentrancy_guard_prevents_concurrent_DoHealthChecksAsync failed in full-suite net10.0 runs with 'expected 1 call, actually received 2' on TryAttainLeadershipLockAsync, while passing in isolation and on net9.0. The guard itself was never broken. The test parked the winning call inside the guard with 'await Task.Yield()' in the mocked lock acquisition, assuming the other nine competing calls would launch before the winner resumed. That is a timing gamble: the yielded continuation lands on an xUnit sync-context worker and can finish the entire mocked election path — releasing the guard — before the test thread starts the second call, which then legitimately re-enters the guard and takes the lock a second time. A warm thread pool plus net10.0 scheduling made that window reliable in full-suite runs. Park the winner on TaskCompletionSources instead (signal 'entered', await 'release') so the guard is provably held while the competitors run, assert they bounce off with AgentCommands.Empty, then release the winner and additionally verify the finally block frees the guard for a subsequent health check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 9, 2026
Merged
This was referenced Jul 14, 2026
Open
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.
What
CoreTests.Runtime.Agents.leader_election_self_visibility_tests.reentrancy_guard_prevents_concurrent_DoHealthChecksAsyncfailed in full-suite net10.0 runs with:while passing in isolation and in full-suite net9.0 runs.
Diagnosis
The reentrancy guard in
NodeAgentController.DoHealthChecksAsync(from #2999) was never broken — the race was in the test's synchronization. The test parked the winning call inside the guard withawait Task.Yield()in the mockedTryAttainLeadershipLockAsync, assuming the other nine competing calls would launch before the winner resumed.Task.Yield()gives no such guarantee: the yielded continuation lands on an xUnit sync-context worker thread and can finish the entire mocked election path — releasing the guard — before the test thread even starts the second call. That second call then legitimately re-enters the guard and acquires the lock a second time. A warm thread pool plus net10.0 scheduling made the window reliable in full-suite runs; cold isolated runs and net9.0 timing happened to stay on the lucky side.Fix (test only)
Park the winner on
TaskCompletionSources instead — the mock signalsenteredand awaitsrelease— so the guard is provably held while the nine competitors run. The test now also asserts:AgentCommands.Emptysingleton and never reachTryAttainLeadershipLockAsyncfinallyblock frees the guard — a subsequent health check goes all the way through againVerification
🤖 Generated with Claude Code