From 0160ef3c327c3714284ec45bc40c38d6240e3c09 Mon Sep 17 00:00:00 2001 From: Thimo Buchheister Date: Tue, 11 Aug 2026 11:27:54 +0200 Subject: [PATCH 1/2] fix: guard BunitHtmlParser.documents against concurrent access BunitHtmlParser keeps parsed documents in a plain List. Parse() adds to it, Dispose() enumerates it, and the two can run on different threads: WaitForHelper evaluates its condition on the renderer's dispatcher, while BunitContext.Dispose() runs Services disposal on the test thread. BunitRenderer.Dispose() drops the task returned by Dispatcher.InvokeAsync(() => base.Dispose(disposing)), so renderer disposal is asynchronous and unawaited and the parser can be disposed while a parse is still in flight. The result is an intermittent System.InvalidOperationException: Collection was modified; enumeration operation may not execute. at Bunit.Rendering.BunitHtmlParser.Dispose() thrown during teardown, naming whichever test happened to lose the race. Guard the list with a lock and dispose a snapshot rather than enumerating the live list. Fixes #1892 --- src/bunit/Rendering/BunitHtmlParser.cs | 24 +++++++++- .../Rendering/BunitHtmlParserTest.cs | 48 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/bunit/Rendering/BunitHtmlParser.cs b/src/bunit/Rendering/BunitHtmlParser.cs index fa1e08736..ac533839e 100644 --- a/src/bunit/Rendering/BunitHtmlParser.cs +++ b/src/bunit/Rendering/BunitHtmlParser.cs @@ -23,6 +23,7 @@ internal sealed class BunitHtmlParser : IDisposable private readonly IBrowsingContext context; private readonly HtmlParser htmlParser; private readonly List documents = new(); + private readonly object documentsLock = new(); /// /// Initializes a new instance of the class @@ -151,7 +152,12 @@ private static (IElement? Context, string? MatchedElement) GetParseContextFromTa private async Task GetNewDocumentAsync() { var result = await context.OpenNewAsync().ConfigureAwait(false); - documents.Add(result); + + lock (documentsLock) + { + documents.Add(result); + } + return result; } @@ -159,7 +165,21 @@ private async Task GetNewDocumentAsync() public void Dispose() { context.Dispose(); - foreach (var doc in documents) + + // Parse() can be running on another thread while this executes, e.g. a + // WaitForAssertion/WaitForState condition being evaluated on the + // renderer's dispatcher while the test's BunitContext is being + // disposed. Take a snapshot under the lock instead of enumerating the + // live list, which would otherwise throw "Collection was modified" if a + // parse completed mid-loop. + IDocument[] documentsToDispose; + lock (documentsLock) + { + documentsToDispose = documents.ToArray(); + documents.Clear(); + } + + foreach (var doc in documentsToDispose) { doc.Dispose(); } diff --git a/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs b/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs index 3f386b6ea..e7a68ddf3 100644 --- a/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs +++ b/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs @@ -1,4 +1,7 @@ using System; +using System.Collections.Concurrent; +using System.Threading; +using System.Threading.Tasks; using AngleSharp.Dom; using AngleSharp.Html.Dom; using Xunit; @@ -170,6 +173,51 @@ public void Test021() actual[1].ShouldBeAssignableTo(); } + [Fact] + public async Task Dispose_does_not_throw_while_another_thread_is_parsing() + { + var disposeExceptions = new ConcurrentBag(); + + for (var i = 0; i < 100; i++) + { + using var cts = new CancellationTokenSource(); + using var parser = new BunitHtmlParser(); + + var parsing = Task.Run( + () => + { + while (!cts.IsCancellationRequested) + { + try + { + parser.Parse("

Hello world

"); + } + catch (Exception) + { + // This loop deliberately races Dispose(), so its own + // failures are expected and are not what is under test. + // Only Dispose() itself is asserted on below. + } + } + }, + Xunit.TestContext.Current.CancellationToken); + + // Give the parsing loop a moment to get into Parse(). + await Task.Delay(1, Xunit.TestContext.Current.CancellationToken); + + var exception = Record.Exception(parser.Dispose); + if (exception is not null) + { + disposeExceptions.Add(exception); + } + + await cts.CancelAsync(); + await parsing; + } + + disposeExceptions.ShouldBeEmpty(); + } + private static void VerifyElementParsedWithId(string expectedElementName, List actual) { var elm = actual.OfType() From 9c622ca932a0fd069221b2d3733bfb5fb6f60074 Mon Sep 17 00:00:00 2001 From: Thimo Buchheister Date: Tue, 11 Aug 2026 11:28:16 +0200 Subject: [PATCH 2/2] docs: add changelog entry for the BunitHtmlParser dispose race fix --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 676334358..dd7160a80 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 + +- `BunitHtmlParser.Dispose()` no longer throws `InvalidOperationException: Collection was modified` when a parse is in flight on another thread, which surfaced as an intermittent failure during test teardown against a random test. Reported and fixed by [@thimobuchheister](https://github.com/thimobuchheister) in #1892. + ## [2.9.0] - 2026-08-03 ### Changed