From 353126eaac3d848ef42ac3d375ef8b2b747cfc43 Mon Sep 17 00:00:00 2001 From: Xavier Klausener Date: Fri, 23 Oct 2026 15:49:36 +0200 Subject: [PATCH] fix: stop the WaitFor timer when the first check passes before it is started When a WaitFor* check passes synchronously, the helper disposes itself from inside the constructor, while the timer field is still null. The constructor then looks at WaitTask.IsCompleted to decide whether to start the timer, but WaitTask is a WhenAny(...).Unwrap() that completes on the thread pool, so it usually still reads as not completed at that point and the timer is started anyway. The later Dispose() from the using block returns early because the helper is already disposed, and nothing ever stops the timer. The timer's state is the helper, so until the timeout elapses it roots the helper, the rendered component, the renderer and everything the renderer holds. With bUnit's default timeout of one second that goes unnoticed. With a long DefaultWaitTimeout, a test suite keeps every page it rendered in memory for its whole run. The constructor now also checks isDisposed before starting the timer, and disposes the timer right after creating it if the check disposed the helper in between. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 1 + .../WaitForHelpers/WaitForHelper.cs | 12 ++++- ...enderedComponentWaitForHelperExtensions.cs | 50 +++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 482ab78d6..82adb1477 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to **bUnit** will be documented in this file. The project ad ### Fixed - `Find` threw `TypeLoadException` with AngleSharp 1.8.6 because `HtmlSelectElementWrapper` did not implement the new `SelectedIndex` setter. Reported by [@poxet](https://github.com/poxet) in #1957. Fixed by [@linkdotnet](https://github.com/linkdotnet). +- A `WaitFor*` call whose first check passed left its timeout timer running, which kept the helper, the renderer and everything the renderer holds alive until the timeout elapsed. With a long `DefaultWaitTimeout`, a test suite could hold every rendered component in memory for its whole run. Fixed by [@xaviergxf](https://github.com/xaviergxf). ## [2.12.5] - 2026-10-19 diff --git a/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs b/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs index 9f3c01ced..99790d681 100644 --- a/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs +++ b/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs @@ -95,7 +95,11 @@ protected WaitForHelper( // If the initial check did not complete successfully, // start the timer and recheck after every render until the timer expires. - if (!WaitTask.IsCompleted) + // A check that completed synchronously has already disposed this helper + // before WaitTask, which completes on the thread pool, reflects it. A timer + // started after that would never be disposed, and would keep this helper, + // and through it the renderer, alive until the timeout elapsed. + if (!isDisposed && !WaitTask.IsCompleted) { timer = new Timer( static (state) => @@ -114,6 +118,12 @@ protected WaitForHelper( this, GetRuntimeTimeout(timeout), Timeout.InfiniteTimeSpan); + + // The check may have completed on the renderer's dispatcher, and disposed + // this helper while the timer field was still null, between the test above + // and the assignment. Dispose(bool) found no timer to stop in that case. + if (isDisposed) + timer.Dispose(); } } diff --git a/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs b/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs index 555a61cd5..34482aff9 100644 --- a/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs +++ b/tests/bunit.tests/Extensions/WaitForHelpers/RenderedComponentWaitForHelperExtensions.cs @@ -1,3 +1,4 @@ +using System.Runtime.CompilerServices; using AngleSharp.Dom; namespace Bunit.Extensions.WaitForHelpers; @@ -295,6 +296,55 @@ public void Test502() expected.Message.ShouldNotContain(WaitForFailedException.CheckNotCompletedMessage); } + [Fact(DisplayName = "WaitForAssertion does not keep the renderer alive after the first check passes")] + public async Task Test503() + { + // The first call is slow while it is being JIT compiled, which is enough for the wait task + // to complete before the helper decides whether to start its timer, so it is not measured. + RenderAndWaitForAssertionThatPassesAtOnce(); + + var renderers = Enumerable.Range(0, 10) + .Select(_ => RenderAndWaitForAssertionThatPassesAtOnce()) + .ToList(); + + // Disposing a context queues work that still references the renderer: cancelling its + // unhandled exception task wakes the helper's failure continuation on the thread pool, and + // the renderer's own base disposal goes through the dispatcher. On a busy test host that + // can still be queued when the first collection runs, so the collection is retried for a + // short while. A timer left running roots the renderer for ten minutes, so no number of + // retries makes the leaking case pass. + var alive = renderers.Count; + for (var attempt = 0; attempt < 20 && alive > 0; attempt++) + { + if (attempt > 0) + await Task.Delay(50, Xunit.TestContext.Current.CancellationToken); + +#pragma warning disable S1215 // A full collection is what proves the renderers are no longer rooted. + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); +#pragma warning restore S1215 + + alive = renderers.Count(r => r.IsAlive); + } + + alive.ShouldBe(0, "a wait whose first check passed should leave no timer running"); + } + + // Kept out of the test method so that nothing on its stack frame keeps the context alive. + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference RenderAndWaitForAssertionThatPassesAtOnce() + { + using var context = new BunitContext(); + var cut = context.Render(); + + // A timeout far longer than the test: a timer left running roots the helper, + // and through it the renderer, until it fires. + cut.WaitForAssertion(() => { }, TimeSpan.FromMinutes(10)); + + return new WeakReference(context.Renderer); + } + private sealed class ThrowsAfterAsyncOperation : ComponentBase { protected override async Task OnInitializedAsync()