[Blazor] Missing IAsyncDisposable implementation on renderer (#26848)

* Implement async disposable on the renderer

* Minor cleanups
This commit is contained in:
Javier Calvarro Nelson 2020-10-13 22:50:54 +02:00 committed by GitHub
parent 905777ca5b
commit 901bf9ba43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 151 additions and 8 deletions

View File

@ -211,6 +211,7 @@ Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrameType.Text = 2 -> Micro
Microsoft.AspNetCore.Components.RenderTree.Renderer
Microsoft.AspNetCore.Components.RenderTree.Renderer.AssignRootComponentId(Microsoft.AspNetCore.Components.IComponent! component) -> int
Microsoft.AspNetCore.Components.RenderTree.Renderer.Dispose() -> void
Microsoft.AspNetCore.Components.RenderTree.Renderer.DisposeAsync() -> System.Threading.Tasks.ValueTask
Microsoft.AspNetCore.Components.RenderTree.Renderer.ElementReferenceContext.get -> Microsoft.AspNetCore.Components.ElementReferenceContext?
Microsoft.AspNetCore.Components.RenderTree.Renderer.ElementReferenceContext.set -> void
Microsoft.AspNetCore.Components.RenderTree.Renderer.GetCurrentRenderTreeFrames(int componentId) -> Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame>

View File

@ -21,7 +21,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
//
// Provides mechanisms for rendering hierarchies of <see cref="IComponent"/> instances,
// dispatching events to them, and notifying when the user interface is being updated.
public abstract partial class Renderer : IDisposable
public abstract partial class Renderer : IDisposable, IAsyncDisposable
{
private readonly IServiceProvider _serviceProvider;
private readonly Dictionary<int, ComponentState> _componentStateById = new Dictionary<int, ComponentState>();
@ -36,6 +36,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
private ulong _lastEventHandlerId;
private List<Task> _pendingTasks;
private bool _disposed;
private Task _disposeTask;
/// <summary>
/// Allows the caller to handle exceptions from the SynchronizationContext when one is available.
@ -763,11 +764,34 @@ namespace Microsoft.AspNetCore.Components.RenderTree
// It's important that we handle all exceptions here before reporting any of them.
// This way we can dispose all components before an error handler kicks in.
List<Exception> exceptions = null;
List<Task> asyncDisposables = null;
foreach (var componentState in _componentStateById.Values)
{
Log.DisposingComponent(_logger, componentState);
if (componentState.Component is IDisposable disposable)
// Components shouldn't need to implement IAsyncDisposable and IDisposable simultaneously,
// but in case they do, we prefer the async overload since we understand the sync overload
// is implemented for more "constrained" scenarios.
// Component authors are responsible for their IAsyncDisposable implementations not taking
// forever.
if (componentState.Component is IAsyncDisposable asyncDisposable)
{
try
{
var task = asyncDisposable.DisposeAsync();
if (!task.IsCompletedSuccessfully)
{
asyncDisposables ??= new();
asyncDisposables.Add(task.AsTask());
}
}
catch (Exception exception)
{
exceptions ??= new List<Exception>();
exceptions.Add(exception);
}
}
else if (componentState.Component is IDisposable disposable)
{
try
{
@ -784,13 +808,42 @@ namespace Microsoft.AspNetCore.Components.RenderTree
_componentStateById.Clear(); // So we know they were all disposed
_batchBuilder.Dispose();
if (exceptions?.Count > 1)
NotifyExceptions(exceptions);
if (asyncDisposables?.Count >= 1)
{
HandleException(new AggregateException("Exceptions were encountered while disposing components.", exceptions));
_disposeTask = HandleAsyncExceptions(asyncDisposables);
}
else if (exceptions?.Count == 1)
async Task HandleAsyncExceptions(List<Task> tasks)
{
HandleException(exceptions[0]);
List<Exception> asyncExceptions = null;
foreach (var task in tasks)
{
try
{
await task;
}
catch (Exception exception)
{
asyncExceptions ??= new List<Exception>();
asyncExceptions.Add(exception);
}
}
NotifyExceptions(asyncExceptions);
}
void NotifyExceptions(List<Exception> exceptions)
{
if (exceptions?.Count > 1)
{
HandleException(new AggregateException("Exceptions were encountered while disposing components.", exceptions));
}
else if (exceptions?.Count == 1)
{
HandleException(exceptions[0]);
}
}
}
@ -801,5 +854,31 @@ namespace Microsoft.AspNetCore.Components.RenderTree
{
Dispose(disposing: true);
}
/// <inheritdoc />
public async ValueTask DisposeAsync()
{
if (_disposed)
{
return;
}
if (_disposeTask != null)
{
await _disposeTask;
}
else
{
Dispose();
if (_disposeTask != null)
{
await _disposeTask;
}
else
{
await default(ValueTask);
}
}
}
}
}

View File

@ -3813,6 +3813,66 @@ namespace Microsoft.AspNetCore.Components.Test
Assert.Contains(exception2, aex.InnerExceptions);
}
[Fact]
public async Task DisposingRenderer_CapturesSyncExceptionsFromAllRegisteredAsyncDisposableComponents()
{
// Arrange
var renderer = new TestRenderer { ShouldHandleExceptions = true };
var exception1 = new InvalidOperationException();
var disposed = false;
var component = new TestComponent(builder =>
{
builder.AddContent(0, "Hello");
builder.OpenComponent<AsyncDisposableComponent>(1);
builder.AddAttribute(1, nameof(AsyncDisposableComponent.AsyncDisposeAction), (Func<ValueTask>)(() => { disposed = true; throw exception1; }));
builder.CloseComponent();
});
var componentId = renderer.AssignRootComponentId(component);
component.TriggerRender();
// Act
await renderer.DisposeAsync();
// Assert
Assert.True(disposed);
var handledException = Assert.Single(renderer.HandledExceptions);
Assert.Same(exception1, handledException);
}
[Fact]
public async Task DisposingRenderer_CapturesAsyncExceptionsFromAllRegisteredAsyncDisposableComponents()
{
// Arrange
var renderer = new TestRenderer { ShouldHandleExceptions = true };
var exception1 = new InvalidOperationException();
var disposed = false;
var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var component = new TestComponent(builder =>
{
builder.AddContent(0, "Hello");
builder.OpenComponent<AsyncDisposableComponent>(1);
builder.AddAttribute(1, nameof(AsyncDisposableComponent.AsyncDisposeAction), (Func<ValueTask>)(async () => { await tcs.Task; disposed = true; throw exception1; }));
builder.CloseComponent();
});
var componentId = renderer.AssignRootComponentId(component);
component.TriggerRender();
// Act
var disposal = renderer.DisposeAsync();
Assert.False(disposed);
Assert.False(disposal.IsCompleted);
tcs.TrySetResult();
await disposal;
// Assert
Assert.True(disposed);
var handledException = Assert.Single(renderer.HandledExceptions);
Assert.Same(exception1, handledException);
}
[Theory]
[InlineData(null)] // No existing attribute to update
[InlineData("old property value")] // Has existing attribute to update

View File

@ -168,7 +168,7 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits
try
{
Renderer.Dispose();
await Renderer.DisposeAsync();
// This cast is needed because it's possible the scope may not support async dispose.
// Our DI container does, but other DI systems may not.

View File

@ -74,7 +74,10 @@ namespace Microsoft.AspNetCore.Components.WebAssembly.Hosting
_disposed = true;
_renderer?.Dispose();
if (_renderer != null)
{
await _renderer.DisposeAsync();
}
if (_scope is IAsyncDisposable asyncDisposableScope)
{