Skip to content

Investigate flaky WaitFor* timeouts on CI - #1952

Open
linkdotnet wants to merge 1 commit into
mainfrom
fix/waitfor-threadpool-starvation
Open

linkdotnet wants to merge 1 commit into
mainfrom
fix/waitfor-threadpool-starvation

Conversation

@linkdotnet

Copy link
Copy Markdown
Collaborator

Diagnostics for sporadic WaitFor* timeouts (e.g. runs 39277565989, 39276248124). Logs thread pool stats and actual vs configured elapsed time when a waiter times out.

@linkdotnet
linkdotnet force-pushed the fix/waitfor-threadpool-starvation branch from 1527b0e to c288c39 Compare October 19, 2026 21:12
@linkdotnet
linkdotnet marked this pull request as ready for review October 19, 2026 21:21
Copilot AI balanced review requested due to automatic review settings October 19, 2026 21:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Timer-initialization failures can leave callers waiting indefinitely, and the scheduling changes need regression coverage.

2 open issues
What changed in this PR

Adds diagnostics and scheduling changes to investigate flaky bUnit WaitFor* timeouts on CI.

Changes:

  • Logs elapsed time and thread-pool statistics on timeout.
  • Defers timer startup and coordinates it with disposal.
  • Raises test thread-pool minimums and initializes element-count state earlier.
File Description
tests/​bunit.tests/​Assembly.cs Raises minimum worker threads for tests.
src/​bunit/​Extensions/​WaitForHelpers/​WaitForHelperLoggerExtensions.cs Expands timeout diagnostics.
src/​bunit/​Extensions/​WaitForHelpers/​WaitForHelper.cs Moves timer startup into dispatcher initialization and synchronizes disposal.
src/​bunit/​Extensions/​WaitForHelpers/​WaitForElementsHelper.cs Uses a primary constructor to capture element count before base initialization.

Review effort: Balanced

Comment thread src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs Outdated
Comment on lines +153 to +156
lock (checkPassedCompletionSource)
{
if (isDisposed || WaitTask.IsCompleted)
return;
@linkdotnet
linkdotnet force-pushed the fix/waitfor-threadpool-starvation branch from c288c39 to 99e4806 Compare October 20, 2026 05:37
@linkdotnet
linkdotnet requested a balanced review from Copilot October 20, 2026 06:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cancellation can leave the new regression test’s dispatcher blocked and potentially hang teardown.

2 open issues
1 resolved since last review

Review effort: Balanced

Comment on lines +254 to +259
dispatcherEntered.Wait(Xunit.TestContext.Current.CancellationToken);

var waitTask = cut.WaitForElementAsync("#notHere", TimeSpan.FromMilliseconds(5));
await Task.Delay(50, Xunit.TestContext.Current.CancellationToken);
releaseDispatcher.Set();
await blocker;
- Start the WaitFor timeout timer only after the initial check ran on the
  renderer dispatcher; previously a busy dispatcher let the timer expire
  before any check executed.
- Capture matchElementCount in WaitForElementsHelper before the base
  constructor starts the timer, which reads TimeoutErrorMessage.
- Raise the thread pool floor in Bunit.Tests: parallel tests blocking pool
  threads starved the continuations they waited on.
- Log configured vs actual elapsed time and thread pool load on timeout.
@linkdotnet
linkdotnet force-pushed the fix/waitfor-threadpool-starvation branch from 99e4806 to 5c6510a Compare October 20, 2026 06:26

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants