Skip to content

Commit

Permalink
[Blazor][Fixes #12197] Dispose the circuit on graceful disconnections (
Browse files Browse the repository at this point in the history
…#12449)

* Immediately releases the circuit when the client disconnects gracefully.
* This functionality is limited to websockets.
* We are able to release the circuit in the following situations:
  * The user closes the browser.
  * The user navigates away.
  * The user reloads the page.
  • Loading branch information
javiercn authored Jul 31, 2019
1 parent b2e11d7 commit 7a0a286
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 5 deletions.
19 changes: 19 additions & 0 deletions src/Components/Server/src/Circuits/CircuitRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ public void Register(CircuitHost circuitHost)
}
}

public void PermanentDisconnect(CircuitHost circuitHost)
{
if (ConnectedCircuits.TryRemove(circuitHost.CircuitId, out _))
{
Log.CircuitDisconnectedPermanently(_logger, circuitHost.CircuitId);
circuitHost.Client.SetDisconnected();
}
}

public virtual Task DisconnectAsync(CircuitHost circuitHost, string connectionId)
{
Log.CircuitDisconnectStarted(_logger, circuitHost.CircuitId, connectionId);
Expand Down Expand Up @@ -314,6 +323,7 @@ private static class Log
private static readonly Action<ILogger, string, Exception> _circuitNotActive;
private static readonly Action<ILogger, string, string, Exception> _circuitConnectedToDifferentConnection;
private static readonly Action<ILogger, string, Exception> _circuitMarkedDisconnected;
private static readonly Action<ILogger, string, Exception> _circuitDisconnectedPermanently;
private static readonly Action<ILogger, string, EvictionReason, Exception> _circuitEvicted;

private static class EventIds
Expand All @@ -330,6 +340,7 @@ private static class EventIds
public static readonly EventId CircuitConnectedToDifferentConnection = new EventId(109, "CircuitConnectedToDifferentConnection");
public static readonly EventId CircuitMarkedDisconnected = new EventId(110, "CircuitMarkedDisconnected");
public static readonly EventId CircuitEvicted = new EventId(111, "CircuitEvicted");
public static readonly EventId CircuitDisconnectedPermanently = new EventId(112, "CircuitDisconnectedPermanently");
}

static Log()
Expand Down Expand Up @@ -394,6 +405,11 @@ static Log()
EventIds.CircuitMarkedDisconnected,
"Circuit with id {CircuitId} is disconnected.");

_circuitDisconnectedPermanently = LoggerMessage.Define<string>(
LogLevel.Debug,
EventIds.CircuitDisconnectedPermanently,
"Circuit with id {CircuitId} has been removed from the registry for permanent disconnection.");

_circuitEvicted = LoggerMessage.Define<string, EvictionReason>(
LogLevel.Debug,
EventIds.CircuitEvicted,
Expand Down Expand Up @@ -436,6 +452,9 @@ public static void CircuitConnectedToDifferentConnection(ILogger logger, string
public static void CircuitMarkedDisconnected(ILogger logger, string circuitId) =>
_circuitMarkedDisconnected(logger, circuitId, null);

public static void CircuitDisconnectedPermanently(ILogger logger, string circuitId) =>
_circuitDisconnectedPermanently(logger, circuitId, null);

public static void CircuitEvicted(ILogger logger, string circuitId, EvictionReason evictionReason) =>
_circuitEvicted(logger, circuitId, evictionReason, null);
}
Expand Down
31 changes: 30 additions & 1 deletion src/Components/Server/src/ComponentHub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,32 @@ public override Task OnDisconnectedAsync(Exception exception)
}

CircuitHost = null;
return _circuitRegistry.DisconnectAsync(circuitHost, Context.ConnectionId);
if (exception != null)
{
return _circuitRegistry.DisconnectAsync(circuitHost, Context.ConnectionId);
}
else
{
// The client will gracefully disconnect when using websockets by correctly closing the TCP connection.
// This happens when the user closes a tab, navigates away from the page or reloads the page.
// In these situations we know the user is done with the circuit, so we can get rid of it at that point.
// This is important to be able to more efficiently manage resources, specially memory.
return TerminateCircuitGracefully(circuitHost);
}
}

private async Task TerminateCircuitGracefully(CircuitHost circuitHost)
{
try
{
Log.CircuitTerminatedGracefully(_logger, circuitHost.CircuitId);
_circuitRegistry.PermanentDisconnect(circuitHost);
await circuitHost.DisposeAsync();
}
catch (Exception e)
{
Log.UnhandledExceptionInCircuit(_logger, circuitHost.CircuitId, e);
}
}

/// <summary>
Expand Down Expand Up @@ -248,6 +273,8 @@ private static class Log
private static readonly Action<ILogger, string, Exception> _circuitHostNotInitialized =
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(6, "CircuitHostNotInitialized"), "Call to '{CallSite}' received before the circuit host initialization.");

private static readonly Action<ILogger, string, Exception> _circuitTerminatedGracefully =
LoggerMessage.Define<string>(LogLevel.Debug, new EventId(7, "CircuitTerminatedGracefully"), "Circuit '{CircuitId}' terminated gracefully.");

public static void NoComponentsRegisteredInEndpoint(ILogger logger, string endpointDisplayName)
{
Expand All @@ -272,6 +299,8 @@ public static void FailedToTransmitException(ILogger logger, string circuitId, E
public static void CircuitAlreadyInitialized(ILogger logger, string circuitId) => _circuitAlreadyInitialized(logger, circuitId, null);

public static void CircuitHostNotInitialized(ILogger logger, [CallerMemberName] string callSite = "") => _circuitHostNotInitialized(logger, callSite, null);

public static void CircuitTerminatedGracefully(ILogger logger, string circuitId) => _circuitTerminatedGracefully(logger, circuitId, null);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Hosting;

namespace Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures
{
Expand All @@ -11,6 +12,8 @@ public class ToggleExecutionModeServerFixture<TClientProgram>
{
public string PathBase { get; set; }

public IWebHost Host { get; set; }

public ExecutionMode ExecutionMode { get; set; } = ExecutionMode.Client;

private AspNetSiteServerFixture.BuildWebHost _buildWebHostMethod;
Expand All @@ -32,7 +35,11 @@ protected override string StartAndGetRootUri()
var underlying = new DevHostServerFixture<TClientProgram>();
underlying.PathBase = PathBase;
_serverToDispose = underlying;
return underlying.RootUri.AbsoluteUri;
var uri = underlying.RootUri.AbsoluteUri; // As a side-effect, this starts the server

Host = underlying.Host;

return uri;
}
else
{
Expand All @@ -41,7 +48,11 @@ protected override string StartAndGetRootUri()
underlying.AdditionalArguments.AddRange(AspNetFixtureAdditionalArguments);
underlying.BuildWebHostMethod = _buildWebHostMethod;
_serverToDispose = underlying;
return underlying.RootUri.AbsoluteUri;
var uri = underlying.RootUri.AbsoluteUri; // As a side-effect, this starts the server

Host = underlying.Host;

return uri;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using BasicTestApp;
using Castle.DynamicProxy.Contributors;
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure;
using Microsoft.AspNetCore.Components.E2ETest.Infrastructure.ServerFixtures;
using Microsoft.AspNetCore.Components.E2ETest.ServerExecutionTests;
using Microsoft.AspNetCore.E2ETesting;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging.Testing;
using OpenQA.Selenium;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.AspNetCore.Components.E2ETests.ServerExecutionTests
{
public class CircuitGracefulTerminationTests : BasicTestAppTestBase, IDisposable
{
public CircuitGracefulTerminationTests(
BrowserFixture browserFixture,
ToggleExecutionModeServerFixture<Program> serverFixture,
ITestOutputHelper output)
: base(browserFixture, serverFixture.WithServerExecution(), output)
{
}

public TaskCompletionSource<object> GracefulDisconnectCompletionSource { get; private set; }
public TestSink Sink { get; private set; }
public List<(Extensions.Logging.LogLevel level, string eventIdName)> Messages { get; private set; }

public override async Task InitializeAsync()
{
// These tests manipulate the browser in ways that make it impossible to use the same browser
// instance across tests (One of the tests closes the browser). For that reason we simply create
// a new browser instance for every test in this class sos that there are no issues when running
// them together.
await base.InitializeAsync(Guid.NewGuid().ToString());
}

protected override void InitializeAsyncCore()
{
Navigate(ServerPathBase, noReload: false);
MountTestComponent<CounterComponent>();
Browser.Equal("Current count: 0", () => Browser.FindElement(By.TagName("p")).Text);

GracefulDisconnectCompletionSource = new TaskCompletionSource<object>(TaskContinuationOptions.RunContinuationsAsynchronously);
Sink = _serverFixture.Host.Services.GetRequiredService<TestSink>();
Messages = new List<(Extensions.Logging.LogLevel level, string eventIdName)>();
Sink.MessageLogged += Log;
}

[Fact]
public async Task ReloadingThePage_GracefullyDisconnects_TheCurrentCircuit()
{
// Arrange & Act
_ = ((IJavaScriptExecutor)Browser).ExecuteScript("location.reload()");
await Task.WhenAny(Task.Delay(10000), GracefulDisconnectCompletionSource.Task);

// Assert
Assert.Contains((Extensions.Logging.LogLevel.Debug, "CircuitTerminatedGracefully"), Messages);
Assert.Contains((Extensions.Logging.LogLevel.Debug, "CircuitDisconnectedPermanently"), Messages);
}

[Fact]
public async Task ClosingTheBrowserWindow_GracefullyDisconnects_TheCurrentCircuit()
{
// Arrange & Act
Browser.Close();
// Set to null so that other tests in this class can create a new browser if necessary so
// that tests don't fail when running together.
Browser = null;

await Task.WhenAny(Task.Delay(10000), GracefulDisconnectCompletionSource.Task);

// Assert
Assert.Contains((Extensions.Logging.LogLevel.Debug, "CircuitTerminatedGracefully"), Messages);
Assert.Contains((Extensions.Logging.LogLevel.Debug, "CircuitDisconnectedPermanently"), Messages);
}

[Fact]
public async Task ClosingTheBrowserWindow_GracefullyDisconnects_WhenNavigatingAwayFromThePage()
{
// Arrange & Act
Browser.Navigate().GoToUrl("about:blank");
await Task.WhenAny(Task.Delay(10000), GracefulDisconnectCompletionSource.Task);

// Assert
Assert.Contains((Extensions.Logging.LogLevel.Debug, "CircuitTerminatedGracefully"), Messages);
Assert.Contains((Extensions.Logging.LogLevel.Debug, "CircuitDisconnectedPermanently"), Messages);
}

private void Log(WriteContext wc)
{
if ((Extensions.Logging.LogLevel.Debug, "CircuitTerminatedGracefully") == (wc.LogLevel, wc.EventId.Name))
{
GracefulDisconnectCompletionSource.TrySetResult(null);
}
Messages.Add((wc.LogLevel, wc.EventId.Name));
}

public void Dispose()
{
if (Sink != null)
{
Sink.MessageLogged -= Log;
}
}
}
}
9 changes: 7 additions & 2 deletions src/Shared/E2ETesting/BrowserTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,14 @@ public Task DisposeAsync()
return Task.CompletedTask;
}

public virtual async Task InitializeAsync()
public virtual Task InitializeAsync()
{
var (browser, logs) = await BrowserFixture.GetOrCreateBrowserAsync(Output);
return InitializeAsync("");
}

public virtual async Task InitializeAsync(string isolationContext)
{
var (browser, logs) = await BrowserFixture.GetOrCreateBrowserAsync(Output, isolationContext);
_asyncBrowser.Value = browser;
_logs.Value = logs;

Expand Down

0 comments on commit 7a0a286

Please sign in to comment.