Skip to content

fix: read the wrapped element once so a concurrent re-render cannot return null - #1936

Closed
psu-prog wants to merge 1 commit into
bUnit-dev:mainfrom
psu-prog:fix/wrapped-element-race
Closed

psu-prog wants to merge 1 commit into
bUnit-dev:mainfrom
psu-prog:fix/wrapped-element-race

Conversation

@psu-prog

Copy link
Copy Markdown

Pull request description

An element returned by Find() sometimes throws a NullReferenceException on any member access when the component re-renders on another thread at the same time, for example from a timer, a debounce or an awaited task. Our CI hit it in ClickAsync with bUnit 2.11.3:

System.NullReferenceException: Object reference not set to an instance of an object.
   at Bunit.AngleSharpExtensions.GetBunitContext(INode node) in /_/src/bunit/Extensions/Internal/AngleSharpExtensions.cs:line 70
   at Bunit.TriggerEventDispatchExtensions.TriggerEventAsync(IElement element, String eventName, EventArgs eventArgs) in /_/src/bunit/EventDispatchExtensions/TriggerEventDispatchExtensions.cs:line 65
   at Bunit.EventHandlerDispatchExtensions.ClickAsync(IElement element, MouseEventArgs eventArgs)

WrapperBase<TElement>.WrappedElement checked the cached element for null and then read the field again to return it. A re-render on another thread runs OnElementReplaced, which sets the field to null. When that happened between the two reads, the getter returned null and every wrapper member threw. The wrapper members are inlined, so the exception surfaces in the caller (GetBunitContext above).

The getter now reads the field once with ??=. A reset after the read only means the next access queries the DOM again, which is the existing behavior.

Test: RefreshingWrappedElementTest.Test040 reads a found element in a loop for one second while another thread re-renders the component. Without the fix it fails with a NullReferenceException in 5 of 5 runs; with the fix it passes in 5 of 5 runs, and the test class passes on net8.0, net9.0, net10.0 and net11.0. The test takes one second, because the race needs time to show; I am happy to shorten it or move it if you prefer. In real tests the window is small: a find-and-click loop against a component that re-renders continuously hit it about once in 20,000 clicks, which is enough to make a large suite flaky in CI.

The verified snapshot WrapperElementsGeneratorTest.Generator#WrapperBase.g.verified.cs changes with the source. bunit.generators.tests, bunit.tests and bunit.web.query.tests pass on net10.0.

PR meta checklist

  • Pull request is targeted at main branch for code
    or targeted at stable branch for documentation that is live on bunit.dev.
  • Pull request is linked to all related issues, if any.
  • I have read the CONTRIBUTING.md document.

Code PR specific checklist

  • My code follows the code style of this project and AspNetCore coding guidelines.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have updated the appropriate sub section in the CHANGELOG.md.
  • I have added, updated or removed tests to according to my changes.
    • All tests passed.

🤖 Generated with Claude Code

…eturn null

WrapperBase.WrappedElement checked the cached element for null and then
read the field again to return it. When the component re-rendered on
another thread (a timer, a debounce, an awaited task), OnElementReplaced
reset the field to null between the two reads, and the getter returned
null. Every member of the wrapper then threw a NullReferenceException,
for example GetBunitContext in ClickAsync.

The getter now reads the field once with ??=.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@psu-prog

Copy link
Copy Markdown
Author

Closing in favor of #1937, which describes the bug and the fix.

@psu-prog psu-prog closed this Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant