-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fixed race condition in the disposing of the QuickGrid #62840
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
Fixed race condition in the disposing of the QuickGrid #62840
Conversation
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs
Outdated
Show resolved
Hide resolved
....AspNetCore.Components.QuickGrid/test/Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj
Show resolved
Hide resolved
...Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs
Outdated
Show resolved
Hide resolved
...Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs
Outdated
Show resolved
Hide resolved
...Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs
Outdated
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
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 race condition in the QuickGrid component that could occur when the component is disposed while a JavaScript module is still being loaded. The fix adds a disposal flag to prevent JavaScript operations from executing after disposal has begun.
- Adds a
_wasDisposed
boolean flag to track disposal state and prevent operations after disposal - Updates the disposal and rendering logic to handle race conditions gracefully
- Introduces comprehensive tests to verify the fix works and demonstrates the original problem
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
QuickGrid.razor.cs | Adds _wasDisposed flag and disposal race condition protection |
GridRaceConditionTest.cs | New test class with comprehensive race condition testing |
NotFailingQuickGrid.cs | Test helper component that uses the fixed QuickGrid implementation |
FailingQuickGrid.cs | Test helper component that simulates the original race condition bug |
Microsoft.AspNetCore.Components.csproj | Adds internal visibility for QuickGrid tests |
Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj | Includes shared test helpers |
Comments suppressed due to low confidence (1)
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs:156
- [nitpick] The field name
_wasDisposed
should be_disposeBool
to match the variable name referenced in the PR description and test code, ensuring consistency across the codebase.
private bool _wasDisposed;
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs
Outdated
Show resolved
Hide resolved
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs
Outdated
Show resolved
Hide resolved
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs
Show resolved
Hide resolved
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.
Great job!
Only a few clean-up updates.
...Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs
Show resolved
Hide resolved
...Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs
Outdated
Show resolved
Hide resolved
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs
Outdated
Show resolved
Hide resolved
src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs
Outdated
Show resolved
Hide resolved
…Grid/test/GridRaceConditionTest.cs Co-authored-by: Ilona Tomkowicz <[email protected]>
Fixed race condition in the disposing of the QuickGrid
Description
This pull request introduces changes to improve the disposal logic of the
QuickGrid
component and adds new tests to ensure proper handling of race conditions during disposal. It also updates project files to include new test dependencies and internal visibility settings. Below is a summary of the most important changes:Changes:
_disposeBool
flag to theQuickGrid
component to track its disposal state and prevent further operations if disposal is in progress.DisposeAsync
method to set_disposeBool
totrue
and ensure proper cleanup of resources.OnAfterRenderAsync
method to check the_disposeBool
flag and exit early if the component is being disposed while the JavaScript module is loading.GridRaceConditionTest
, to verify that theQuickGrid
component can handle race conditions during disposal without errors. This includes simulating JavaScript interop behavior and rendering scenarios.InternalsVisibleTo
forMicrosoft.AspNetCore.Components.QuickGrid.Tests
in theMicrosoft.AspNetCore.Components.csproj
file to allow internal access for testing purposes.Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj
file to include shared test helpers and dependencies for the new tests.Fixes #47173