From defbadb26b4224d5025549992fc5ae4e95e0d288 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sat, 13 Oct 2018 11:43:20 -0700 Subject: [PATCH] Various timer cleanup (#3129) This change does 2 things: - It disables the websocket keep alive since SignalR has its own bidirectional pings. This should remove a significant timer overhead per WebSocket connection that we end up with today. We have a single timer that sends to all connection on an interval. - Don't pass the CancellationToken to ReadAsync in the handshake since the Pipe implementation holds onto the token for longer than it needs to which keeps Timer objects alive (see dotnet/corefx#32806) I found this when reading the source code and looking at dumps of a couple of SignalR applications. --- .../HubConnectionContext.cs | 10 ++++++++-- .../SignalRDependencyInjectionExtensions.cs | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs index 98583d9903..6dc3a93620 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs @@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.SignalR { public class HubConnectionContext { + private static readonly Action _cancelReader = state => ((PipeReader)state).CancelPendingRead(); private static readonly WaitCallback _abortedCallback = AbortConnection; private readonly ConnectionContext _connectionContext; @@ -344,7 +345,10 @@ namespace Microsoft.AspNetCore.SignalR { try { + var input = Input; + using (var cts = new CancellationTokenSource()) + using (var registration = cts.Token.Register(_cancelReader, input)) { if (!Debugger.IsAttached) { @@ -353,7 +357,8 @@ namespace Microsoft.AspNetCore.SignalR while (true) { - var result = await _connectionContext.Transport.Input.ReadAsync(cts.Token); + var result = await input.ReadAsync(); + var buffer = result.Buffer; var consumed = buffer.Start; var examined = buffer.End; @@ -363,6 +368,7 @@ namespace Microsoft.AspNetCore.SignalR if (result.IsCanceled) { Log.HandshakeCanceled(_logger); + await WriteHandshakeResponseAsync(new HandshakeResponseMessage("Handshake was canceled.")); return false; } @@ -434,7 +440,7 @@ namespace Microsoft.AspNetCore.SignalR } finally { - _connectionContext.Transport.Input.AdvanceTo(consumed, examined); + input.AdvanceTo(consumed, examined); } } } diff --git a/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs b/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs index 8974bd094f..f663ac7254 100644 --- a/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs +++ b/src/Microsoft.AspNetCore.SignalR/SignalRDependencyInjectionExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.SignalR; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -36,6 +37,8 @@ namespace Microsoft.Extensions.DependencyInjection public static ISignalRServerBuilder AddSignalR(this IServiceCollection services) { services.AddConnections(); + // Disable the WebSocket keep alive since SignalR has it's own + services.Configure(o => o.KeepAliveInterval = TimeSpan.Zero); services.TryAddSingleton(); services.TryAddEnumerable(ServiceDescriptor.Singleton, HubOptionsSetup>()); return services.AddSignalRCore();