Fix timing race condition in AwaitAssertAsync causing premature timeouts#7976
Closed
Conversation
Fixed a check-then-act race condition in TestKit's AwaitAssertAsync that could cause tests to timeout prematurely with only 1 retry attempt instead of the expected number of retries within the timeout window. The bug was introduced in PR akkadotnet#4075 (Dec 2019) when async API was added to TestKit. The timeout check occurred BEFORE Task.Delay(), creating a timing window where thread scheduling delays, GC pauses, or system load could cause the actual elapsed time to exceed the timeout even though the pre-check indicated a retry should occur. Changes: - Move timeout check to AFTER Task.Delay() in both AwaitAssertAsync overloads - Changed boundary condition from >= to > to allow final retry when at exact timeout - Re-run assertion on timeout to propagate the actual exception instead of generic timeout - Added explanatory comments documenting the race condition This fixes flaky test failures in cluster tests where initialization under load would take longer than expected, causing false timeout failures. Fixes StartEntitySpec.StartEntity_while_the_entity_is_waiting_for_restart_should_restart_it_immediately
Aaronontheweb
force-pushed
the
claude-wt-StartEntitySpec.StartEntity_while_the_entity_is_waiting_for_restart_should_restart_it_immediately
branch
from
de11f30 to
5a234b6
Compare
Aaronontheweb
commented
Dec 29, 2025
Member
Author
Aaronontheweb
left a comment
Aaronontheweb
left a comment
There was a problem hiding this comment.
Detailed my changes
| catch(Exception) | ||
| { | ||
| var stopped = Now + t; | ||
| if (stopped >= stop) |
Member
Author
There was a problem hiding this comment.
The problem here is that we don't get 1 last chance to test the assertion if the first attempt failed and took too long
| await Task.Delay(t, cancellationToken); | ||
|
|
||
| // Check if we've exceeded the timeout AFTER sleeping | ||
| if (Now > stop) |
Member
Author
There was a problem hiding this comment.
Now, after the delay has been completed, even if we're overdue on time we still check one final time on exit.
…he_entity_is_waiting_for_restart_should_restart_it_immediately
Arkatufus
requested changes
Dec 29, 2025
Comment on lines
123
to
128
| if (Now > stop) | ||
| { | ||
| Sys.Log.Warning("AwaitAssert failed, timeout [{0}] is over after [{1}] attempts and [{2}] elapsed time", max, attempts, Now - start); | ||
| // Re-run the assertion one final time to get the actual exception | ||
| await assertion(); | ||
| } |
Contributor
There was a problem hiding this comment.
My only problem is that this code logs a fail warning regardless if the extra assertion invocation failed or not.
Member
Author
There was a problem hiding this comment.
Yeah we should remove that
Member
Author
There was a problem hiding this comment.
There were some other errors with this code too, like if the final assertion succeeded we'd end up running again
…he_entity_is_waiting_for_restart_should_restart_it_immediately
…he_entity_is_waiting_for_restart_should_restart_it_immediately
auto-merge was automatically disabled
January 9, 2026 17:23
Pull request was closed
Aaronontheweb
deleted the
claude-wt-StartEntitySpec.StartEntity_while_the_entity_is_waiting_for_restart_should_restart_it_immediately
branch
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
Fixed a 6-year-old check-then-act race condition in TestKit's
AwaitAssertAsyncthat could cause tests to timeout prematurely with only 1 retry attempt instead of the expected number of retries within the timeout window.Problem
The bug was introduced in PR #4075 (Dec 2019) when async API was added to TestKit. The timeout check occurred BEFORE
Task.Delay(), creating a timing window where thread scheduling delays, GC pauses, or system load could cause the actual elapsed time to exceed the timeout even though the pre-check indicated a retry should occur.This manifested as flaky test failures like:
Only 1 attempt in 4.6 seconds instead of ~30 attempts in 3 seconds.
Changes
Task.Delay()in bothAwaitAssertAsyncoverloads>=to>to allow final retry when at exact timeoutImpact
This benefits all tests across the entire Akka.NET suite that use
AwaitAssert(), preventing false timeout failures under load.Testing
Verified that
StartEntitySpec.StartEntity_while_the_entity_is_waiting_for_restart_should_restart_it_immediatelynow passes with the default 3-second timeout (previously required 10 seconds as a workaround).