Skip to content

Commit 3a86d70

Browse files
pranavkmpranavkm
committed
Dispose components on client disconnects
Fixes #4047
1 parent 6765fd0 commit 3a86d70

File tree

9 files changed

+155
-34
lines changed

9 files changed

+155
-34
lines changed

‎src/Components/blazor/src/Microsoft.AspNetCore.Blazor/Rendering/WebAssemblyRenderer.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
1515
/// Provides mechanisms for rendering <see cref="IComponent"/> instances in a
1616
/// web browser, dispatching events to them, and refreshing the UI as required.
1717
/// </summary>
18-
public class WebAssemblyRenderer : Renderer, IDisposable
18+
public class WebAssemblyRenderer : Renderer
1919
{
2020
private readonly int _webAssemblyRendererId;
2121

@@ -71,11 +71,10 @@ public void AddComponent(Type componentType, string domElementSelector)
7171
RenderRootComponent(componentId);
7272
}
7373

74-
/// <summary>
75-
/// Disposes the instance.
76-
/// </summary>
77-
public void Dispose()
74+
/// <inheritdoc />
75+
protected override void Dispose(bool disposing)
7876
{
77+
base.Dispose(disposing);
7978
RendererRegistry.Current.TryRemove(_webAssemblyRendererId);
8079
}
8180

‎src/Components/src/Microsoft.AspNetCore.Components.Browser/Properties/AssemblyInfo.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@
22

