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