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()