From ecdbe6b388a1a2ca4a7be40974b0eb7c6ef5b244 Mon Sep 17 00:00:00 2001 From: Daria Tiurina Date: Mon, 21 Jul 2025 14:38:58 +0200 Subject: [PATCH 1/8] Fixed race condition in the disposing of the QuickGrid --- .../Microsoft.AspNetCore.Components.csproj | 1 + .../src/QuickGrid.razor.cs | 8 + .../test/GridRaceConditionTest.cs | 144 ++++++++++++++++++ ...pNetCore.Components.QuickGrid.Tests.csproj | 8 +- 4 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs diff --git a/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj b/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj index 8d36fe78ad80..f165e6f82489 100644 --- a/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj +++ b/src/Components/Components/src/Microsoft.AspNetCore.Components.csproj @@ -89,6 +89,7 @@ + diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs index 363ad846cbf7..8dcfd7430f21 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs @@ -152,6 +152,8 @@ public partial class QuickGrid : IAsyncDisposable // If the PaginationState mutates, it raises this event. We use it to trigger a re-render. private readonly EventCallbackSubscriber _currentPageItemsChanged; + private bool _disposeBool; + /// /// Constructs an instance of . /// @@ -206,6 +208,11 @@ protected override async Task OnAfterRenderAsync(bool firstRender) if (firstRender) { _jsModule = await JS.InvokeAsync("import", "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js"); + if (_disposeBool) + { + // If the component has been disposed while JS module was being loaded, we don't need to continue + return; + } _jsEventDisposable = await _jsModule.InvokeAsync("init", _tableReference); } @@ -434,6 +441,7 @@ private string GridClass() /// public async ValueTask DisposeAsync() { + _disposeBool = true; _currentPageItemsChanged.Dispose(); try diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs new file mode 100644 index 000000000000..be0e5a529eae --- /dev/null +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs @@ -0,0 +1,144 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.AspNetCore.Components.RenderTree; +using Microsoft.AspNetCore.Components.QuickGrid; +using Microsoft.AspNetCore.Components.Test.Helpers; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.JSInterop; + +namespace Microsoft.AspNetCore.Components.QuickGrid.Test; + +public class GridRaceConditionTest +{ + private TestRenderer _renderer = new(); + private TaskCompletionSource _tcs = new(); + + public GridRaceConditionTest() + { + var testJsRuntime = new TestJsRuntime(_tcs); + var serviceProvider = new ServiceCollection() + .AddSingleton(testJsRuntime) + .BuildServiceProvider(); + _renderer = new(serviceProvider); + } + + [Fact] + public async Task CanCorrectlyDisposeAsync() + { + var testComponent = new TestComponent(); + + var componentId = _renderer.AssignRootComponentId(testComponent); + _renderer.RenderRootComponent(componentId); + await Task.Delay(10); + _renderer.RenderRootComponent(componentId); + _tcs.SetResult(); + } +} + +internal class TestComponent : ComponentBase +{ + private bool _firstRender = true; + private PaginationState _pagination = new() { ItemsPerPage = 2 }; + + internal class Person + { + public int Id { get; set; } + public string Name { get; set; } = string.Empty; + public int Age { get; set; } + } + + private IQueryable _people = new List + { + new Person { Id = 1, Name = "John Doe", Age = 30 }, + new Person { Id = 2, Name = "Jane Smith", Age = 25 }, + new Person { Id = 3, Name = "Alice Johnson", Age = 22 } + }.AsQueryable(); + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + if (_firstRender) + { + //Render the QuickGrid + builder.OpenComponent>(0); + builder.AddAttribute(1, "Items", _people); + builder.AddAttribute(2, "Pagination", _pagination); + builder.AddAttribute(3, nameof(QuickGrid.ChildContent), + (RenderFragment)(builder => BuildColumnsRenderFragment(builder))); + builder.CloseComponent(); + + builder.OpenComponent(4); + builder.AddAttribute(5, "State", _pagination); + builder.CloseComponent(); + _firstRender = false; + } + } + + protected void BuildColumnsRenderFragment(RenderTreeBuilder builder) + { + //Render the PropertyColumn for Id + builder.OpenComponent>(0); + builder.AddAttribute(1, nameof(PropertyColumn.Property), + (System.Linq.Expressions.Expression>)(p => p.Id)); + builder.AddAttribute(2, nameof(PropertyColumn.Sortable), true); + builder.CloseComponent(); + + //Render the PropertyColumn for Name + builder.OpenComponent>(3); + builder.AddAttribute(4, nameof(PropertyColumn.Property), + (System.Linq.Expressions.Expression>)(p => p.Name)); + builder.AddAttribute(5, nameof(PropertyColumn.Sortable), true); + builder.CloseComponent(); + + //Render the PropertyColumn for Age + builder.OpenComponent>(6); + builder.AddAttribute(7, nameof(PropertyColumn.Property), + (System.Linq.Expressions.Expression>)(p => p.Age)); + builder.AddAttribute(8, nameof(PropertyColumn.Sortable), true); + builder.CloseComponent(); + } + + public void TriggerRender() + { + InvokeAsync(StateHasChanged); + } + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + if (firstRender) + { + await Task.Delay(1); + StateHasChanged(); + } + await base.OnAfterRenderAsync(firstRender); + } +} + +internal class TestJsRuntime(TaskCompletionSource tcs) : IJSRuntime +{ + private readonly TaskCompletionSource _tcs = tcs; + + public async ValueTask InvokeAsync(string identifier, object[] args = null) + { + if (identifier == "import" && args != null && args.Length > 0 && args[0] is string modulePath) + { + if (modulePath == "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js") + { + await _tcs.Task; + return default!; + } + } + throw new Exception("JS import was not correctly processed while disposing of the component."); + } + + public ValueTask InvokeAsync(string identifier, params object[] args) + { + throw new NotImplementedException(); + } + + public ValueTask InvokeAsync(string identifier, CancellationToken cancellationToken, object[] args) + { + throw new NotImplementedException(); + } +} diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj index 402e6a12a8f7..21eb671ff544 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/Microsoft.AspNetCore.Components.QuickGrid.Tests.csproj @@ -1,4 +1,4 @@ - + $(DefaultNetCoreTargetFramework) @@ -8,8 +8,10 @@ - - + + + + From 1be64276d6226e4e521abfea1d9dba22045d8d6f Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Wed, 23 Jul 2025 13:31:39 +0200 Subject: [PATCH 2/8] WIP: Update tests to check for fixed and not fixed scenario. --- .../test/FailingQuickGrid.cs | 109 ++++++++ .../test/GridRaceConditionTest.cs | 262 ++++++++++++------ 2 files changed, 289 insertions(+), 82 deletions(-) create mode 100644 src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs new file mode 100644 index 000000000000..c9f141944065 --- /dev/null +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs @@ -0,0 +1,109 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Components; +using Microsoft.AspNetCore.Components.QuickGrid; +using Microsoft.JSInterop; +using System.Reflection; + +namespace Microsoft.AspNetCore.Components.QuickGrid.Test; + +/// +/// A QuickGrid implementation that simulates the behavior before the race condition fix. +/// This class intentionally does NOT set _disposeBool during disposal to simulate the race condition. +/// +/// The type of data represented by each row in the grid. +internal class FailingQuickGrid : QuickGrid, IAsyncDisposable +{ + [Inject] private IJSRuntime JS { get; set; } = default!; + + private readonly TaskCompletionSource _onAfterRenderCompleted = new(); + private bool _completionSignaled; + + public bool DisposeAsyncWasCalled { get; private set; } + + /// + /// Task that completes when OnAfterRenderAsync has finished executing. + /// This allows tests to wait deterministically for the race condition to occur. + /// + public Task OnAfterRenderCompleted => _onAfterRenderCompleted.Task; + + /// + /// Intentionally does NOT call base.DisposeAsync() to prevent _disposeBool from being set. + /// This simulates the behavior before the fix was implemented. + /// + public new async ValueTask DisposeAsync() + { + DisposeAsyncWasCalled = true; + // Intentionally do nothing to prevent _disposeBool from being set to true + // This means the OnAfterRenderAsync method will not detect that the component is disposed + // and will proceed to call init() even after disposal, demonstrating the race condition + + // DO NOT call base.DisposeAsync() - this is the key to simulating the race condition + await Task.CompletedTask; + } + + /// + /// Explicit interface implementation to ensure our disposal method is called. + /// + async ValueTask IAsyncDisposable.DisposeAsync() + { + await DisposeAsync(); + } + + /// + /// Check if _disposeBool is false, proving we didn't call base.DisposeAsync(). + /// This is used by tests to verify that our simulation is working correctly. + /// + public bool IsDisposeBoolFalse() + { + var field = typeof(QuickGrid).GetField("_disposeBool", BindingFlags.NonPublic | BindingFlags.Instance); + return field?.GetValue(this) is false; + } + + /// + /// Override OnAfterRenderAsync to simulate the race condition by NOT checking _disposeBool. + /// This exactly replicates the code path that existed before the race condition fix. + /// + protected override async Task OnAfterRenderAsync(bool firstRender) + { + try + { + if (firstRender) + { + // Get the IJSRuntime (same as base class) + if (JS != null) + { + // Import the JS module (this will trigger our TestJsRuntime's import logic) + var jsModule = await JS.InvokeAsync("import", + "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js"); + + // THE KEY DIFFERENCE: The original code did NOT check _disposeBool here + // The fix added: if (_disposeBool) return; + // By omitting this check, we demonstrate the race condition where init gets called on disposed components + + // Call init - this happens even if component was disposed during import + // For our test, we don't need a real table reference, just need to trigger the JS call + await jsModule.InvokeAsync("init", new object()); + + // Signal completion only after the init call has completed, and only once + if (!_completionSignaled) + { + _completionSignaled = true; + _onAfterRenderCompleted.TrySetResult(); + } + return; + } + } + } + finally + { + // Only signal completion if we haven't already done it and this is the first render + if (firstRender && !_completionSignaled) + { + _completionSignaled = true; + _onAfterRenderCompleted.TrySetResult(); + } + } + } +} \ No newline at end of file diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs index be0e5a529eae..0e87b1d07822 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.AspNetCore.Components.Rendering; -using Microsoft.AspNetCore.Components.RenderTree; using Microsoft.AspNetCore.Components.QuickGrid; using Microsoft.AspNetCore.Components.Test.Helpers; using Microsoft.Extensions.DependencyInjection; @@ -12,133 +11,232 @@ namespace Microsoft.AspNetCore.Components.QuickGrid.Test; public class GridRaceConditionTest { - private TestRenderer _renderer = new(); - private TaskCompletionSource _tcs = new(); - public GridRaceConditionTest() + [Fact] + public async Task CanCorrectlyDisposeAsync() { - var testJsRuntime = new TestJsRuntime(_tcs); + var moduleLoadCompletion = new TaskCompletionSource(); + var moduleImportStarted = new TaskCompletionSource(); + var testJsRuntime = new TestJsRuntime(moduleLoadCompletion, moduleImportStarted); var serviceProvider = new ServiceCollection() .AddSingleton(testJsRuntime) .BuildServiceProvider(); - _renderer = new(serviceProvider); + var renderer = new TestRenderer(serviceProvider); + + var testComponent = new SimpleTestComponent(); + + var componentId = renderer.AssignRootComponentId(testComponent); + renderer.RenderRootComponent(componentId); + + // Wait until JS import has started but not completed + await moduleImportStarted.Task; + + // Dispose component while JS module loading is pending + testJsRuntime.MarkDisposed(); + await testComponent.DisposeAsync(); + + // Complete the JS module loading + moduleLoadCompletion.SetResult(); + + // Assert that init was not called after disposal (the fix working correctly) + Assert.False(testJsRuntime.InitWasCalledAfterDisposal, + "Init should not be called on a disposed component - this indicates the race condition fix is not working"); } [Fact] - public async Task CanCorrectlyDisposeAsync() + public async Task FailingQuickGrid_CallsInitAfterDisposal_DemonstratesRaceCondition() { - var testComponent = new TestComponent(); + var moduleLoadCompletion = new TaskCompletionSource(); + var moduleImportStarted = new TaskCompletionSource(); + var testJsRuntime = new TestJsRuntime(moduleLoadCompletion, moduleImportStarted); + var serviceProvider = new ServiceCollection() + .AddSingleton(testJsRuntime) + .BuildServiceProvider(); + var renderer = new TestRenderer(serviceProvider); + + var testComponent = new FailingGridTestComponent(); + + var componentId = renderer.AssignRootComponentId(testComponent); + renderer.RenderRootComponent(componentId); + + // Wait until JS import has started but not completed + await moduleImportStarted.Task; + + // Dispose component while JS module loading is pending + testJsRuntime.MarkDisposed(); + await testComponent.DisposeAsync(); - var componentId = _renderer.AssignRootComponentId(testComponent); - _renderer.RenderRootComponent(componentId); - await Task.Delay(10); - _renderer.RenderRootComponent(componentId); - _tcs.SetResult(); + // Verify our FailingQuickGrid's DisposeAsync was called and _disposeBool should still be false + var failingGrid = testComponent.FailingQuickGrid; + Assert.NotNull(failingGrid); + Assert.True(failingGrid.DisposeAsyncWasCalled, "FailingQuickGrid.DisposeAsync should have been called"); + Assert.True(failingGrid.IsDisposeBoolFalse(), "_disposeBool should still be false since we didn't call base.DisposeAsync()"); + + // Complete the JS module loading - this allows the FailingQuickGrid's OnAfterRenderAsync to continue + // and demonstrate the race condition by calling init after disposal + moduleLoadCompletion.SetResult(); + + // Wait for OnAfterRenderAsync to complete - deterministic timing instead of arbitrary delay + await failingGrid.OnAfterRenderCompleted; + + // Assert that init WAS called after disposal (demonstrating the race condition bug) + // The FailingQuickGrid's OnAfterRenderAsync should have called init despite being disposed + Assert.True(testJsRuntime.InitWasCalledAfterDisposal, + $"FailingQuickGrid should call init after disposal, demonstrating the race condition bug. " + + $"InitWasCalledAfterDisposal: {testJsRuntime.InitWasCalledAfterDisposal}, " + + $"DisposeAsyncWasCalled: {failingGrid.DisposeAsyncWasCalled}, " + + $"_disposeBool is false: {failingGrid.IsDisposeBoolFalse()}"); } } -internal class TestComponent : ComponentBase +internal class Person { - private bool _firstRender = true; - private PaginationState _pagination = new() { ItemsPerPage = 2 }; + public int Id { get; set; } + public string Name { get; set; } = string.Empty; +} - internal class Person - { - public int Id { get; set; } - public string Name { get; set; } = string.Empty; - public int Age { get; set; } - } +internal class SimpleTestComponent : ComponentBase, IAsyncDisposable +{ + [Inject] public IJSRuntime JSRuntime { get; set; } = default!; - private IQueryable _people = new List - { - new Person { Id = 1, Name = "John Doe", Age = 30 }, - new Person { Id = 2, Name = "Jane Smith", Age = 25 }, - new Person { Id = 3, Name = "Alice Johnson", Age = 22 } - }.AsQueryable(); + private readonly List _people = [ + new() { Id = 1, Name = "John" }, + new() { Id = 2, Name = "Jane" } + ]; protected override void BuildRenderTree(RenderTreeBuilder builder) { - if (_firstRender) + builder.OpenComponent>(0); + builder.AddAttribute(1, "Items", _people.AsQueryable()); + builder.AddAttribute(2, "ChildContent", (RenderFragment)(b => { - //Render the QuickGrid - builder.OpenComponent>(0); - builder.AddAttribute(1, "Items", _people); - builder.AddAttribute(2, "Pagination", _pagination); - builder.AddAttribute(3, nameof(QuickGrid.ChildContent), - (RenderFragment)(builder => BuildColumnsRenderFragment(builder))); - builder.CloseComponent(); - - builder.OpenComponent(4); - builder.AddAttribute(5, "State", _pagination); - builder.CloseComponent(); - _firstRender = false; - } + b.OpenComponent>(0); + b.AddAttribute(1, "Property", (System.Linq.Expressions.Expression>)(p => p.Id)); + b.CloseComponent(); + })); + builder.CloseComponent(); } - protected void BuildColumnsRenderFragment(RenderTreeBuilder builder) + public ValueTask DisposeAsync() { - //Render the PropertyColumn for Id - builder.OpenComponent>(0); - builder.AddAttribute(1, nameof(PropertyColumn.Property), - (System.Linq.Expressions.Expression>)(p => p.Id)); - builder.AddAttribute(2, nameof(PropertyColumn.Sortable), true); - builder.CloseComponent(); + if (JSRuntime is TestJsRuntime testRuntime) + { + testRuntime.MarkDisposed(); + } + return ValueTask.CompletedTask; + } +} - //Render the PropertyColumn for Name - builder.OpenComponent>(3); - builder.AddAttribute(4, nameof(PropertyColumn.Property), - (System.Linq.Expressions.Expression>)(p => p.Name)); - builder.AddAttribute(5, nameof(PropertyColumn.Sortable), true); - builder.CloseComponent(); +internal class FailingGridTestComponent : ComponentBase, IAsyncDisposable +{ + [Inject] public IJSRuntime JSRuntime { get; set; } = default!; - //Render the PropertyColumn for Age - builder.OpenComponent>(6); - builder.AddAttribute(7, nameof(PropertyColumn.Property), - (System.Linq.Expressions.Expression>)(p => p.Age)); - builder.AddAttribute(8, nameof(PropertyColumn.Sortable), true); - builder.CloseComponent(); - } + private FailingQuickGrid _failingGrid; + + public FailingQuickGrid FailingQuickGrid => _failingGrid; - public void TriggerRender() + private readonly List _people = [ + new() { Id = 1, Name = "John" }, + new() { Id = 2, Name = "Jane" } + ]; + + protected override void BuildRenderTree(RenderTreeBuilder builder) { - InvokeAsync(StateHasChanged); + builder.OpenComponent>(0); + builder.AddAttribute(1, "Items", _people.AsQueryable()); + builder.AddAttribute(2, "ChildContent", (RenderFragment)(b => + { + b.OpenComponent>(0); + b.AddAttribute(1, "Property", (System.Linq.Expressions.Expression>)(p => p.Id)); + b.CloseComponent(); + })); + builder.AddComponentReferenceCapture(3, component => _failingGrid = (FailingQuickGrid)component); + builder.CloseComponent(); } - protected override async Task OnAfterRenderAsync(bool firstRender) + public async ValueTask DisposeAsync() { - if (firstRender) + if (JSRuntime is TestJsRuntime testRuntime) { - await Task.Delay(1); - StateHasChanged(); + testRuntime.MarkDisposed(); + } + + if (_failingGrid is not null) + { + await _failingGrid.DisposeAsync(); } - await base.OnAfterRenderAsync(firstRender); } } -internal class TestJsRuntime(TaskCompletionSource tcs) : IJSRuntime +internal class TestJsRuntime(TaskCompletionSource moduleCompletion, TaskCompletionSource importStarted) : IJSRuntime { - private readonly TaskCompletionSource _tcs = tcs; + private readonly TaskCompletionSource _moduleCompletion = moduleCompletion; + private readonly TaskCompletionSource _importStarted = importStarted; + private bool _disposed; + + public bool InitWasCalledAfterDisposal { get; private set; } public async ValueTask InvokeAsync(string identifier, object[] args = null) { - if (identifier == "import" && args != null && args.Length > 0 && args[0] is string modulePath) + if (identifier == "import" && args?.Length > 0 && args[0] is string modulePath && + modulePath == "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js") { - if (modulePath == "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js") - { - await _tcs.Task; - return default!; - } + // Signal that import has started + _importStarted.TrySetResult(); + + // Wait for test to control when import completes + await _moduleCompletion.Task; + + // Return a mock IJSObjectReference + return (TValue)(object)new TestJSObjectReference(this); } - throw new Exception("JS import was not correctly processed while disposing of the component."); + throw new InvalidOperationException($"Unexpected JS call: {identifier}"); } - public ValueTask InvokeAsync(string identifier, params object[] args) + public void MarkDisposed() => _disposed = true; + + public void RecordInitCall() { - throw new NotImplementedException(); + if (_disposed) + { + InitWasCalledAfterDisposal = true; + } } - public ValueTask InvokeAsync(string identifier, CancellationToken cancellationToken, object[] args) + public ValueTask InvokeAsync(string identifier, CancellationToken cancellationToken, object[] args) => + InvokeAsync(identifier, args); +} + +/// +/// Mock JS object reference for the QuickGrid module +/// +internal class TestJSObjectReference(TestJsRuntime jsRuntime) : IJSObjectReference +{ + private readonly TestJsRuntime _jsRuntime = jsRuntime; + + public ValueTask InvokeAsync(string identifier, object[] args) { - throw new NotImplementedException(); + if (identifier == "init") + { + _jsRuntime.RecordInitCall(); + } + return ValueTask.FromResult(default(TValue)!); } + + public ValueTask InvokeVoidAsync(string identifier, object[] args) + { + if (identifier == "init") + { + _jsRuntime.RecordInitCall(); + } + return ValueTask.CompletedTask; + } + + public ValueTask InvokeAsync(string identifier, CancellationToken cancellationToken, object[] args) => + InvokeAsync(identifier, args); + + public ValueTask InvokeVoidAsync(string identifier, CancellationToken cancellationToken, object[] args) => + InvokeVoidAsync(identifier, args); + + public ValueTask DisposeAsync() => ValueTask.CompletedTask; } From f086aaf6b88b9f4655a63b0b8488c2e2f2a37048 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Wed, 23 Jul 2025 13:44:29 +0200 Subject: [PATCH 3/8] Cleanup. --- .../test/GridRaceConditionTest.cs | 75 +++++-------------- 1 file changed, 20 insertions(+), 55 deletions(-) diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs index 0e87b1d07822..ad2fded7c47a 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs @@ -95,10 +95,14 @@ internal class Person public string Name { get; set; } = string.Empty; } -internal class SimpleTestComponent : ComponentBase, IAsyncDisposable +internal abstract class BaseTestComponent : ComponentBase, IAsyncDisposable + where TGrid : ComponentBase { [Inject] public IJSRuntime JSRuntime { get; set; } = default!; + protected TGrid _grid; + public TGrid Grid => _grid; + private readonly List _people = [ new() { Id = 1, Name = "John" }, new() { Id = 2, Name = "Jane" } @@ -106,7 +110,7 @@ internal class SimpleTestComponent : ComponentBase, IAsyncDisposable protected override void BuildRenderTree(RenderTreeBuilder builder) { - builder.OpenComponent>(0); + builder.OpenComponent(0); builder.AddAttribute(1, "Items", _people.AsQueryable()); builder.AddAttribute(2, "ChildContent", (RenderFragment)(b => { @@ -114,60 +118,36 @@ protected override void BuildRenderTree(RenderTreeBuilder builder) b.AddAttribute(1, "Property", (System.Linq.Expressions.Expression>)(p => p.Id)); b.CloseComponent(); })); - builder.CloseComponent(); - } - - public ValueTask DisposeAsync() - { - if (JSRuntime is TestJsRuntime testRuntime) + if (typeof(TGrid) != typeof(QuickGrid)) { - testRuntime.MarkDisposed(); + builder.AddComponentReferenceCapture(3, component => _grid = (TGrid)component); } - return ValueTask.CompletedTask; - } -} - -internal class FailingGridTestComponent : ComponentBase, IAsyncDisposable -{ - [Inject] public IJSRuntime JSRuntime { get; set; } = default!; - - private FailingQuickGrid _failingGrid; - - public FailingQuickGrid FailingQuickGrid => _failingGrid; - - private readonly List _people = [ - new() { Id = 1, Name = "John" }, - new() { Id = 2, Name = "Jane" } - ]; - - protected override void BuildRenderTree(RenderTreeBuilder builder) - { - builder.OpenComponent>(0); - builder.AddAttribute(1, "Items", _people.AsQueryable()); - builder.AddAttribute(2, "ChildContent", (RenderFragment)(b => - { - b.OpenComponent>(0); - b.AddAttribute(1, "Property", (System.Linq.Expressions.Expression>)(p => p.Id)); - b.CloseComponent(); - })); - builder.AddComponentReferenceCapture(3, component => _failingGrid = (FailingQuickGrid)component); builder.CloseComponent(); } - public async ValueTask DisposeAsync() + public virtual async ValueTask DisposeAsync() { if (JSRuntime is TestJsRuntime testRuntime) { testRuntime.MarkDisposed(); } - if (_failingGrid is not null) + if (_grid is IAsyncDisposable disposableGrid) { - await _failingGrid.DisposeAsync(); + await disposableGrid.DisposeAsync(); } } } +internal class SimpleTestComponent : BaseTestComponent> +{ +} + +internal class FailingGridTestComponent : BaseTestComponent> +{ + public FailingQuickGrid FailingQuickGrid => Grid; +} + internal class TestJsRuntime(TaskCompletionSource moduleCompletion, TaskCompletionSource importStarted) : IJSRuntime { private readonly TaskCompletionSource _moduleCompletion = moduleCompletion; @@ -207,9 +187,6 @@ public ValueTask InvokeAsync(string identifier, CancellationToke InvokeAsync(identifier, args); } -/// -/// Mock JS object reference for the QuickGrid module -/// internal class TestJSObjectReference(TestJsRuntime jsRuntime) : IJSObjectReference { private readonly TestJsRuntime _jsRuntime = jsRuntime; @@ -223,20 +200,8 @@ public ValueTask InvokeAsync(string identifier, object[] args) return ValueTask.FromResult(default(TValue)!); } - public ValueTask InvokeVoidAsync(string identifier, object[] args) - { - if (identifier == "init") - { - _jsRuntime.RecordInitCall(); - } - return ValueTask.CompletedTask; - } - public ValueTask InvokeAsync(string identifier, CancellationToken cancellationToken, object[] args) => InvokeAsync(identifier, args); - public ValueTask InvokeVoidAsync(string identifier, CancellationToken cancellationToken, object[] args) => - InvokeVoidAsync(identifier, args); - public ValueTask DisposeAsync() => ValueTask.CompletedTask; } From 32d0698c61c34567a7ee494f6908ef995fb5df2d Mon Sep 17 00:00:00 2001 From: Daria Tiurina Date: Wed, 23 Jul 2025 15:31:35 +0200 Subject: [PATCH 4/8] Fixes and feedback --- .../src/QuickGrid.razor.cs | 8 ++-- .../test/FailingQuickGrid.cs | 15 ++---- .../test/GridRaceConditionTest.cs | 46 +++++++++---------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs index 8dcfd7430f21..fa20650559b8 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/src/QuickGrid.razor.cs @@ -152,7 +152,8 @@ public partial class QuickGrid : IAsyncDisposable // If the PaginationState mutates, it raises this event. We use it to trigger a re-render. private readonly EventCallbackSubscriber _currentPageItemsChanged; - private bool _disposeBool; + // If the QuickGrid is disposed while the JS module is being loaded, we need to avoid calling JS methods + private bool _wasDisposed; /// /// Constructs an instance of . @@ -208,9 +209,10 @@ protected override async Task OnAfterRenderAsync(bool firstRender) if (firstRender) { _jsModule = await JS.InvokeAsync("import", "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js"); - if (_disposeBool) + if (_wasDisposed) { // If the component has been disposed while JS module was being loaded, we don't need to continue + await _jsModule.DisposeAsync(); return; } _jsEventDisposable = await _jsModule.InvokeAsync("init", _tableReference); @@ -441,7 +443,7 @@ private string GridClass() /// public async ValueTask DisposeAsync() { - _disposeBool = true; + _wasDisposed = true; _currentPageItemsChanged.Dispose(); try diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs index c9f141944065..b6c8801e78e8 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs @@ -6,7 +6,7 @@ using Microsoft.JSInterop; using System.Reflection; -namespace Microsoft.AspNetCore.Components.QuickGrid.Test; +namespace Microsoft.AspNetCore.Components.QuickGrid.Tests; /// /// A QuickGrid implementation that simulates the behavior before the race condition fix. @@ -55,9 +55,9 @@ async ValueTask IAsyncDisposable.DisposeAsync() /// Check if _disposeBool is false, proving we didn't call base.DisposeAsync(). /// This is used by tests to verify that our simulation is working correctly. /// - public bool IsDisposeBoolFalse() + public bool IsWasDisposedFalse() { - var field = typeof(QuickGrid).GetField("_disposeBool", BindingFlags.NonPublic | BindingFlags.Instance); + var field = typeof(QuickGrid).GetField("_wasDisposed", BindingFlags.NonPublic | BindingFlags.Instance); return field?.GetValue(this) is false; } @@ -71,19 +71,12 @@ protected override async Task OnAfterRenderAsync(bool firstRender) { if (firstRender) { - // Get the IJSRuntime (same as base class) if (JS != null) { // Import the JS module (this will trigger our TestJsRuntime's import logic) var jsModule = await JS.InvokeAsync("import", "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js"); - // THE KEY DIFFERENCE: The original code did NOT check _disposeBool here - // The fix added: if (_disposeBool) return; - // By omitting this check, we demonstrate the race condition where init gets called on disposed components - - // Call init - this happens even if component was disposed during import - // For our test, we don't need a real table reference, just need to trigger the JS call await jsModule.InvokeAsync("init", new object()); // Signal completion only after the init call has completed, and only once @@ -106,4 +99,4 @@ protected override async Task OnAfterRenderAsync(bool firstRender) } } } -} \ No newline at end of file +} diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs index ad2fded7c47a..c38e08fc9c0b 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs @@ -6,8 +6,9 @@ using Microsoft.AspNetCore.Components.Test.Helpers; using Microsoft.Extensions.DependencyInjection; using Microsoft.JSInterop; +using Xunit.Sdk; -namespace Microsoft.AspNetCore.Components.QuickGrid.Test; +namespace Microsoft.AspNetCore.Components.QuickGrid.Tests; public class GridRaceConditionTest { @@ -33,14 +34,16 @@ public async Task CanCorrectlyDisposeAsync() // Dispose component while JS module loading is pending testJsRuntime.MarkDisposed(); - await testComponent.DisposeAsync(); + await renderer.DisposeAsync(); // Complete the JS module loading moduleLoadCompletion.SetResult(); - // Assert that init was not called after disposal (the fix working correctly) + // Assert that init was not called after disposal Assert.False(testJsRuntime.InitWasCalledAfterDisposal, - "Init should not be called on a disposed component - this indicates the race condition fix is not working"); + "Init should not be called on a disposed component."); + await Task.Yield(); + Assert.True(testJsRuntime.JsModuleDisposed); } [Fact] @@ -64,13 +67,13 @@ public async Task FailingQuickGrid_CallsInitAfterDisposal_DemonstratesRaceCondit // Dispose component while JS module loading is pending testJsRuntime.MarkDisposed(); - await testComponent.DisposeAsync(); + await renderer.DisposeAsync(); // Verify our FailingQuickGrid's DisposeAsync was called and _disposeBool should still be false var failingGrid = testComponent.FailingQuickGrid; Assert.NotNull(failingGrid); Assert.True(failingGrid.DisposeAsyncWasCalled, "FailingQuickGrid.DisposeAsync should have been called"); - Assert.True(failingGrid.IsDisposeBoolFalse(), "_disposeBool should still be false since we didn't call base.DisposeAsync()"); + Assert.True(failingGrid.IsWasDisposedFalse(), "_wasDisposed should still be false since we didn't call base.DisposeAsync()"); // Complete the JS module loading - this allows the FailingQuickGrid's OnAfterRenderAsync to continue // and demonstrate the race condition by calling init after disposal @@ -85,7 +88,8 @@ public async Task FailingQuickGrid_CallsInitAfterDisposal_DemonstratesRaceCondit $"FailingQuickGrid should call init after disposal, demonstrating the race condition bug. " + $"InitWasCalledAfterDisposal: {testJsRuntime.InitWasCalledAfterDisposal}, " + $"DisposeAsyncWasCalled: {failingGrid.DisposeAsyncWasCalled}, " + - $"_disposeBool is false: {failingGrid.IsDisposeBoolFalse()}"); + $"_disposeBool is false: {failingGrid.IsWasDisposedFalse()}"); + Assert.False(testJsRuntime.JsModuleDisposed); } } @@ -95,7 +99,7 @@ internal class Person public string Name { get; set; } = string.Empty; } -internal abstract class BaseTestComponent : ComponentBase, IAsyncDisposable +internal abstract class BaseTestComponent : ComponentBase where TGrid : ComponentBase { [Inject] public IJSRuntime JSRuntime { get; set; } = default!; @@ -124,19 +128,6 @@ protected override void BuildRenderTree(RenderTreeBuilder builder) } builder.CloseComponent(); } - - public virtual async ValueTask DisposeAsync() - { - if (JSRuntime is TestJsRuntime testRuntime) - { - testRuntime.MarkDisposed(); - } - - if (_grid is IAsyncDisposable disposableGrid) - { - await disposableGrid.DisposeAsync(); - } - } } internal class SimpleTestComponent : BaseTestComponent> @@ -154,6 +145,8 @@ internal class TestJsRuntime(TaskCompletionSource moduleCompletion, TaskCompleti private readonly TaskCompletionSource _importStarted = importStarted; private bool _disposed; + public bool JsModuleDisposed { get; private set; } + public bool InitWasCalledAfterDisposal { get; private set; } public async ValueTask InvokeAsync(string identifier, object[] args = null) @@ -166,8 +159,6 @@ public async ValueTask InvokeAsync(string identifier, object[] a // Wait for test to control when import completes await _moduleCompletion.Task; - - // Return a mock IJSObjectReference return (TValue)(object)new TestJSObjectReference(this); } throw new InvalidOperationException($"Unexpected JS call: {identifier}"); @@ -175,6 +166,8 @@ public async ValueTask InvokeAsync(string identifier, object[] a public void MarkDisposed() => _disposed = true; + public void MarkJsModuleDisposed() => JsModuleDisposed = true; + public void RecordInitCall() { if (_disposed) @@ -197,11 +190,14 @@ public ValueTask InvokeAsync(string identifier, object[] args) { _jsRuntime.RecordInitCall(); } - return ValueTask.FromResult(default(TValue)!); + return default!; } public ValueTask InvokeAsync(string identifier, CancellationToken cancellationToken, object[] args) => InvokeAsync(identifier, args); - public ValueTask DisposeAsync() => ValueTask.CompletedTask; + public ValueTask DisposeAsync() { + _jsRuntime.MarkJsModuleDisposed(); + return default!; + } } From a4f306506e8b51d463d7ee59573ebd512f2b5a72 Mon Sep 17 00:00:00 2001 From: Daria Tiurina Date: Thu, 24 Jul 2025 10:41:12 +0200 Subject: [PATCH 5/8] Fixes --- .../test/FailingQuickGrid.cs | 5 -- .../test/GridRaceConditionTest.cs | 27 +++++----- .../test/NotFailingQuickGrid.cs | 49 +++++++++++++++++++ 3 files changed, 61 insertions(+), 20 deletions(-) create mode 100644 src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs index b6c8801e78e8..100bfa497c99 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs @@ -35,11 +35,6 @@ internal class FailingQuickGrid : QuickGrid, IAsyncDisposa public new async ValueTask DisposeAsync() { DisposeAsyncWasCalled = true; - // Intentionally do nothing to prevent _disposeBool from being set to true - // This means the OnAfterRenderAsync method will not detect that the component is disposed - // and will proceed to call init() even after disposal, demonstrating the race condition - - // DO NOT call base.DisposeAsync() - this is the key to simulating the race condition await Task.CompletedTask; } diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs index c38e08fc9c0b..36c1a9256776 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs @@ -39,15 +39,18 @@ public async Task CanCorrectlyDisposeAsync() // Complete the JS module loading moduleLoadCompletion.SetResult(); - // Assert that init was not called after disposal + // Wait until after OnAfterRenderAsync has completed to test the disposal of the jsModule + var notFailingGrid = testComponent.NotFailingGrid; + await notFailingGrid.OnAfterRenderCompleted; + + // Assert that init was not called after disposal and JsModule was disposed of Assert.False(testJsRuntime.InitWasCalledAfterDisposal, "Init should not be called on a disposed component."); - await Task.Yield(); Assert.True(testJsRuntime.JsModuleDisposed); } [Fact] - public async Task FailingQuickGrid_CallsInitAfterDisposal_DemonstratesRaceCondition() + public async Task FailingQuickGridCallsInitAfterDisposal() { var moduleLoadCompletion = new TaskCompletionSource(); var moduleImportStarted = new TaskCompletionSource(); @@ -69,21 +72,17 @@ public async Task FailingQuickGrid_CallsInitAfterDisposal_DemonstratesRaceCondit testJsRuntime.MarkDisposed(); await renderer.DisposeAsync(); - // Verify our FailingQuickGrid's DisposeAsync was called and _disposeBool should still be false - var failingGrid = testComponent.FailingQuickGrid; - Assert.NotNull(failingGrid); - Assert.True(failingGrid.DisposeAsyncWasCalled, "FailingQuickGrid.DisposeAsync should have been called"); - Assert.True(failingGrid.IsWasDisposedFalse(), "_wasDisposed should still be false since we didn't call base.DisposeAsync()"); - // Complete the JS module loading - this allows the FailingQuickGrid's OnAfterRenderAsync to continue // and demonstrate the race condition by calling init after disposal moduleLoadCompletion.SetResult(); // Wait for OnAfterRenderAsync to complete - deterministic timing instead of arbitrary delay + var failingGrid = testComponent.FailingQuickGrid; await failingGrid.OnAfterRenderCompleted; - // Assert that init WAS called after disposal (demonstrating the race condition bug) + // Assert that init WAS called after disposal // The FailingQuickGrid's OnAfterRenderAsync should have called init despite being disposed + // The FailingQuickGrid should not have disposed of JsModule Assert.True(testJsRuntime.InitWasCalledAfterDisposal, $"FailingQuickGrid should call init after disposal, demonstrating the race condition bug. " + $"InitWasCalledAfterDisposal: {testJsRuntime.InitWasCalledAfterDisposal}, " + @@ -122,16 +121,14 @@ protected override void BuildRenderTree(RenderTreeBuilder builder) b.AddAttribute(1, "Property", (System.Linq.Expressions.Expression>)(p => p.Id)); b.CloseComponent(); })); - if (typeof(TGrid) != typeof(QuickGrid)) - { - builder.AddComponentReferenceCapture(3, component => _grid = (TGrid)component); - } + builder.AddComponentReferenceCapture(3, component => _grid = (TGrid)component); builder.CloseComponent(); } } -internal class SimpleTestComponent : BaseTestComponent> +internal class SimpleTestComponent : BaseTestComponent> { + public NotFailingGrid NotFailingGrid => Grid; } internal class FailingGridTestComponent : BaseTestComponent> diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs new file mode 100644 index 000000000000..fd96fb290ee9 --- /dev/null +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Text; +using Microsoft.JSInterop; + +namespace Microsoft.AspNetCore.Components.QuickGrid.Tests; +/// +/// A QuickGrid implementation that uses the same implementation of the basic QuickGrid with the additions of the OnAfterRenderCompleted task. +/// +/// /// The type of data represented by each row in the grid. +internal class NotFailingGrid : QuickGrid +{ + [Inject] private IJSRuntime JS { get; set; } = default!; + + private readonly TaskCompletionSource _onAfterRenderCompleted = new(); + + private bool _completionSignaled; + + /// + /// Task that completes when OnAfterRenderAsync has finished executing. + /// This allows tests to wait deterministically for the race condition to occur. + /// + public Task OnAfterRenderCompleted => _onAfterRenderCompleted.Task; + + protected override async Task OnAfterRenderAsync(bool firstRender) + { + try + { + if (firstRender) + { + await base.OnAfterRenderAsync(firstRender); + _onAfterRenderCompleted.TrySetResult(); + _completionSignaled = true; + return; + } + } + finally + { + if (firstRender && _completionSignaled) + { + _onAfterRenderCompleted.TrySetResult(); + _completionSignaled = true; + } + } + } +} From 540c4fb91bce952ee5e9291aaa4255bfcb865ea2 Mon Sep 17 00:00:00 2001 From: Daria Tiurina Date: Thu, 24 Jul 2025 16:53:28 +0200 Subject: [PATCH 6/8] Feedback --- .../test/NotFailingQuickGrid.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs index fd96fb290ee9..90453060702a 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs @@ -32,14 +32,18 @@ protected override async Task OnAfterRenderAsync(bool firstRender) if (firstRender) { await base.OnAfterRenderAsync(firstRender); - _onAfterRenderCompleted.TrySetResult(); - _completionSignaled = true; + if (!_completionSignaled) + { + _completionSignaled = true; + _onAfterRenderCompleted.TrySetResult(); + } + ; return; } } finally { - if (firstRender && _completionSignaled) + if (firstRender && !_completionSignaled) { _onAfterRenderCompleted.TrySetResult(); _completionSignaled = true; From 274e71b2f91a9439ae7faf877411317ce29366b3 Mon Sep 17 00:00:00 2001 From: Daria Tiurina Date: Fri, 25 Jul 2025 09:51:12 +0200 Subject: [PATCH 7/8] Update src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs Co-authored-by: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> --- .../test/GridRaceConditionTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs index 36c1a9256776..ffa4e6d3b409 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/GridRaceConditionTest.cs @@ -76,7 +76,7 @@ public async Task FailingQuickGridCallsInitAfterDisposal() // and demonstrate the race condition by calling init after disposal moduleLoadCompletion.SetResult(); - // Wait for OnAfterRenderAsync to complete - deterministic timing instead of arbitrary delay + // Wait until after OnAfterRenderAsync has completed, to make sure jsmodule import started and the reported issue is reproduced var failingGrid = testComponent.FailingQuickGrid; await failingGrid.OnAfterRenderCompleted; From d885d72164d362d37c70edf0beebd4c2c836fedb Mon Sep 17 00:00:00 2001 From: Daria Tiurina Date: Fri, 25 Jul 2025 10:02:59 +0200 Subject: [PATCH 8/8] Feedback --- .../test/FailingQuickGrid.cs | 14 +------------- .../test/NotFailingQuickGrid.cs | 12 +----------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs index 100bfa497c99..d7715af78e57 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/FailingQuickGrid.cs @@ -18,7 +18,6 @@ internal class FailingQuickGrid : QuickGrid, IAsyncDisposa [Inject] private IJSRuntime JS { get; set; } = default!; private readonly TaskCompletionSource _onAfterRenderCompleted = new(); - private bool _completionSignaled; public bool DisposeAsyncWasCalled { get; private set; } @@ -71,25 +70,14 @@ protected override async Task OnAfterRenderAsync(bool firstRender) // Import the JS module (this will trigger our TestJsRuntime's import logic) var jsModule = await JS.InvokeAsync("import", "./_content/Microsoft.AspNetCore.Components.QuickGrid/QuickGrid.razor.js"); - await jsModule.InvokeAsync("init", new object()); - - // Signal completion only after the init call has completed, and only once - if (!_completionSignaled) - { - _completionSignaled = true; - _onAfterRenderCompleted.TrySetResult(); - } - return; } } } finally { - // Only signal completion if we haven't already done it and this is the first render - if (firstRender && !_completionSignaled) + if (firstRender) { - _completionSignaled = true; _onAfterRenderCompleted.TrySetResult(); } } diff --git a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs index 90453060702a..b84d8c7b66fd 100644 --- a/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs +++ b/src/Components/QuickGrid/Microsoft.AspNetCore.Components.QuickGrid/test/NotFailingQuickGrid.cs @@ -17,8 +17,6 @@ internal class NotFailingGrid : QuickGrid private readonly TaskCompletionSource _onAfterRenderCompleted = new(); - private bool _completionSignaled; - /// /// Task that completes when OnAfterRenderAsync has finished executing. /// This allows tests to wait deterministically for the race condition to occur. @@ -32,21 +30,13 @@ protected override async Task OnAfterRenderAsync(bool firstRender) if (firstRender) { await base.OnAfterRenderAsync(firstRender); - if (!_completionSignaled) - { - _completionSignaled = true; - _onAfterRenderCompleted.TrySetResult(); - } - ; - return; } } finally { - if (firstRender && !_completionSignaled) + if (firstRender) { _onAfterRenderCompleted.TrySetResult(); - _completionSignaled = true; } } }