Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 22 additions & 2 deletions src/bunit/Rendering/BunitHtmlParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal sealed class BunitHtmlParser : IDisposable
private readonly IBrowsingContext context;
private readonly HtmlParser htmlParser;
private readonly List<IDocument> documents = new();
private readonly object documentsLock = new();

/// <summary>
/// Initializes a new instance of the <see cref="BunitHtmlParser"/> class
Expand Down Expand Up @@ -151,15 +152,34 @@ private static (IElement? Context, string? MatchedElement) GetParseContextFromTa
private async Task<IDocument> GetNewDocumentAsync()
{
var result = await context.OpenNewAsync().ConfigureAwait(false);
documents.Add(result);

lock (documentsLock)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the lock here is not needed - the only usage does a synchronous wait for it.

{
documents.Add(result);
}

return result;
}

/// <inheritdoc/>
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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lock can go through the while Dispose method. Context should be safe here as well. That would also remove the need for re-enumerating.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also Parse should for sure have the lock - dispose and parse can run against each other

{
documentsToDispose = documents.ToArray();
documents.Clear();
}

foreach (var doc in documentsToDispose)
{
doc.Dispose();
}
Expand Down
48 changes: 48 additions & 0 deletions tests/bunit.tests/Rendering/BunitHtmlParserTest.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -170,6 +173,51 @@ public void Test021()
actual[1].ShouldBeAssignableTo<IHtmlHtmlElement>();
}

[Fact]
public async Task Dispose_does_not_throw_while_another_thread_is_parsing()
{
var disposeExceptions = new ConcurrentBag<Exception>();

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("<p>Hello world</p>");
}
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<INode> actual)
{
var elm = actual.OfType<IElement>()
Expand Down