From d937177e6b3dad81dc9e46af10157a1d15364c1a Mon Sep 17 00:00:00 2001
From: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Date: Tue, 10 Aug 2021 12:55:03 +0100
Subject: [PATCH] Dispatch dispose to sync context. Fixes #32411 (#35217)
---
.../Components/src/RenderTree/Renderer.cs | 25 ++++++--
.../Components/test/RendererTest.cs | 62 +++++++++++++++++--
2 files changed, 77 insertions(+), 10 deletions(-)
diff --git a/src/Components/Components/src/RenderTree/Renderer.cs b/src/Components/Components/src/RenderTree/Renderer.cs
index 66aa9e4908f..ed3bc5951b6 100644
--- a/src/Components/Components/src/RenderTree/Renderer.cs
+++ b/src/Components/Components/src/RenderTree/Renderer.cs
@@ -43,6 +43,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
private ulong _lastEventHandlerId;
private List<Task>? _pendingTasks;
private Task? _disposeTask;
+ private bool _rendererIsDisposed;
/// <summary>
/// Allows the caller to handle exceptions from the SynchronizationContext when one is available.
@@ -128,7 +129,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
/// <summary>
/// Gets whether the renderer has been disposed.
/// </summary>
- internal bool Disposed { get; private set; }
+ internal bool Disposed => _rendererIsDisposed;
private async void RenderRootComponentsOnHotReload()
{
@@ -582,9 +583,10 @@ namespace Microsoft.AspNetCore.Components.RenderTree
/// </summary>
protected virtual void ProcessPendingRender()
{
- if (Disposed)
+ if (_rendererIsDisposed)
{
- throw new ObjectDisposedException(nameof(Renderer), "Cannot process pending renders after the renderer has been disposed.");
+ // Once we're disposed, we'll disregard further attempts to render anything
+ return;
}
ProcessRenderQueue();
@@ -974,7 +976,17 @@ namespace Microsoft.AspNetCore.Components.RenderTree
/// <param name="disposing"><see langword="true"/> if this method is being invoked by <see cref="IDisposable.Dispose"/>, otherwise <see langword="false"/>.</param>
protected virtual void Dispose(bool disposing)
{
- Disposed = true;
+ if (!Dispatcher.CheckAccess())
+ {
+ // It's important that we only call the components' Dispose/DisposeAsync lifecycle methods
+ // on the sync context, like other lifecycle methods. In almost all cases we'd already be
+ // on the sync context here since DisposeAsync dispatches, but just in case someone is using
+ // Dispose directly, we'll dispatch and block.
+ Dispatcher.InvokeAsync(() => Dispose(disposing)).Wait();
+ return;
+ }
+
+ _rendererIsDisposed = true;
if (TestableMetadataUpdate.IsSupported)
{
@@ -1079,7 +1091,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
- if (Disposed)
+ if (_rendererIsDisposed)
{
return;
}
@@ -1090,7 +1102,8 @@ namespace Microsoft.AspNetCore.Components.RenderTree
}
else
{
- Dispose();
+ await Dispatcher.InvokeAsync(Dispose);
+
if (_disposeTask != null)
{
await _disposeTask;
diff --git a/src/Components/Components/test/RendererTest.cs b/src/Components/Components/test/RendererTest.cs
index 152c3a841d9..a2b5288acc2 100644
--- a/src/Components/Components/test/RendererTest.cs
+++ b/src/Components/Components/test/RendererTest.cs
@@ -3930,15 +3930,24 @@ namespace Microsoft.AspNetCore.Components.Test
}
[Fact]
- public void DisposingRenderer_RejectsAttemptsToStartMoreRenderBatches()
+ public void DisposingRenderer_DisregardsAttemptsToStartMoreRenderBatches()
{
// Arrange
var renderer = new TestRenderer();
+ var component = new TestComponent(builder =>
+ {
+ builder.OpenElement(0, "my element");
+ builder.AddContent(1, "some text");
+ builder.CloseElement();
+ });
+
+ // Act
+ renderer.AssignRootComponentId(component);
renderer.Dispose();
+ component.TriggerRender();
- // Act/Assert
- var ex = Assert.Throws<ObjectDisposedException>(() => renderer.ProcessPendingRender());
- Assert.Contains("Cannot process pending renders after the renderer has been disposed.", ex.Message);
+ // Assert
+ Assert.Empty(renderer.Batches);
}
[Fact]
@@ -4679,6 +4688,51 @@ namespace Microsoft.AspNetCore.Components.Test
Assert.Same(exception2, renderer.HandledExceptions[1]);
}
+ [Fact]
+ public void DisposeCallsComponentDisposeOnSyncContext()
+ {
+ // Arrange
+ var renderer = new TestRenderer();
+ var wasOnSyncContext = false;
+ var component = new DisposableComponent
+ {
+ DisposeAction = () =>
+ {
+ wasOnSyncContext = renderer.Dispatcher.CheckAccess();
+ }
+ };
+
+ // Act
+ var componentId = renderer.AssignRootComponentId(component);
+ renderer.Dispose();
+
+ // Assert
+ Assert.True(wasOnSyncContext);
+ }
+
+ [Fact]
+ public async Task DisposeAsyncCallsComponentDisposeAsyncOnSyncContext()
+ {
+ // Arrange
+ var renderer = new TestRenderer();
+ var wasOnSyncContext = false;
+ var component = new AsyncDisposableComponent
+ {
+ AsyncDisposeAction = () =>
+ {
+ wasOnSyncContext = renderer.Dispatcher.CheckAccess();
+ return ValueTask.CompletedTask;
+ }
+ };
+
+ // Act
+ var componentId = renderer.AssignRootComponentId(component);
+ await renderer.DisposeAsync();
+
+ // Assert
+ Assert.True(wasOnSyncContext);
+ }
+
private class TestComponentActivator<TResult> : IComponentActivator where TResult : IComponent, new()
{
public List<Type> RequestedComponentTypes { get; } = new List<Type>();
--
GitLab