Skip to content
Merged
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 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
Expand Down
29 changes: 21 additions & 8 deletions src/bunit/Rendering/BunitHtmlParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ internal sealed class BunitHtmlParser : IDisposable
private readonly IBrowsingContext context;
private readonly HtmlParser htmlParser;
private readonly List<IDocument> documents = new();
private readonly object parserLock = new();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use the Lock type instead of object?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have to #if NET9_0_OR_GREATER - we never use System.Lock until now but that can be done in bunit v3

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

right, over and out :D

private bool disposed;

/// <summary>
/// Initializes a new instance of the <see cref="BunitHtmlParser"/> class
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -158,10 +163,18 @@ private async Task<IDocument> GetNewDocumentAsync()
/// <inheritdoc/>
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();
}
}
}

Expand Down
57 changes: 57 additions & 0 deletions tests/bunit.tests/Rendering/BunitHtmlParserTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,63 @@ public void Test021()
actual[1].ShouldBeAssignableTo<IHtmlHtmlElement>();
}

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