ParameterState: Add Snapshot to avoid race condition#12023
Conversation
|
Missing a unit test for now, but it does fixes the reproduction project. |
There was a problem hiding this comment.
Code Review
This pull request introduces a snapshot mechanism to the ParameterState framework to address a race condition that can occur with rapid, successive calls to SetParametersAsync. By creating an immutable snapshot of the parameter's state when a change is detected, the change handler can be invoked safely later, preventing state corruption from concurrent updates. The implementation is clean, well-documented, and correctly isolates the state for asynchronous handler execution. The changes are logically sound and include necessary updates to tests, interfaces, and container classes to support the new snapshot pattern. Overall, this is an excellent improvement to the robustness of the framework.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a snapshot mechanism to capture parameter invocation state before handlers are executed, preventing race conditions when concurrent SetParametersAsync calls occur. The key change is that instead of directly invoking handlers on live state, the code now creates immutable snapshots containing cloned event arguments and handler references.
- Replaces direct handler invocation with snapshot-based deferred execution
- Introduces
IParameterStateInvocationSnapshotinterface andParameterStateInvocationSnapshot<T>implementation - Adds
Clone()method toParameterChangedEventArgs<T>for safe state capture
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
ParameterStateInternalOfT.cs |
Replaces ParameterChangeHandleAsync() with CreateInvocationSnapshot() to capture state for deferred invocation |
ParameterScopeContainer.cs |
Updates to create snapshots before baseSetParametersAsync and invoke handlers from snapshots |
ParameterContainer.cs |
Adds snapshot creation step in parameter change detection pipeline |
ParameterChangedEventArgs.cs |
Adds Clone() method to create independent copies of event arguments |
ParameterStateInvocationSnapshot.cs |
New class implementing snapshot-based handler invocation with captured state |
IParameterStateInvocationSnapshot.cs |
New interface defining contract for parameter invocation snapshots |
IParameterComponentLifeCycle.cs |
Updates interface to replace ParameterChangeHandleAsync() with CreateInvocationSnapshot() |
ParameterHandlerUniquenessComparer.cs |
Extends comparer to support IParameterStateInvocationSnapshot equality checks |
ParameterStateTests.cs |
Updates tests to use new snapshot-based invocation pattern |
ParameterHandlerUniquenessComparerTests.cs |
Adds test coverage for snapshot equality and hash code operations |
ParameterContainerTests.cs |
Adds new test verifying snapshot mechanism prevents race condition issues |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
From discord:
Explanation:
It looks like this is caused by
Navigation.NavigateTo.What I’m seeing is that when the page initializes, it calls Blazor’s
SetParametersAsync, which tells us the parameters changed and queues that info up. But then, because of theNavigateTo, anotherSetParametersAsyncfires almost right away, before the first one even finishes. At that point, it sees that the parameter was already set to the same value, since it reuses sameParameterStateInternalreference and removes the info that parameter changed:MudBlazor/src/MudBlazor/State/ParameterStateInternalOfT.cs
Line 131 in 3c678ef
Then, when the first
SetParametersAsyncgets to finish to call the change handler it sees no changes (since it was already cleared earlier by secondSetParametersAsync), and doesn’t triggerParameterChangeHandleAsync.Checklist:
dev).