Repository navigation
Conversation
…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>
Author
|
Closing in favor of #1937, which describes the bug and the fix. |
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.
Pull request description
An element returned by
Find()sometimes throws aNullReferenceExceptionon 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 inClickAsyncwith bUnit 2.11.3:WrapperBase<TElement>.WrappedElementchecked the cached element for null and then read the field again to return it. A re-render on another thread runsOnElementReplaced, which sets the field tonull. When that happened between the two reads, the getter returnednulland every wrapper member threw. The wrapper members are inlined, so the exception surfaces in the caller (GetBunitContextabove).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.Test040reads a found element in a loop for one second while another thread re-renders the component. Without the fix it fails with aNullReferenceExceptionin 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.cschanges with the source.bunit.generators.tests,bunit.testsandbunit.web.query.testspass on net10.0.PR meta checklist
mainbranch for codeor targeted at
stablebranch for documentation that is live on bunit.dev.Code PR specific checklist
🤖 Generated with Claude Code