Repository navigation
fix: latestRenderNodes was not thread-safe (fixes #1938) - #1948
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The regression test needs controlled synchronization to reliably detect the original race.
1 open issue
What changed in this PR
Fixes stale DOM-node caching in bUnit when rendering overlaps HTML parsing.
Changes:
- Associates cached nodes with their markup snapshot.
- Adds a cross-thread rendering regression test.
- Documents the fix in the changelog.
| File | Description |
|---|---|
| tests/bunit.tests/Rendering/RenderedComponentTest.cs | Adds concurrent rendering coverage. |
| src/bunit/Rendering/RenderedComponent.cs | Validates cached nodes against current markup. |
| CHANGELOG.md | Records the stale-node fix. |
Review effort: Balanced
Comment on lines
+308
to
+311
| var render = Task.Run(() => cut.Render(ps => ps.AddChildContent(content)), Xunit.TestContext.Current.CancellationToken); | ||
| while (!render.IsCompleted) | ||
| _ = cut.Nodes; | ||
| await render; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Summary
Nodescached a parse of the old markup when a render (e.g. timer) finished mid-parse;Findthen served stale nodes with removed event handler IDs →UnknownEventHandlerIdException.Nodes are now cached with the markup string they were parsed from and re-parsed when it no longer matches:
Not needed in v3: #1910 (shared render tree) replaces this with immutable markup snapshots — take its side on conflict.
Evidence
Test030(deterministic): render<p>1</p>without readingNodes; holdBunitHtmlParser's parse lock so a reader thread pauses after capturing the old markup; render<p>2</p>; release;cut.Find("p").TextContent.ShouldBe("2").should be "2" but was "1"After: passed 5/5; full net10.0 suite 2274/2274 passed
Merge Danger
Door: two-way
Blast Radius: small
Only
RenderedComponent.Nodeschanges; same instance/new-instance semantics as before (Test006–008 unchanged).