From 7cd0c332500c235d103f76d42d1cf034d16e6de0 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 12:13:06 +0100 Subject: [PATCH 1/9] Unit test that reproduces the problem. --- .../test/OwningComponentBaseTest.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/Components/Components/test/OwningComponentBaseTest.cs b/src/Components/Components/test/OwningComponentBaseTest.cs index ae9a2b4a67ac..3fca2950e470 100644 --- a/src/Components/Components/test/OwningComponentBaseTest.cs +++ b/src/Components/Components/test/OwningComponentBaseTest.cs @@ -111,6 +111,33 @@ public MyService(Counter counter) void IDisposable.Dispose() => Counter.DisposedCount++; } + [Fact] + public async Task DisposeAsync_CallsDispose_WithDisposingTrue() + { + var services = new ServiceCollection(); + services.AddTransient(); + var serviceProvider = services.BuildServiceProvider(); + + var renderer = new TestRenderer(serviceProvider); + var component = (ComponentWithDispose)renderer.InstantiateComponent(); + + _ = component.MyService; + await ((IAsyncDisposable)component).DisposeAsync(); + Assert.True(component.DisposingParameter); + } + + private class ComponentWithDispose : OwningComponentBase + { + public MyService MyService => Service; + public bool? DisposingParameter { get; private set; } + + protected override void Dispose(bool disposing) + { + DisposingParameter = disposing; + base.Dispose(disposing); + } + } + private class MyOwningComponent : OwningComponentBase { public MyService MyService => Service; From 5a90f4ae16c711ae50123a28114f1358b31c5860 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 12:17:44 +0100 Subject: [PATCH 2/9] Missing commit for test. --- src/Components/Components/test/OwningComponentBaseTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Components/Components/test/OwningComponentBaseTest.cs b/src/Components/Components/test/OwningComponentBaseTest.cs index 3fca2950e470..937024cf7d0b 100644 --- a/src/Components/Components/test/OwningComponentBaseTest.cs +++ b/src/Components/Components/test/OwningComponentBaseTest.cs @@ -115,6 +115,7 @@ public MyService(Counter counter) public async Task DisposeAsync_CallsDispose_WithDisposingTrue() { var services = new ServiceCollection(); + services.AddSingleton(); services.AddTransient(); var serviceProvider = services.BuildServiceProvider(); From af69922e8773dff9db7a07f4b11d31d859513f26 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 12:22:50 +0100 Subject: [PATCH 3/9] Fix. --- .../Components/src/OwningComponentBase.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Components/Components/src/OwningComponentBase.cs b/src/Components/Components/src/OwningComponentBase.cs index b2e9d9ef554a..6768b9589c2f 100644 --- a/src/Components/Components/src/OwningComponentBase.cs +++ b/src/Components/Components/src/OwningComponentBase.cs @@ -44,7 +44,7 @@ protected IServiceProvider ScopedServices } } - /// + /// void IDisposable.Dispose() { Dispose(disposing: true); @@ -69,12 +69,15 @@ protected virtual void Dispose(bool disposing) } } - /// + /// async ValueTask IAsyncDisposable.DisposeAsync() { - await DisposeAsyncCore().ConfigureAwait(false); - - Dispose(disposing: false); + if (!IsDisposed) + { + await DisposeAsyncCore().ConfigureAwait(false); + + Dispose(disposing: true); + } GC.SuppressFinalize(this); } @@ -84,13 +87,11 @@ async ValueTask IAsyncDisposable.DisposeAsync() /// A task that represents the asynchronous dispose operation. protected virtual async ValueTask DisposeAsyncCore() { - if (!IsDisposed && _scope.HasValue) + if (_scope.HasValue) { await _scope.Value.DisposeAsync().ConfigureAwait(false); _scope = null; } - - IsDisposed = true; } } From 71ceda1cb5bacb4ffac7d3c797fdad56cd7bc831 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Mon, 8 Dec 2025 13:37:30 +0100 Subject: [PATCH 4/9] Update src/Components/Components/src/OwningComponentBase.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/Components/Components/src/OwningComponentBase.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Components/Components/src/OwningComponentBase.cs b/src/Components/Components/src/OwningComponentBase.cs index 6768b9589c2f..d25e12ee2ee4 100644 --- a/src/Components/Components/src/OwningComponentBase.cs +++ b/src/Components/Components/src/OwningComponentBase.cs @@ -75,7 +75,6 @@ async ValueTask IAsyncDisposable.DisposeAsync() if (!IsDisposed) { await DisposeAsyncCore().ConfigureAwait(false); - Dispose(disposing: true); } GC.SuppressFinalize(this); From ed3c36576bc0266c6805b00124e01e77dba41d6d Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 14:35:27 +0100 Subject: [PATCH 5/9] Feedback: restore the check to make sure we don't break net10 users. --- src/Components/Components/src/OwningComponentBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/Components/src/OwningComponentBase.cs b/src/Components/Components/src/OwningComponentBase.cs index d25e12ee2ee4..f633dffc5f45 100644 --- a/src/Components/Components/src/OwningComponentBase.cs +++ b/src/Components/Components/src/OwningComponentBase.cs @@ -86,7 +86,7 @@ async ValueTask IAsyncDisposable.DisposeAsync() /// A task that represents the asynchronous dispose operation. protected virtual async ValueTask DisposeAsyncCore() { - if (_scope.HasValue) + if (!IsDisposed && _scope.HasValue) { await _scope.Value.DisposeAsync().ConfigureAwait(false); _scope = null; From b6959c689a6a5f87e5e150a8301e6ef06bfcdc04 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 14:36:18 +0100 Subject: [PATCH 6/9] Feedback: DisposeAsyncCore might throw. --- src/Components/Components/src/OwningComponentBase.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Components/Components/src/OwningComponentBase.cs b/src/Components/Components/src/OwningComponentBase.cs index f633dffc5f45..a60f396e89ec 100644 --- a/src/Components/Components/src/OwningComponentBase.cs +++ b/src/Components/Components/src/OwningComponentBase.cs @@ -74,8 +74,14 @@ async ValueTask IAsyncDisposable.DisposeAsync() { if (!IsDisposed) { - await DisposeAsyncCore().ConfigureAwait(false); - Dispose(disposing: true); + try + { + await DisposeAsyncCore().ConfigureAwait(false); + } + finally + { + Dispose(disposing: true); + } } GC.SuppressFinalize(this); } From a2c79c01eb5973278d2a57635436f5801060cb85 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 14:44:24 +0100 Subject: [PATCH 7/9] Cover exception and double async/sync disposal with unit tests. --- .../test/OwningComponentBaseTest.cs | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/Components/Components/test/OwningComponentBaseTest.cs b/src/Components/Components/test/OwningComponentBaseTest.cs index 937024cf7d0b..a83079f30721 100644 --- a/src/Components/Components/test/OwningComponentBaseTest.cs +++ b/src/Components/Components/test/OwningComponentBaseTest.cs @@ -127,10 +127,79 @@ public async Task DisposeAsync_CallsDispose_WithDisposingTrue() Assert.True(component.DisposingParameter); } + [Fact] + public async Task DisposeAsync_ThenDispose_IsIdempotent() + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddTransient(); + var serviceProvider = services.BuildServiceProvider(); + + var counter = serviceProvider.GetRequiredService(); + var renderer = new TestRenderer(serviceProvider); + var component = (ComponentWithDispose)renderer.InstantiateComponent(); + + _ = component.MyService; + + // First disposal via DisposeAsync + await ((IAsyncDisposable)component).DisposeAsync(); + var firstCallCount = component.DisposeCallCount; + Assert.Equal(1, counter.DisposedCount); + + // Second disposal via Dispose - user override is called but base class prevents double-disposal + ((IDisposable)component).Dispose(); + // User override is called again, but base.Dispose() returns early due to IsDisposed check + Assert.True(component.DisposeCallCount >= firstCallCount); // Override may be called, but... + Assert.Equal(1, counter.DisposedCount); // ...service should only be disposed once + } + + [Fact] + public async Task DisposeAsyncCore_Override_WithException_StillCallsDispose() + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddTransient(); + var serviceProvider = services.BuildServiceProvider(); + + var renderer = new TestRenderer(serviceProvider); + var component = (ComponentWithThrowingDisposeAsyncCore)renderer.InstantiateComponent(); + + _ = component.MyService; + + // Even if DisposeAsyncCore throws, Dispose(true) should still be called + await Assert.ThrowsAsync(async () => + await ((IAsyncDisposable)component).DisposeAsync()); + + // Dispose should have been called due to try-finally + Assert.True(component.DisposingParameter); + Assert.True(component.IsDisposedPublic); + } + private class ComponentWithDispose : OwningComponentBase { public MyService MyService => Service; public bool? DisposingParameter { get; private set; } + public int DisposeCallCount { get; private set; } + + protected override void Dispose(bool disposing) + { + DisposingParameter = disposing; + DisposeCallCount++; + base.Dispose(disposing); + } + } + + private class ComponentWithThrowingDisposeAsyncCore : OwningComponentBase + { + public MyService MyService => Service; + public bool? DisposingParameter { get; private set; } + public bool IsDisposedPublic => IsDisposed; + + protected override async ValueTask DisposeAsyncCore() + { + await base.DisposeAsyncCore(); + throw new InvalidOperationException("Something went wrong in async disposal"); + } protected override void Dispose(bool disposing) { From e4de7a004f0c99a65818d29a85d0e81557733486 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 14:51:02 +0100 Subject: [PATCH 8/9] Feedback: Add more non-trivial cases tests. --- .../test/OwningComponentBaseTest.cs | 98 +++++++++++++++++-- 1 file changed, 90 insertions(+), 8 deletions(-) diff --git a/src/Components/Components/test/OwningComponentBaseTest.cs b/src/Components/Components/test/OwningComponentBaseTest.cs index a83079f30721..4c42a84279dd 100644 --- a/src/Components/Components/test/OwningComponentBaseTest.cs +++ b/src/Components/Components/test/OwningComponentBaseTest.cs @@ -141,16 +141,13 @@ public async Task DisposeAsync_ThenDispose_IsIdempotent() _ = component.MyService; - // First disposal via DisposeAsync await ((IAsyncDisposable)component).DisposeAsync(); var firstCallCount = component.DisposeCallCount; Assert.Equal(1, counter.DisposedCount); - // Second disposal via Dispose - user override is called but base class prevents double-disposal ((IDisposable)component).Dispose(); - // User override is called again, but base.Dispose() returns early due to IsDisposed check - Assert.True(component.DisposeCallCount >= firstCallCount); // Override may be called, but... - Assert.Equal(1, counter.DisposedCount); // ...service should only be disposed once + Assert.True(component.DisposeCallCount >= firstCallCount); + Assert.Equal(1, counter.DisposedCount); } [Fact] @@ -166,11 +163,8 @@ public async Task DisposeAsyncCore_Override_WithException_StillCallsDispose() _ = component.MyService; - // Even if DisposeAsyncCore throws, Dispose(true) should still be called - await Assert.ThrowsAsync(async () => await ((IAsyncDisposable)component).DisposeAsync()); - // Dispose should have been called due to try-finally Assert.True(component.DisposingParameter); Assert.True(component.IsDisposedPublic); } @@ -215,4 +209,92 @@ private class MyOwningComponent : OwningComponentBase // Expose IsDisposed for testing public bool IsDisposedPublic => IsDisposed; } + + [Fact] + public async Task ComplexComponent_DisposesResourcesOnlyWhenDisposingIsTrue() + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddTransient(); + var serviceProvider = services.BuildServiceProvider(); + + var renderer = new TestRenderer(serviceProvider); + var component = (ComplexComponent)renderer.InstantiateComponent(); + + _ = component.MyService; + + await ((IAsyncDisposable)component).DisposeAsync(); + + // Verify all managed resources were disposed because disposing=true + Assert.True(component.TimerDisposed); + Assert.True(component.CancellationTokenSourceDisposed); + Assert.True(component.EventUnsubscribed); + Assert.Equal(1, component.ManagedResourcesCleanedUpCount); + } + + [Fact] + public void ComplexComponent_WithDisposingFalse_SkipsManagedResourceCleanup() + { + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddTransient(); + var serviceProvider = services.BuildServiceProvider(); + + var renderer = new TestRenderer(serviceProvider); + var component = (ComplexComponent)renderer.InstantiateComponent(); + + _ = component.MyService; + + component.TestDisposeWithFalse(); + + Assert.False(component.TimerDisposed); + Assert.False(component.CancellationTokenSourceDisposed); + Assert.False(component.EventUnsubscribed); + Assert.Equal(0, component.ManagedResourcesCleanedUpCount); + } + + private class ComplexComponent : OwningComponentBase + { + private readonly System.Threading.Timer _timer; + private readonly CancellationTokenSource _cts; + private bool _eventSubscribed; + + public MyService MyService => Service; + public bool TimerDisposed { get; private set; } + public bool CancellationTokenSourceDisposed { get; private set; } + public bool EventUnsubscribed { get; private set; } + public int ManagedResourcesCleanedUpCount { get; private set; } + + public ComplexComponent() + { + _timer = new System.Threading.Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + _cts = new CancellationTokenSource(); + _eventSubscribed = true; + } + + public void TestDisposeWithFalse() => Dispose(disposing: false); + + protected override void Dispose(bool disposing) + { + if (disposing) + { + _timer?.Dispose(); + TimerDisposed = true; + + _cts?.Cancel(); + _cts?.Dispose(); + CancellationTokenSourceDisposed = true; + + if (_eventSubscribed) + { + EventUnsubscribed = true; + _eventSubscribed = false; + } + + ManagedResourcesCleanedUpCount++; + } + + base.Dispose(disposing); + } + } } From 012d1bb099485f6d4051917e80b9b7d50c9eddb5 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Mon, 8 Dec 2025 14:58:24 +0100 Subject: [PATCH 9/9] Remove unit testing the bug buggy behavior. --- .../test/OwningComponentBaseTest.cs | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/src/Components/Components/test/OwningComponentBaseTest.cs b/src/Components/Components/test/OwningComponentBaseTest.cs index 4c42a84279dd..8064f75cb2c3 100644 --- a/src/Components/Components/test/OwningComponentBaseTest.cs +++ b/src/Components/Components/test/OwningComponentBaseTest.cs @@ -163,6 +163,7 @@ public async Task DisposeAsyncCore_Override_WithException_StillCallsDispose() _ = component.MyService; + await Assert.ThrowsAsync(async () => await ((IAsyncDisposable)component).DisposeAsync()); Assert.True(component.DisposingParameter); @@ -232,27 +233,6 @@ public async Task ComplexComponent_DisposesResourcesOnlyWhenDisposingIsTrue() Assert.Equal(1, component.ManagedResourcesCleanedUpCount); } - [Fact] - public void ComplexComponent_WithDisposingFalse_SkipsManagedResourceCleanup() - { - var services = new ServiceCollection(); - services.AddSingleton(); - services.AddTransient(); - var serviceProvider = services.BuildServiceProvider(); - - var renderer = new TestRenderer(serviceProvider); - var component = (ComplexComponent)renderer.InstantiateComponent(); - - _ = component.MyService; - - component.TestDisposeWithFalse(); - - Assert.False(component.TimerDisposed); - Assert.False(component.CancellationTokenSourceDisposed); - Assert.False(component.EventUnsubscribed); - Assert.Equal(0, component.ManagedResourcesCleanedUpCount); - } - private class ComplexComponent : OwningComponentBase { private readonly System.Threading.Timer _timer; @@ -272,8 +252,6 @@ public ComplexComponent() _eventSubscribed = true; } - public void TestDisposeWithFalse() => Dispose(disposing: false); - protected override void Dispose(bool disposing) { if (disposing)