Repository navigation
De-flake DispatcherModelSpec: await conditions instead of blocking on fixed deadlines - #8609
Merged
Aaronontheweb merged 2 commits intoSep 23, 2026
Conversation
…blocking on fixed deadlines Three tests in `Akka.Tests.Actor.Dispatch.DispatcherModelSpec` failed together on the Windows unit-test lane of akkadotnet#8601 (ADO build 131736) within ~20s of each other, while Linux passed: A_dispatcher_must_process_messages_in_parallel "Failed to count down within 3000 milliseconds. Should process other actors in parallel" A_dispatcher_must_dynamically_handle_its_own_lifecycle "System.Exception : Await failed" A_dispatcher_must_continue_to_process_messages_when_exception_is_thrown "Assert.True() Failure", on one of the Assert.True(fN.Wait(...)) lines akkadotnet#8601 is not implicated. It touches Settings.cs, the ActorSystemImpl scheduler site, AkkaFeatures.cs, TypeExtensions.cs, an AOT canary app and src/xunitSettings.props -- none of them on a dispatcher or test-scheduling path, and it does not touch this file (the PR merge commit's copy of ActorModelSpec.cs is byte-identical to dev's). Its xunitSettings.props change re-anchors the xunit.runner.json copy from $(SolutionDir) to $(MSBuildThisFileDirectory); build-system/azure-pipeline.template.yaml compiles with `dotnet build -c Release` from the repo root -- a solution build, so $(SolutionDir) is set -- and then runs the tests with --no-build, so the file was already reaching bin/ on dev. Confirmed locally: a project-path build leaves no xunit.runner.json in bin, a -p:SolutionDir=... build puts it there. Root cause: all three are the same failure mode. `my-test-dispatcher` runs on `executor = default-executor`, i.e. the shared .NET ThreadPool (MessageDispatcherConfigurator.ConfigureExecutor -> ThreadPoolExecutorServiceFactory -> FullThreadPoolExecutorServiceImpl -> ThreadPool.UnsafeQueueUserWorkItem). Under xUnit v3 with collection parallelism off there is no sync context, so the test body itself runs on a ThreadPool worker -- the same pool, whose floor is 2 on a 2-vCPU hosted agent. Every wait in this harness blocked that worker: * AssertCountdown called CountdownEvent.Wait(ms). In A_dispatcher_must_process_messages_in_parallel, once aStart has passed the test thread is parked in bParallel.Wait while actor `a` is parked in the Meet handler's WaitFor.Wait() -- that is the whole pool floor. b.Tell was enqueued from the test thread with preferLocal: true, so b's mailbox run sits in the blocked thread's local queue and needs a third worker to steal it. CountdownEvent.Wait blocks inside ManualResetEventSlim, which does NOT trip the pool's blocking compensation, so the only source of a third thread is the starvation heuristic (~1 per 500ms, and slower still when the CPU is saturated). * Await(until, condition), shared by AssertDispatcher and AssertRef, was a SpinWait hot loop. It pinned its own thread AND burned a core, which on 2 vCPUs competes with the very workers it waits for and pushes the injection heuristic toward its slow rate. AssertDispatcher's deadline was ShutdownTimeout*5 (5s); AssertRef ran seven successive Awaits against one shared 1s budget, so the counter that settled last got only the leftovers. * A_dispatcher_must_continue_to_process_messages_when_exception_is_thrown used the synchronous EventFilter...Expect, which blocks through Nito's WaitAndUnwrapException, plus four Assert.True(fN.Wait(GetTimeoutOrDefault(null))) -- a raw, NON-dilated 3s. And the cascade: A_dispatcher_must_process_messages_in_parallel signalled aStop only AFTER the assertion that failed. With the latch never released, actor `a` stayed in the unbounded meet.WaitFor.Wait() for good, the ActorSystem could not terminate (TestKit's 5s Shutdown cap and then force-shutdown, matching the 5s dispose in the log), and that ThreadPool worker was gone for the rest of the test host's life. That is why the other two failed seconds later: they are victims, not independent flakes. Fix, in the style of akkadotnet#8299. No timeout was widened, nothing was skipped, and no warning was suppressed: * try/finally around the latch-gated sections of A_dispatcher_must_process_messages_in_parallel and A_dispatcher_must_process_messages_one_at_a_time, so a failed assertion always releases the gate. * Meet now carries a MaxWait, so its handler's wait is bounded (dilated 10s) and no failing test can pin a worker for the life of the process. * AssertCountdown/AssertNoCountdown became AssertCountdownAsync/AssertNoCountdownAsync, which poll latch.IsSet through AwaitConditionNoThrowAsync -- Task.Delay between checks, so the thread goes back to the pool. Assertion messages are unchanged. * Await's SpinWait loop is gone. AssertDispatcherAsync and AssertRefAsync use AwaitAssertAsync, which polls with Task.Delay and dilates its own timeout. Same nominal bounds as before: ShutdownTimeout*5 and 1s. AssertRefAsync checks all seven counters in one polling assertion instead of draining a shared budget serially. * A_dispatcher_must_dynamically_handle_its_own_lifecycle awaits a.WatchAsync() before asserting the dispatcher's stop count. Sys.Stop is fire-and-forget and only the Terminate mailbox run needs a pool worker; the dispatcher's idle shutdown then fires on the HashedWheelTimer's own thread. Waiting for termination first keeps "the actor never stopped" distinguishable from "the dispatcher never shut down". * A_dispatcher_must_continue_to_process_messages_when_exception_is_thrown awaits ExpectAsync(2, async () => ...) and reads each ask with (await fN.WaitAsync(RemainingOrDefault)) -- the dilated single-expect default -- instead of Task.Wait. * All 8 tests in ActorModelSpec/DispatcherModelSpec are async Task now, and the last two sync TestKit calls (AwaitCondition in A_dispatcher_must_not_double_deregister and in AwaitStarted) are awaited. * Dropped the AwaitLatch/Wait/WaitAck messages and their handlers. Nothing has constructed them since akkadotnet#8299 replaced Wait(1000) with Meet, and they held the last unbounded Wait() and the last Thread.Sleep calls in the harness -- the remaining route back to this bug. akka.test.timefactor is 1.0 on CI, so "dilated" is a no-op today. The point of routing every wait through Dilated/AwaitAssertAsync is that the knob becomes effective for the Windows lane if we ever need to stretch it. Test-only change. No production code under src/core/Akka/Dispatch/ was modified; the dispatcher's behavior is correct.
…InProgress in the hand-over remark The <remarks> added by akkadotnet#8600 on AwaitHandOverConfirmationAsync explains the "at least one, not exactly one" expectation solely through HandOverRetryTimer. That is not the path CI actually hit, and attributing it to the timer invites someone to tighten the expectation back to an exact count once the timer looks harmless. Build 131691's log has ZERO "Retry [n], sending HandOverToMe" lines, and the two "Hand-over in progress at" lines on the new oldest carry the same millisecond on the same thread. What happened is a timer-less cross-fire: * the new oldest sends its first HandOverToMe on OldestChanged (ClusterSingletonManager.cs:916); * independently the previous oldest sends TakeOverFromMe (:801), re-sent once a second from WasOldest (:1237-1244); * the new oldest, still in BecomingOldest, answers that TakeOverFromMe with a SECOND HandOverToMe (:1061); * the previous oldest, now in HandingOver, answers every HandOverToMe it sees with HandOverInProgress (:1304-1308). So the new oldest logs the line once per HandOverToMe it sent, with no timer involved, whenever the remote TakeOverFromMe round trip beats the local PoisonPill/Terminated one. The HandOverRetryTimer is a real second path (armed at :1420, re-sends at :1080-1083, drawing another HandOverInProgress from the same :1308 case), but the first confirmation cancels it (:983) -- which is exactly why it leaves no trace in a log that already has the line. Both paths are designed behavior and both produce N >= 1 lines, so "at least one" remains the only correct expectation. The remark now names both. Comment-only change; no test logic touched.
Aaronontheweb
changed the title
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.
Three
DispatcherModelSpectests flaked together on the.NET Unit Tests (Windows)lane of #8601 and dragged each other down in one 20s window.The root cause is a single cascade, not three independent flakes.
A_dispatcher_must_process_messages_in_parallelonly released its latch after the assertion that failed, so actorastayed pinned in an unbounded wait. It held the only pool worker left on the 2-vCPU agent, the ActorSystem could not terminate, and the other two tests failed seconds later waiting for a thread that never returned.Changes
try/finallyin both parallel-send tests, so a failed assertion always frees the gate.Meet's handler wait with a dilatedMaxWait(10s), so no failing test can pin a worker for the process lifetime.CountdownEvent.WaitandSpinWaitpolling withAssertCountdownAsync/AwaitAssertAsync, which release the thread back to the pool betweenTask.Delaychecks. Nominal bounds are unchanged (5s shutdown, 1s counter budget).WatchAsyncbefore asserting the dispatcher's stop count in the lifecycle test, so "actor never stopped" stays distinguishable from "dispatcher never shut down".AwaitLatch/Wait/WaitAckmessages and handlers that carried the last unboundedWait()andThread.Sleepcalls.<remarks>inClusterSingletonRestartSpec: theHandOverInProgressre-send is hit as a timer-less cross-fire between the old and new oldest, not only viaHandOverRetryTimer. Both paths are intended, both yield N >= 1, so "at least one" stays the only correct expectation.No timeout was widened, no test skipped or deleted, no warning suppressed. With
akka.test.timefactorat 1.0 on CI the dilation is a no-op today, but it makes the Windows lane reproducible if the knob ever needs stretching.Verified locally on Linux (
net10.0,-c Release,taskset -c 0,1) soProcessorCount == 2matches the hosted agent: 20/20 green on a quiet box and 20/20 green with 4 competingdotnet testruns hammering both CPUs.BREAKING_CHANGES_V1.6.mdentry needed