Skip to content

Commit 0a54860

Browse files
committed
fix: Race condition between HTML parser disposal and concurrent parsing
1 parent fa46d8a commit 0a54860

3 files changed

Lines changed: 82 additions & 8 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ All notable changes to **bUnit** will be documented in this file. The project ad
66

77
## [Unreleased]
88

9+
### Fixed
10+
11+
- `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).
12+
913
## [2.9.0] - 2026-08-03
1014

1115
### Changed

‎src/bunit/Rendering/BunitHtmlParser.cs‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ internal sealed class BunitHtmlParser : IDisposable
2323
private readonly IBrowsingContext context;
2424
private readonly HtmlParser htmlParser;
2525
private readonly List<IDocument> documents = new();
26+
private readonly object parserLock = new();
27+
private bool disposed;
2628

2729
/// <summary>
2830
/// Initializes a new instance of the <see cref="BunitHtmlParser"/> class
@@ -69,13 +71,16 @@ public INodeList Parse([StringSyntax("Html")] string markup)
6971
{
7072
ArgumentNullException.ThrowIfNull(markup);
7173

72-
var document = GetNewDocumentAsync().GetAwaiter().GetResult();
74+
lock (parserLock)
75+
{
76+
var document = GetNewDocumentAsync().GetAwaiter().GetResult();
7377

74-
var (ctx, matchedElement) = GetParseContext(markup, document);
78+
var (ctx, matchedElement) = GetParseContext(markup, document);
7579

76-
return ctx is null && matchedElement is not null
77-
? ParseSpecial(markup, matchedElement)
78-
: htmlParser.ParseFragment(markup, ctx!);
80+
return ctx is null && matchedElement is not null
81+
? ParseSpecial(markup, matchedElement)
82+
: htmlParser.ParseFragment(markup, ctx!);
83+
}
7984
}
8085

8186
private INodeList ParseSpecial(string markup, string matchedElement)
@@ -158,10 +163,18 @@ private async Task<IDocument> GetNewDocumentAsync()
158163
/// <inheritdoc/>
159164
public void Dispose()
160165
{
161-
context.Dispose();
162-
foreach (var doc in documents)
166+
lock (parserLock)
163167
{
164-
doc.Dispose();
168+
if (disposed)
169+
return;
170+
171+
disposed = true;
172+
173+
context.Dispose();
174+
foreach (var doc in documents)
175+
{
176+
doc.Dispose();
177+
}
165178
}
166179
}
167180

‎tests/bunit.tests/Rendering/BunitHtmlParserTest.cs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,63 @@ public void Test021()
170170
actual[1].ShouldBeAssignableTo<IHtmlHtmlElement>();
171171
}
172172

173+
[Fact(DisplayName = "Dispose() does not throw while another thread is parsing")]
174+
public async Task DisposeDoesNotThrowWhileAnotherThreadIsParsing()
175+
{
176+
using var cts = new CancellationTokenSource();
177+
using var parser = new BunitHtmlParser();
178+
var parseCount = 0;
179+
Exception? parseException = null;
180+
181+
var parsing = Task.Run(
182+
() =>
183+
{
184+
while (!cts.IsCancellationRequested)
185+
{
186+
try
187+
{
188+
parser.Parse("<p>Hello world</p>");
189+
Interlocked.Increment(ref parseCount);
190+
}
191+
catch (InvalidOperationException ex)
192+
{
193+
// "Collection was modified" - the race this test guards against.
194+
parseException = ex;
195+
return;
196+
}
197+
catch (Exception)
198+
{
199+
// Expected once Dispose() has completed: parsing against a
200+
// disposed AngleSharp browsing context.
201+
}
202+
}
203+
},
204+
CancellationToken.None);
205+
206+
// Let the parser build up a sizeable document list, so the enumeration in
207+
// Dispose() is long enough to overlap with a concurrent call to Parse().
208+
while (Volatile.Read(ref parseCount) < 100)
209+
{
210+
await Task.Yield();
211+
}
212+
213+
Should.NotThrow(parser.Dispose);
214+
215+
await cts.CancelAsync();
216+
await parsing;
217+
218+
parseException.ShouldBeNull();
219+
}
220+
221+
[Fact(DisplayName = "Dispose() is idempotent")]
222+
public void DisposeIsIdempotent()
223+
{
224+
using var parser = new BunitHtmlParser();
225+
parser.Dispose();
226+
227+
Should.NotThrow(parser.Dispose);
228+
}
229+
173230
private static void VerifyElementParsedWithId(string expectedElementName, List<INode> actual)
174231
{
175232
var elm = actual.OfType<IElement>()

0 commit comments

Comments
 (0)