diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index bfb6e7112..3825e07db 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -10,14 +10,14 @@ "rollForward": false }, "docfx": { - "version": "2.78.5", + "version": "2.81.0", "commands": [ "docfx" ], "rollForward": false }, "dotnet-dump": { - "version": "10.0.745401", + "version": "10.0.750501", "commands": [ "dotnet-dump" ], diff --git a/CHANGELOG.md b/CHANGELOG.md index e732dd9da..589d75cd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ All notable changes to **bUnit** will be documented in this file. The project ad ## [Unreleased] +### Fixed + +- `WrapperBase.WrappedElement` could return `null` and throw `NullReferenceException` when an element was accessed concurrently with component re-renders. Reported by [@psu-prog](https://github.com/psu-prog) in #1937. Fixed by [@Nacl-HCl](https://github.com/Nacl-HCl). +- `DisposeComponentsAsync()` could dispose nothing when the renderer's dispatcher was busy, e.g. with a timer calling `InvokeAsync`. Reported by [@xfunc-tim-schneider](https://github.com/xfunc-tim-schneider) in #1939. Fixed by [@linkdotnet](https://github.com/linkdotnet). +- Markup generation and `FindComponents` could recurse until the test host crashed with a stack overflow when a render threw while instantiating a child component. Reported in #1935. Fixed by [@linkdotnet](https://github.com/linkdotnet). +- `Find()` could keep returning DOM nodes from the previous render when a render, e.g. from a timer, happened while the nodes were parsed, causing `UnknownEventHandlerIdException`. Reported by [@xfunc-tim-schneider](https://github.com/xfunc-tim-schneider) in #1938. Fixed by [@linkdotnet](https://github.com/linkdotnet). + ## [2.11.3] - 2026-09-13 ### Fixed diff --git a/Directory.Packages.props b/Directory.Packages.props index 04c706758..8a23376ef 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -8,7 +8,7 @@ - + @@ -18,8 +18,8 @@ - - + + @@ -29,61 +29,61 @@ - + - - + + - - - + + + - + - - - - - - - - - - - - + + + + + + + + + + + + - - - + + + - - - - - - + + + + + + - - - - - - - - - - - - + + + + + + + + + + + + @@ -91,7 +91,7 @@ - + @@ -99,16 +99,16 @@ - - - - - + + + + + - - - - + + + + @@ -117,5 +117,6 @@ + diff --git a/src/bunit.generators.internal/Web.AngleSharp/WrapperBase.cs b/src/bunit.generators.internal/Web.AngleSharp/WrapperBase.cs index 8dd4d5963..46bcc12f2 100644 --- a/src/bunit.generators.internal/Web.AngleSharp/WrapperBase.cs +++ b/src/bunit.generators.internal/Web.AngleSharp/WrapperBase.cs @@ -24,13 +24,7 @@ public abstract class WrapperBase : IElementWrapper public TElement WrappedElement { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - if (element is null) - element = elementFactory.GetElement(); - - return element; - } + get => element ??= elementFactory.GetElement(); } /// diff --git a/src/bunit/Rendering/BunitRenderer.cs b/src/bunit/Rendering/BunitRenderer.cs index 88740fcbc..46fedfb45 100644 --- a/src/bunit/Rendering/BunitRenderer.cs +++ b/src/bunit/Rendering/BunitRenderer.cs @@ -281,27 +281,21 @@ public IReadOnlyList> FindComponents( public Task DisposeComponents() { ObjectDisposedException.ThrowIf(disposed, this); - Task? returnTask; - lock (renderTreeUpdateLock) + return Dispatcher.InvokeAsync(async () => { - returnTask = Dispatcher.InvokeAsync(async () => - { - ResetUnhandledException(); - - foreach (var root in rootComponents) - { - root.Detach(); - } - - await Task.WhenAll(disposalTasks).ConfigureAwait(false); - disposalTasks.Clear(); - }); + ResetUnhandledException(); + var roots = rootComponents.ToArray(); rootComponents.Clear(); - } - return returnTask; + foreach (var root in roots) + { + root.Detach(); + } + + await Task.WhenAll(disposalTasks.ToArray()).ConfigureAwait(false); + }); } /// @@ -425,21 +419,11 @@ protected override void AddPendingTask(ComponentState? componentState, Task task if (componentState is null) { ArgumentNullException.ThrowIfNull(task); - AddDisposalTaskToQueue(); + disposalTasks.RemoveAll(static t => t.IsCompleted); + disposalTasks.Add(task); } base.AddPendingTask(componentState, task); - - void AddDisposalTaskToQueue() - { - var t = task; - t = task.ContinueWith(_ => - { - disposalTasks.Remove(t); - }, TaskScheduler.Current); - - disposalTasks.Add(t); - } } internal Task SetDirectParametersAsync(IRenderedComponent renderedComponent, ParameterView parameters) @@ -602,7 +586,6 @@ protected override void Dispose(bool disposing) if (disposing) { returnedRenderedComponentIds.Clear(); - disposalTasks.Clear(); unhandledExceptionTsc.TrySetCanceled(); } @@ -678,7 +661,7 @@ void FindComponentsInRenderTree(int componentId) for (var i = 0; i < frames.Count; i++) { ref var frame = ref frames.Array[i]; - if (frame.FrameType == RenderTreeFrameType.Component) + if (frame.FrameType == RenderTreeFrameType.Component && frame.Component is not null) { if (frame.Component is TComponent) { diff --git a/src/bunit/Rendering/Internal/Htmlizer.cs b/src/bunit/Rendering/Internal/Htmlizer.cs index 9f31c4bd0..32bb5768e 100644 --- a/src/bunit/Rendering/Internal/Htmlizer.cs +++ b/src/bunit/Rendering/Internal/Htmlizer.cs @@ -131,6 +131,14 @@ int position ) { var frame = frames.Array[position]; + + // A component frame is left uninstantiated (ComponentId 0) when the render that added it threw mid-diff. + // Following that id would recurse into the first root component, which can be an ancestor. + if (frame.Component is null) + { + return position + frame.ComponentSubtreeLength; + } + var childFrames = context.GetRenderTreeFrames(frame.ComponentId); RenderFrames(context, childFrames, 0, childFrames.Count); return position + frame.ComponentSubtreeLength; diff --git a/src/bunit/Rendering/RenderedComponent.cs b/src/bunit/Rendering/RenderedComponent.cs index afd868453..22e98e1d2 100644 --- a/src/bunit/Rendering/RenderedComponent.cs +++ b/src/bunit/Rendering/RenderedComponent.cs @@ -18,7 +18,7 @@ internal sealed class RenderedComponent : ComponentState, IRenderedC private readonly BunitHtmlParser htmlParser; private string markup = string.Empty; - private INodeList? latestRenderNodes; + private MarkupNodes? latestRenderNodes; /// /// Gets the component under test. @@ -68,7 +68,17 @@ public INodeList Nodes get { EnsureComponentNotDisposed(); - return latestRenderNodes ??= htmlParser.Parse(Markup); + // A render can finish while Markup is parsed; keying on the markup reference stops that stale parse from being cached (#1938). + // Not needed in v3: the shared render tree (#1910) binds nodes to an immutable markup snapshot. + var currentMarkup = Markup; + var cached = latestRenderNodes; + if (cached is null || !ReferenceEquals(cached.Markup, currentMarkup)) + { + cached = new MarkupNodes(currentMarkup, htmlParser.Parse(currentMarkup)); + latestRenderNodes = cached; + } + + return cached.Nodes; } } @@ -169,4 +179,6 @@ public override ValueTask DisposeAsync() Dispose(); return base.DisposeAsync(); } + + private sealed record MarkupNodes(string Markup, INodeList Nodes); } diff --git a/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#WrapperBase.g.verified.cs b/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#WrapperBase.g.verified.cs index 2f8cc5e05..6135001c7 100644 --- a/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#WrapperBase.g.verified.cs +++ b/tests/bunit.generators.tests/Web.AngleSharp/WrapperElementsGeneratorTest.Generator#WrapperBase.g.verified.cs @@ -1,4 +1,4 @@ -//HintName: WrapperBase.g.cs +//HintName: WrapperBase.g.cs #nullable enable using System.CodeDom.Compiler; using System.Diagnostics; @@ -25,13 +25,7 @@ public abstract class WrapperBase : IElementWrapper public TElement WrappedElement { [MethodImpl(MethodImplOptions.AggressiveInlining)] - get - { - if (element is null) - element = elementFactory.GetElement(); - - return element; - } + get => element ??= elementFactory.GetElement(); } /// diff --git a/tests/bunit.generators.tests/bunit.generators.tests.csproj b/tests/bunit.generators.tests/bunit.generators.tests.csproj index 28d889510..96775d3d7 100644 --- a/tests/bunit.generators.tests/bunit.generators.tests.csproj +++ b/tests/bunit.generators.tests/bunit.generators.tests.csproj @@ -2,6 +2,8 @@ net8.0;net9.0;net10.0;net11.0 + OpenSource + 2027-10 Bunit Bunit.Generator.Tests true @@ -21,6 +23,7 @@ + diff --git a/tests/bunit.testassets/.globalconfig b/tests/bunit.testassets/.globalconfig new file mode 100644 index 000000000..0cf91a1c0 --- /dev/null +++ b/tests/bunit.testassets/.globalconfig @@ -0,0 +1,5 @@ +# Razor SDK 11 moves component declarations into *_razor.decl.g.cs, where Sonar flags @implements IDisposable. +# .editorconfig path sections do not match generated files, so this must be global. +is_global = true + +dotnet_diagnostic.S3881.severity = none diff --git a/tests/bunit.tests/BunitContextTest.cs b/tests/bunit.tests/BunitContextTest.cs index cdbc74601..e9863752d 100644 --- a/tests/bunit.tests/BunitContextTest.cs +++ b/tests/bunit.tests/BunitContextTest.cs @@ -52,6 +52,28 @@ public async Task Test104() callStack.Count.ShouldBe(1); } + [Fact(DisplayName = "DisposeComponents disposes components while the dispatcher is busy")] + public async Task Test105() + { + var callStack = new List(); + Render(ps => ps.Add(p => p.CallStack, callStack)); + using var entered = new ManualResetEventSlim(); + using var release = new ManualResetEventSlim(); + var blocker = Task.Run(() => Renderer.Dispatcher.InvokeAsync(() => + { + entered.Set(); + release.Wait(); + }), Xunit.TestContext.Current.CancellationToken); + entered.Wait(Xunit.TestContext.Current.CancellationToken); + + var disposeTask = DisposeComponentsAsync(); + release.Set(); + await disposeTask; + await blocker; + + callStack.Count.ShouldBe(1); + } + [Fact(DisplayName = "The test service provider should register a placeholder HttpClient which throws exceptions")] public void Test024() { diff --git a/tests/bunit.tests/Extensions/RefreshingWrappedElementTest.cs b/tests/bunit.tests/Extensions/RefreshingWrappedElementTest.cs index 4f1602435..e432b470b 100644 --- a/tests/bunit.tests/Extensions/RefreshingWrappedElementTest.cs +++ b/tests/bunit.tests/Extensions/RefreshingWrappedElementTest.cs @@ -76,6 +76,53 @@ public void Test031() Should.Throw(() => btn.TextContent); } + [Fact(DisplayName = "Found element is never null when accessed concurrently with component re-renders")] + public async Task Test040() + { + var cut = Render(); + var elm = cut.Find("span"); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(Xunit.TestContext.Current.CancellationToken); + cts.CancelAfter(TimeSpan.FromSeconds(1)); + var renders = Task.Run(async () => + { + while (!cts.IsCancellationRequested) + { + await cut.InvokeAsync(cut.Instance.Increment); + } + }, Xunit.TestContext.Current.CancellationToken); + + try + { + while (!cts.IsCancellationRequested) + { + elm.TextContent.ShouldNotBeNull(); + } + } + finally + { + await cts.CancelAsync(); + await renders; + } + } + + private sealed class Counter : ComponentBase + { + private int count; + + public void Increment() + { + count++; + StateHasChanged(); + } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + builder.OpenElement(0, "span"); + builder.AddContent(1, count); + builder.CloseElement(); + } + } + private sealed class Markup : ComponentBase { [Parameter] public string Base { get; set; } = string.Empty; diff --git a/tests/bunit.tests/Rendering/BunitRendererTest.cs b/tests/bunit.tests/Rendering/BunitRendererTest.cs index 681140d43..fe0c0356b 100644 --- a/tests/bunit.tests/Rendering/BunitRendererTest.cs +++ b/tests/bunit.tests/Rendering/BunitRendererTest.cs @@ -450,6 +450,27 @@ public void Test211() cut.Find("h1").TextContent.ShouldBe($"Hello world {RenderMode.InteractiveWebAssembly}"); } + [Fact(DisplayName = "Markup generation skips child components whose instantiation threw during a render")] + public async Task Test212() + { + var cut = Render(); + var trigger = cut.FindComponent(); + await cut.Instance.ShowThrowingChild(); + + await trigger.Instance.TriggerWithValue("after"); + + cut.Markup.ShouldBe("after"); + } + + [Fact(DisplayName = "FindComponents skips child components whose instantiation threw during a render")] + public async Task Test213() + { + var cut = Render(); + await cut.Instance.ShowThrowingChild(); + + cut.FindComponents().Count.ShouldBe(1); + } + internal sealed class LifeCycleMethodInvokeCounter : ComponentBase { public int InitilizedCount { get; private set; } @@ -532,6 +553,33 @@ protected override void BuildRenderTree(RenderTreeBuilder builder) } } + internal sealed class ThrowsOnConstruction : ComponentBase + { + public ThrowsOnConstruction() => throw new InvalidOperationException("THROWS ON PURPOSE"); + } + + internal sealed class ShowsThrowingChild : ComponentBase + { + private bool showThrowing; + + public Task ShowThrowingChild() + { + showThrowing = true; + return InvokeAsync(StateHasChanged); + } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + builder.OpenComponent(0); + builder.CloseComponent(); + if (showThrowing) + { + builder.OpenComponent(1); + builder.CloseComponent(); + } + } + } + internal sealed class ToggleChild : ComponentBase { private bool showing = true; diff --git a/tests/bunit.tests/Rendering/RenderedComponentTest.cs b/tests/bunit.tests/Rendering/RenderedComponentTest.cs index bdf04d193..f2ab250f4 100644 --- a/tests/bunit.tests/Rendering/RenderedComponentTest.cs +++ b/tests/bunit.tests/Rendering/RenderedComponentTest.cs @@ -297,6 +297,29 @@ public void Test029() results.ShouldBeEmpty(); } + [Fact(DisplayName = "Nodes reflects the latest markup after a render on another thread (#1938)")] + public void Test030() + { + var cut = Render(ps => ps.AddChildContent("

0

")); + _ = cut.Nodes; + cut.Render(ps => ps.AddChildContent("

1

")); + // Holding the parser's lock pauses the reader after it captured the old markup but before it caches the parsed nodes. + var parserLock = typeof(BunitHtmlParser) + .GetField("parserLock", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(Services.GetRequiredService())!; + var reader = new Thread(() => _ = cut.Nodes); + + lock (parserLock) + { + reader.Start(); + SpinWait.SpinUntil(() => reader.ThreadState.HasFlag(ThreadState.WaitSleepJoin), TimeSpan.FromSeconds(5)).ShouldBeTrue(); + cut.Render(ps => ps.AddChildContent("

2

")); + } + + reader.Join(); + cut.Find("p").TextContent.ShouldBe("2"); + } + private class BaseComponent : ComponentBase { protected override void BuildRenderTree(RenderTreeBuilder builder) diff --git a/version.json b/version.json index 3f50706f4..20c1c480d 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,6 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "2.11", + "version": "2.12", "assemblyVersion": { "precision": "revision" },