-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix Components: prevent collection modification during nested component navigation with SupplyParameterFromQuery #64963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ent navigation with SupplyParameterFromQuery When NavigateTo causes a parent component with [SupplyParameterFromQuery] to conditionally render a child component (also with [SupplyParameterFromQuery]), the child's subscription modifies the _subscribers HashSet during enumeration in OnLocationChanged, causing InvalidOperationException. Added _isProcessingLocationChange flag to route new subscribers to _pendingSubscribers during location change processing. Fixes dotnet#64929
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a collection modification exception that occurs when navigation causes nested components with [SupplyParameterFromQuery] to be conditionally rendered during location change processing.
Key changes:
- Introduces a flag-based mechanism to defer subscriber registration during location change processing
- Adds comprehensive test coverage for the nested component navigation scenario
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Components/Components/src/Routing/SupplyParameterFromQueryValueProvider.cs | Adds _isProcessingLocationChange flag and logic to route new subscribers to a pending collection during location change processing, preventing collection modification exceptions |
| src/Components/Components/test/Rendering/SupplyParameterFromQueryValueProviderTest.cs | Adds new test file with comprehensive test that reproduces the collection modification scenario with nested components using query parameters |
|
Tests aspnetcore-ci (Build Tests: Helix x64 Subset 1) are failing, but it does not seem to be related with the modification. |
@dotnet-policy-service agree |
ilonatommy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@claudiogodoy99, thank you for your contribution.
This approach looks good. It helps us keep the _subscribers of O(1) access type and avoids copying the collection to an array.
Because the update is connected with Routing, it makes sense to move the test file to src\Components\Components\test\Routing, not Rendering. The namespace for test should be Microsoft.AspNetCore.Components.Routing; (without Test).
fix(Components): prevent collection modification during nested component
Summary of the changes (Less than 80 chars)
Description
Problem
When
NavigateTocauses a parent component with[SupplyParameterFromQuery]to conditionally render a child component(also with
[SupplyParameterFromQuery]), the child's subscriptionmodifies the
_subscribersHashSet during enumeration inOnLocationChanged, causing anInvalidOperationException.Solution
Added
_isProcessingLocationChangeflag to route new subscribers to_pendingSubscribersduring location change processing. This ensuresthat the
_subscriberscollection is not modified while beingenumerated.
Changes
SupplyParameterFromQueryValueProvider.cs:
_isProcessingLocationChangeboolean fieldSubscribemethod to check this flag and route to pendingsubscribers when processing
OnLocationChangedlogic in try-finally block to set/resetthe flag
_pendingSubscribersduringlocation change processing
SupplyParameterFromQueryValueProviderTest.cs (new file):
modification scenario
parameters
scenario
Fixes #64929
Testing
NavigationWithNestedComponentsDoesNotThrowCollectionModifiedExceptionthat specifically reproduces and validates the fix for the concurrent
modification issue
based on query parameters
doesn't throw exceptions