diff --git a/CHANGELOG.md b/CHANGELOG.md index 676334358..c568bdff2 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 during test teardown. Reported by [@thimobuchheister](https://github.com/thimobuchheister) in #1892. Fixed by [@linkdotnet](https://github.com/linkdotnet). + ## [2.9.0] - 2026-08-03 ### Changed diff --git a/src/bunit/Rendering/BunitHtmlParser.cs b/src/bunit/Rendering/BunitHtmlParser.cs index fa1e08736..47fcf0095 100644 --- a/src/bunit/Rendering/BunitHtmlParser.cs +++ b/src/bunit/Rendering/BunitHtmlParser.cs @@ -23,6 +23,8 @@ internal sealed class BunitHtmlParser : IDisposable private readonly IBrowsingContext context; private readonly HtmlParser htmlParser; private readonly List documents = new(); + private readonly object parserLock = new(); + private bool disposed; /// /// Initializes a new instance of the class @@ -69,13 +71,16 @@ public INodeList Parse([StringSyntax("Html")] string markup) { ArgumentNullException.ThrowIfNull(markup); - var document = GetNewDocumentAsync().GetAwaiter().GetResult(); + lock (parserLock) + { + var document = GetNewDocumentAsync().GetAwaiter().GetResult(); - var (ctx, matchedElement) = GetParseContext(markup, document); + var (ctx, matchedElement) = GetParseContext(markup, document); - return ctx is null && matchedElement is not null - ? ParseSpecial(markup, matchedElement) - : htmlParser.ParseFragment(markup, ctx!); + return ctx is null && matchedElement is not null + ? ParseSpecial(markup, matchedElement) + : htmlParser.ParseFragment(markup, ctx!); + } } private INodeList ParseSpecial(string markup, string matchedElement) @@ -158,10 +163,18 @@ private async Task GetNewDocumentAsync() /// public void Dispose() { - context.Dispose(); - foreach (var doc in documents) + lock (parserLock) { - doc.Dispose(); + if (disposed) + return; + + disposed = true; + + context.Dispose(); + foreach (var doc in documents) + { + doc.Dispose(); + } } } diff --git a/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs b/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs index 3f386b6ea..1b85f8ced 100644 --- a/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs +++ b/tests/bunit.tests/Rendering/BunitHtmlParserTest.cs @@ -170,6 +170,63 @@ public void Test021() actual[1].ShouldBeAssignableTo(); } + [Fact(DisplayName = "Dispose() does not throw while another thread is parsing")] + public async Task DisposeDoesNotThrowWhileAnotherThreadIsParsing() + { + using var cts = new CancellationTokenSource(); + using var parser = new BunitHtmlParser(); + var parseCount = 0; + Exception? parseException = null; + + var parsing = Task.Run( + () => + { + while (!cts.IsCancellationRequested) + { + try + { + parser.Parse("

Hello world

"); + Interlocked.Increment(ref parseCount); + } + catch (InvalidOperationException ex) + { + // "Collection was modified" - the race this test guards against. + parseException = ex; + return; + } + catch (Exception) + { + // Expected once Dispose() has completed: parsing against a + // disposed AngleSharp browsing context. + } + } + }, + CancellationToken.None); + + // Let the parser build up a sizeable document list, so the enumeration in + // Dispose() is long enough to overlap with a concurrent call to Parse(). + while (Volatile.Read(ref parseCount) < 100) + { + await Task.Yield(); + } + + Should.NotThrow(parser.Dispose); + + await cts.CancelAsync(); + await parsing; + + parseException.ShouldBeNull(); + } + + [Fact(DisplayName = "Dispose() is idempotent")] + public void DisposeIsIdempotent() + { + using var parser = new BunitHtmlParser(); + parser.Dispose(); + + Should.NotThrow(parser.Dispose); + } + private static void VerifyElementParsedWithId(string expectedElementName, List actual) { var elm = actual.OfType()