diff --git a/CHANGELOG.md b/CHANGELOG.md index f289f6066..6c303723e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to **bUnit** will be documented in this file. The project ad ## [Unreleased] +### Fixed + +- 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 ### Fixed diff --git a/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs b/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs index d8e5111e1..d99de823b 100644 --- a/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs +++ b/src/bunit/Extensions/WaitForHelpers/WaitForHelper.cs @@ -94,7 +94,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) => @@ -112,6 +116,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 0d0044ea4..d0e50bc6b 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; @@ -240,6 +241,55 @@ await cut.WaitForAssertionAsync(async () => expected.Message.ShouldStartWith(WaitForAssertionHelper.TimeoutMessage); } + [Fact(DisplayName = "WaitForAssertion does not keep the renderer alive after the first check passes")] + public async Task Test500() + { + // 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()