33
[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Blazor, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
44
[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Components.Server, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]
5+
6+
[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Components.Server.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")]

‎src/Components/src/Microsoft.AspNetCore.Components.Server/Circuits/CircuitHost.cs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,26 @@ namespace Microsoft.AspNetCore.Components.Server.Circuits
1616
{
1717
internal class CircuitHost : IDisposable
1818
{
19-
private static AsyncLocal<CircuitHost> _current = new AsyncLocal<CircuitHost>();
19+
private static readonly AsyncLocal<CircuitHost> _current = new AsyncLocal<CircuitHost>();
2020

2121
/// <summary>
2222
/// Gets the current <see cref="Circuit"/>, if any.
2323
/// </summary>
2424
public static CircuitHost Current => _current.Value;
2525

2626
/// <summary>
27-
/// Sets the current <see cref="Circuit"/>.
27+
/// Sets the current <see cref="Circuits.Circuit"/>.
2828
/// </summary>
29-
/// <param name="circuitHost">The <see cref="Circuit"/>.</param>
29+
/// <param name="circuitHost">The <see cref="Circuits.Circuit"/>.</param>
3030
/// <remarks>
3131
/// Calling <see cref="SetCurrentCircuitHost(CircuitHost)"/> will store the circuit
3232
/// and other related values such as the <see cref="IJSRuntime"/> and <see cref="Renderer"/>
3333
/// in the local execution context. Application code should not need to call this method,
34-
/// it is primarily used by the Server-Side Blazor infrastructure.
34+
/// it is primarily used by the Server-Side Components infrastructure.
3535
/// </remarks>
3636
public static void SetCurrentCircuitHost(CircuitHost circuitHost)
3737
{
38-
if (circuitHost == null)
39-
{
40-
throw new ArgumentNullException(nameof(circuitHost));
41-
}
42-
43-
_current.Value = circuitHost;
38+
_current.Value = circuitHost ?? throw new ArgumentNullException(nameof(circuitHost));
4439

4540
Microsoft.JSInterop.JSRuntime.SetCurrentJSRuntime(circuitHost.JSRuntime);
4641
RendererRegistry.SetCurrentRendererRegistry(circuitHost.RendererRegistry);
@@ -134,6 +129,7 @@ await SynchronizationContext.Invoke(() =>
134129
public void Dispose()
135130
{
136131
Scope.Dispose();
132+
Renderer.Dispose();
137133
}
138134

139135
private void AssertInitialized()

‎src/Components/src/Microsoft.AspNetCore.Components.Server/Circuits/RemoteRenderer.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,10 @@ public override Task InvokeAsync(Func<Task> workItem)
120120
}
121121
}
122122

123-
/// <summary>
124-
/// Disposes the instance.
125-
/// </summary>
126-
public void Dispose()
123+
/// <inheritdoc />
124+
protected override void Dispose(bool disposing)
127125
{
126+
base.Dispose(true);
128127
_rendererRegistry.TryRemove(_id);
129128
}
130129

‎src/Components/src/Microsoft.AspNetCore.Components/Rendering/ComponentState.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,16 @@ public void DisposeInBatch(RenderBatchBuilder batchBuilder)
8282
{
8383
_componentWasDisposed = true;
8484

85-
// TODO: Handle components throwing during dispose. Shouldn't break the whole render batch.
8685
if (_component is IDisposable disposable)
8786
{
88-
disposable.Dispose();
87+
try
88+
{
89+
disposable.Dispose();
90+
}
91+
catch
92+
{
93+
// Ignore exceptions thrown by components when they're being disposed.
94+
}
8995
}
9096

9197
RenderTreeDiffBuilder.DisposeFrames(batchBuilder, _renderTreeBuilderCurrent.GetFrames());

‎src/Components/src/Microsoft.AspNetCore.Components/Rendering/Renderer.cs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,16 @@ namespace Microsoft.AspNetCore.Components.Rendering
1212
/// Provides mechanisms for rendering hierarchies of <see cref="IComponent"/> instances,
1313
/// dispatching events to them, and notifying when the user interface is being updated.
1414
/// </summary>
15-
public abstract class Renderer
15+
public abstract class Renderer : IDisposable
1616
{
1717
private readonly ComponentFactory _componentFactory;
18-
private int _nextComponentId = 0; // TODO: change to 'long' when Mono .NET->JS interop supports it
19-
private readonly Dictionary<int, ComponentState> _componentStateById
20-
= new Dictionary<int, ComponentState>();
21-
18+
private readonly Dictionary<int, ComponentState> _componentStateById = new Dictionary<int, ComponentState>();
2219
private readonly RenderBatchBuilder _batchBuilder = new RenderBatchBuilder();
23-
private bool _isBatchInProgress;
20+
private readonly Dictionary<int, EventHandlerInvoker> _eventBindings = new Dictionary<int, EventHandlerInvoker>();
2421

22+
private int _nextComponentId = 0; // TODO: change to 'long' when Mono .NET->JS interop supports it
23+
private bool _isBatchInProgress;
2524
private int _lastEventHandlerId = 0;
26-
private readonly Dictionary<int, EventHandlerInvoker> _eventBindings = new Dictionary<int, EventHandlerInvoker>();
2725

2826
/// <summary>
2927
/// Constructs an instance of <see cref="Renderer"/>.
@@ -175,7 +173,7 @@ internal void AssignEventHandlerId(ref RenderTreeFrame frame)
175173

176174
if (frame.AttributeValue is MulticastDelegate @delegate)
177175
{
178-
_eventBindings.Add(id, new EventHandlerInvoker(@delegate));
176+
_eventBindings.Add(id, new EventHandlerInvoker(@delegate));
179177
}
180178

181179
frame = frame.WithAttributeEventHandlerId(id);
@@ -295,5 +293,35 @@ private void RemoveEventHandlerIds(ArrayRange<int> eventHandlerIds, Task afterTa
295293
RemoveEventHandlerIds(eventHandlerIdsClone, Task.CompletedTask));
296294
}
297295
}
296+
297+
/// <summary>
298+
/// Releases all resources currently used by this <see cref="Renderer"/> instance.
299+
/// </summary>
300+
/// <param name="disposing"><see langword="true"/> if this method is being invoked by <see cref="IDisposable.Dispose"/>, otherwise <see langword="false"/>.</param>
301+
protected virtual void Dispose(bool disposing)
302+
{
303+
foreach (var componentState in _componentStateById.Values)
304+
{
305+
if (componentState.Component is IDisposable disposable)
306+
{
307+
try
308+
{
309+
disposable.Dispose();
310+
}
311+
catch
312+
{
313+
// Ignore failures in components throwing
314+
}
315+
}
316+
}
317+
}
318+
319+
/// <summary>
320+
/// Releases all resources currently used by this <see cref="Renderer"/> instance.
321+
/// </summary>
322+
public void Dispose()
323+
{
324+
Dispose(disposing: true);
325+
}
298326
}
299327
}

‎src/Components/test/Microsoft.AspNetCore.Components.Server.Test/Circuits/CircuirHostTest.cs

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Components.Browser;
5+
using Microsoft.AspNetCore.Components.Browser.Rendering;
6+
using Microsoft.AspNetCore.SignalR;
7+
using Microsoft.Extensions.DependencyInjection;
8+
using Microsoft.JSInterop;
9+
using Moq;
10+
using System;
11+
using System.Threading;
12+
using Xunit;
13+
14+
namespace Microsoft.AspNetCore.Components.Server.Circuits
15+
{
16+
public class CircuirHostTest
17+
{
18+
[Fact]
19+
public void Dispose_DisposesResources()
20+
{
21+
// Arrange
22+
var serviceScope = new Mock<IServiceScope>();
23+
var clientProxy = Mock.Of<IClientProxy>();
24+
var renderRegistry = new RendererRegistry();
25+
var jsRuntime = Mock.Of<IJSRuntime>();
26+
var syncContext = new CircuitSynchronizationContext();
27+
28+
var remoteRenderer = new TestRemoteRenderer(
29+
Mock.Of<IServiceProvider>(),
30+
renderRegistry,
31+
jsRuntime,
32+
clientProxy,
33+
syncContext);
34+
35+
var circuitHost = new CircuitHost(serviceScope.Object, clientProxy, renderRegistry, remoteRenderer, configure: _ => { }, jsRuntime: jsRuntime, synchronizationContext: syncContext);
36+
37+
// Act
38+
circuitHost.Dispose();
39+
40+
// Assert
41+
serviceScope.Verify(s => s.Dispose(), Times.Once());
42+
Assert.True(remoteRenderer.Disposed);
43+
}
44+
45+
private class TestRemoteRenderer : RemoteRenderer
46+
{
47+
public TestRemoteRenderer(IServiceProvider serviceProvider, RendererRegistry rendererRegistry, IJSRuntime jsRuntime, IClientProxy client, SynchronizationContext syncContext)
48+
: base(serviceProvider, rendererRegistry, jsRuntime, client, syncContext)
49+
{
50+
}
51+
52+
public bool Disposed { get; set; }
53+
54+
protected override void Dispose(bool disposing)
55+
{
56+
base.Dispose(disposing);
57+
Disposed = true;
58+
}
59+
}
60+
}
61+
}

‎src/Components/test/Microsoft.AspNetCore.Components.Server.Test/Microsoft.AspNetCore.Components.Server.Test.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<TargetFramework>netcoreapp3.0</TargetFramework>
@@ -7,6 +7,7 @@
77

88
<ItemGroup>
99
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.5.0" />
10+
<PackageReference Include="Moq" Version="4.10.0" />
1011
<PackageReference Include="xunit" Version="2.3.1" />
1112
<PackageReference Include="xunit.runner.visualstudio" Version="2.3.1" />
1213
</ItemGroup>

‎src/Components/test/Microsoft.AspNetCore.Components.Test/RendererTest.cs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using Microsoft.AspNetCore.Components.Rendering;
5+
using Microsoft.AspNetCore.Components.RenderTree;
6+
using Microsoft.AspNetCore.Components.Test.Helpers;
47
using System;
58
using System.Collections.Generic;
69
using System.Linq;
710
using System.Threading.Tasks;
8-
using Microsoft.AspNetCore.Components;
9-
using Microsoft.AspNetCore.Components.Rendering;
10-
using Microsoft.AspNetCore.Components.RenderTree;
11-
using Microsoft.AspNetCore.Components.Test.Helpers;
1211
using Xunit;
1312

1413
namespace Microsoft.AspNetCore.Components.Test
@@ -1139,6 +1138,21 @@ public async Task CanTriggerEventHandlerDisposedInEarlierPendingBatch()
11391138
Assert.Equal(2, numEventsFired);
11401139
}
11411140

1141+
[Fact]
1142+
public void DisposingRenderer_DisposesComponents()
1143+
{
1144+
// Arrange
1145+
var renderer = new TestRenderer();
1146+
var component = new DisposableComponent();
1147+
renderer.AssignRootComponentId(component);
1148+
1149+
// Act
1150+
renderer.Dispose();
1151+
1152+
// Assert
1153+
Assert.True(component.Disposed);
1154+
}
1155+
11421156
private class NoOpRenderer : Renderer
11431157
{
11441158
public NoOpRenderer() : base(new TestServiceProvider())
@@ -1398,6 +1412,21 @@ protected override void BuildRenderTree(RenderTreeBuilder builder)
13981412
}
13991413
}
14001414

1415+
private class DisposableComponent : AutoRenderComponent, IDisposable
1416+
{
1417+
public bool Disposed { get; private set; }
1418+
1419+
public void Dispose()
1420+
{
1421+
Disposed = true;
1422+
}
1423+
1424+
protected override void BuildRenderTree(RenderTreeBuilder builder)
1425+
{
1426+
throw new NotImplementedException();
1427+
}
1428+
}
1429+
14011430
class TestAsyncRenderer : TestRenderer
14021431
{
14031432
public Task NextUpdateDisplayReturnTask { get; set; }

0 commit comments

Comments
 (0)