diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs index abf6b69524..c9495d7156 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs @@ -84,13 +84,6 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal Features.Set(this); } - internal HttpConnectionContext(string id, IDuplexPipe transport, IDuplexPipe application, ILogger logger = null) - : this(id, null, logger) - { - Transport = transport; - Application = application; - } - public CancellationTokenSource Cancellation { get; set; } public HttpTransportType TransportType { get; set; } diff --git a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs index b0f4b079fb..aec84c5dab 100644 --- a/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs +++ b/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs @@ -10,7 +10,6 @@ using System.IO.Pipelines; using System.Net.WebSockets; using System.Security.Cryptography; using System.Threading.Tasks; -using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Internal; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Internal; @@ -31,24 +30,14 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal private readonly TimerAwaitable _nextHeartbeat; private readonly ILogger _logger; private readonly ILogger _connectionLogger; - private readonly bool _useSendTimeout = true; private readonly TimeSpan _disconnectTimeout; - public HttpConnectionManager(ILoggerFactory loggerFactory, IHostApplicationLifetime appLifetime) - : this(loggerFactory, appLifetime, Options.Create(new ConnectionOptions() { DisconnectTimeout = ConnectionOptionsSetup.DefaultDisconectTimeout })) - { - } - public HttpConnectionManager(ILoggerFactory loggerFactory, IHostApplicationLifetime appLifetime, IOptions connectionOptions) { _logger = loggerFactory.CreateLogger(); _connectionLogger = loggerFactory.CreateLogger(); _nextHeartbeat = new TimerAwaitable(_heartbeatTickRate, _heartbeatTickRate); _disconnectTimeout = connectionOptions.Value.DisconnectTimeout ?? ConnectionOptionsSetup.DefaultDisconectTimeout; - if (AppContext.TryGetSwitch("Microsoft.AspNetCore.Http.Connections.DoNotUseSendTimeout", out var timeoutDisabled)) - { - _useSendTimeout = !timeoutDisabled; - } // Register these last as the callbacks could run immediately appLifetime.ApplicationStarted.Register(() => Start()); @@ -176,7 +165,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Internal } else { - if (!Debugger.IsAttached && _useSendTimeout) + if (!Debugger.IsAttached) { connection.TryCancelSend(utcNow.Ticks); } diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs index 34626ca05b..5f084a232f 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs @@ -2347,10 +2347,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests private static HttpConnectionManager CreateConnectionManager(ILoggerFactory loggerFactory) { - return new HttpConnectionManager(loggerFactory ?? new LoggerFactory(), new EmptyApplicationLifetime()); + return CreateConnectionManager(loggerFactory, null); } - private static HttpConnectionManager CreateConnectionManager(ILoggerFactory loggerFactory, TimeSpan disconnectTimeout) + private static HttpConnectionManager CreateConnectionManager(ILoggerFactory loggerFactory, TimeSpan? disconnectTimeout) { var connectionOptions = new ConnectionOptions(); connectionOptions.DisconnectTimeout = disconnectTimeout; diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs index 05a29f0e73..dcf8dfefa9 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs @@ -7,6 +7,7 @@ using System.IO.Pipelines; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http.Connections.Internal; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Tests; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -411,7 +412,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests private static HttpConnectionManager CreateConnectionManager(ILoggerFactory loggerFactory, IHostApplicationLifetime lifetime = null) { lifetime = lifetime ?? new EmptyApplicationLifetime(); - return new HttpConnectionManager(loggerFactory, lifetime); + return new HttpConnectionManager(loggerFactory, lifetime, Options.Create(new ConnectionOptions())); } [Flags] diff --git a/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs b/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs index 27ad53d9d9..b5e50d7894 100644 --- a/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs +++ b/src/SignalR/common/Http.Connections/test/WebSocketsTests.cs @@ -31,11 +31,15 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application, LoggerFactory.CreateLogger("HttpConnectionContext1")); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext1")) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { - var connectionContext = new HttpConnectionContext(string.Empty, null, null, LoggerFactory.CreateLogger("HttpConnectionContext2")); + var connectionContext = new HttpConnectionContext(string.Empty, connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext2")); var ws = new WebSocketsServerTransport(new WebSocketOptions(), connection.Application, connectionContext, LoggerFactory); // Give the server socket to the transport and run it @@ -79,11 +83,15 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application, LoggerFactory.CreateLogger("HttpConnectionContext1")); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext1")) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { - var connectionContext = new HttpConnectionContext(string.Empty, null, null, LoggerFactory.CreateLogger("HttpConnectionContext2")); + var connectionContext = new HttpConnectionContext(string.Empty, connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext2")); connectionContext.ActiveFormat = transferFormat; var ws = new WebSocketsServerTransport(new WebSocketOptions(), connection.Application, connectionContext, LoggerFactory); @@ -116,7 +124,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application, LoggerFactory.CreateLogger("HttpConnectionContext1")); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext1")) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { @@ -139,7 +151,7 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests } } - var connectionContext = new HttpConnectionContext(string.Empty, null, null, LoggerFactory.CreateLogger("HttpConnectionContext2")); + var connectionContext = new HttpConnectionContext(string.Empty, connectionToken: null, LoggerFactory.CreateLogger("HttpConnectionContext2")); var ws = new WebSocketsServerTransport(new WebSocketOptions(), connection.Application, connectionContext, LoggerFactory); // Give the server socket to the transport and run it @@ -169,7 +181,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext))) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { @@ -201,7 +217,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext))) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { @@ -236,7 +256,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext))) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { @@ -271,7 +295,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext))) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { @@ -311,7 +339,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext))) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { @@ -354,7 +386,11 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests using (StartVerifiableLog()) { var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); - var connection = new HttpConnectionContext("foo", pair.Transport, pair.Application); + var connection = new HttpConnectionContext("foo", connectionToken: null, LoggerFactory.CreateLogger(nameof(HttpConnectionContext))) + { + Transport = pair.Transport, + Application = pair.Application, + }; using (var feature = new TestWebSocketConnectionFeature()) { diff --git a/src/SignalR/common/Shared/ISystemClock.cs b/src/SignalR/common/Shared/ISystemClock.cs new file mode 100644 index 0000000000..86e2b035ea --- /dev/null +++ b/src/SignalR/common/Shared/ISystemClock.cs @@ -0,0 +1,26 @@ +// 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; + +namespace Microsoft.AspNetCore.Internal +{ + internal interface ISystemClock + { + /// + /// Retrieves the current UTC system time. + /// + DateTimeOffset UtcNow { get; } + + /// + /// Retrieves ticks for the current UTC system time. + /// + long UtcNowTicks { get; } + + /// + /// Retrieves the current UTC system time. + /// This is only safe to use from code called by the Heartbeat. + /// + DateTimeOffset UtcNowUnsynchronized { get; } + } +} diff --git a/src/SignalR/common/Shared/SystemClock.cs b/src/SignalR/common/Shared/SystemClock.cs new file mode 100644 index 0000000000..de5a9b711c --- /dev/null +++ b/src/SignalR/common/Shared/SystemClock.cs @@ -0,0 +1,28 @@ +// 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; + +namespace Microsoft.AspNetCore.Internal +{ + /// + /// Provides access to the normal system clock. + /// + internal class SystemClock : ISystemClock + { + /// + /// Retrieves the current UTC system time. + /// + public DateTimeOffset UtcNow => DateTimeOffset.UtcNow; + + /// + /// Retrieves ticks for the current UTC system time. + /// + public long UtcNowTicks => DateTimeOffset.UtcNow.Ticks; + + /// + /// Retrieves the current UTC system time. + /// + public DateTimeOffset UtcNowUnsynchronized => DateTimeOffset.UtcNow; + } +} diff --git a/src/SignalR/server/Core/src/HubConnectionContext.cs b/src/SignalR/server/Core/src/HubConnectionContext.cs index 1dc7ad89c4..91516701e4 100644 --- a/src/SignalR/server/Core/src/HubConnectionContext.cs +++ b/src/SignalR/server/Core/src/HubConnectionContext.cs @@ -13,6 +13,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Connections.Features; using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.Extensions.Logging; @@ -34,13 +35,11 @@ namespace Microsoft.AspNetCore.SignalR private readonly long _keepAliveInterval; private readonly long _clientTimeoutInterval; private readonly SemaphoreSlim _writeLock = new SemaphoreSlim(1); - private readonly bool _useAbsoluteClientTimeout; private readonly object _receiveMessageTimeoutLock = new object(); + private readonly ISystemClock _systemClock; private StreamTracker _streamTracker; - private long _lastSendTimeStamp = DateTime.UtcNow.Ticks; - private long _lastReceivedTimeStamp = DateTime.UtcNow.Ticks; - private bool _receivedMessageThisInterval = false; + private long _lastSendTimeStamp; private ReadOnlyMemory _cachedPingMessage; private bool _clientTimeoutActive; private volatile bool _connectionAborted; @@ -70,10 +69,8 @@ namespace Microsoft.AspNetCore.SignalR HubCallerContext = new DefaultHubCallerContext(this); - if (AppContext.TryGetSwitch("Microsoft.AspNetCore.SignalR.UseAbsoluteClientTimeout", out var useAbsoluteClientTimeout)) - { - _useAbsoluteClientTimeout = useAbsoluteClientTimeout; - } + _systemClock = contextOptions.SystemClock ?? new SystemClock(); + _lastSendTimeStamp = _systemClock.UtcNowTicks; } internal StreamTracker StreamTracker @@ -558,7 +555,7 @@ namespace Microsoft.AspNetCore.SignalR private void KeepAliveTick() { - var currentTime = DateTime.UtcNow.Ticks; + var currentTime = _systemClock.UtcNowTicks; // Implements the keep-alive tick behavior // Each tick, we check if the time since the last send is larger than the keep alive duration (in ticks). @@ -597,35 +594,17 @@ namespace Microsoft.AspNetCore.SignalR return; } - if (_useAbsoluteClientTimeout) + lock (_receiveMessageTimeoutLock) { - // If it's been too long since we've heard from the client, then close this - if (DateTime.UtcNow.Ticks - Volatile.Read(ref _lastReceivedTimeStamp) > _clientTimeoutInterval) + if (_receivedMessageTimeoutEnabled) { - if (!_receivedMessageThisInterval) + _receivedMessageElapsedTicks = _systemClock.UtcNowTicks - _receivedMessageTimestamp; + + if (_receivedMessageElapsedTicks >= _clientTimeoutInterval) { Log.ClientTimeout(_logger, TimeSpan.FromTicks(_clientTimeoutInterval)); AbortAllowReconnect(); } - - _receivedMessageThisInterval = false; - Volatile.Write(ref _lastReceivedTimeStamp, DateTime.UtcNow.Ticks); - } - } - else - { - lock (_receiveMessageTimeoutLock) - { - if (_receivedMessageTimeoutEnabled) - { - _receivedMessageElapsedTicks = DateTime.UtcNow.Ticks - _receivedMessageTimestamp; - - if (_receivedMessageElapsedTicks >= _clientTimeoutInterval) - { - Log.ClientTimeout(_logger, TimeSpan.FromTicks(_clientTimeoutInterval)); - AbortAllowReconnect(); - } - } } } } @@ -670,37 +649,24 @@ namespace Microsoft.AspNetCore.SignalR } } - internal void ResetClientTimeout() - { - _receivedMessageThisInterval = true; - } - internal void BeginClientTimeout() { - // check if new timeout behavior is in use - if (!_useAbsoluteClientTimeout) + lock (_receiveMessageTimeoutLock) { - lock (_receiveMessageTimeoutLock) - { - _receivedMessageTimeoutEnabled = true; - _receivedMessageTimestamp = DateTime.UtcNow.Ticks; - } + _receivedMessageTimeoutEnabled = true; + _receivedMessageTimestamp = _systemClock.UtcNowTicks; } } internal void StopClientTimeout() { - // check if new timeout behavior is in use - if (!_useAbsoluteClientTimeout) + lock (_receiveMessageTimeoutLock) { - lock (_receiveMessageTimeoutLock) - { - // we received a message so stop the timer and reset it - // it will resume after the message has been processed - _receivedMessageElapsedTicks = 0; - _receivedMessageTimestamp = 0; - _receivedMessageTimeoutEnabled = false; - } + // we received a message so stop the timer and reset it + // it will resume after the message has been processed + _receivedMessageElapsedTicks = 0; + _receivedMessageTimestamp = 0; + _receivedMessageTimeoutEnabled = false; } } diff --git a/src/SignalR/server/Core/src/HubConnectionContextOptions.cs b/src/SignalR/server/Core/src/HubConnectionContextOptions.cs index c95c58afe6..fc1d34383e 100644 --- a/src/SignalR/server/Core/src/HubConnectionContextOptions.cs +++ b/src/SignalR/server/Core/src/HubConnectionContextOptions.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.Internal; namespace Microsoft.AspNetCore.SignalR { @@ -29,5 +30,7 @@ namespace Microsoft.AspNetCore.SignalR /// Gets or sets the maximum message size the client can send. /// public long? MaximumReceiveMessageSize { get; set; } + + internal ISystemClock SystemClock { get; set; } } } diff --git a/src/SignalR/server/Core/src/HubConnectionHandler.cs b/src/SignalR/server/Core/src/HubConnectionHandler.cs index 0a8f3380f9..6667890496 100644 --- a/src/SignalR/server/Core/src/HubConnectionHandler.cs +++ b/src/SignalR/server/Core/src/HubConnectionHandler.cs @@ -7,6 +7,7 @@ using System.IO; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.Extensions.DependencyInjection; @@ -31,6 +32,9 @@ namespace Microsoft.AspNetCore.SignalR private readonly bool _enableDetailedErrors; private readonly long? _maximumMessageSize; + // Internal for testing + internal ISystemClock SystemClock { get; set; } = new SystemClock(); + /// /// Initializes a new instance of the class. /// @@ -98,6 +102,7 @@ namespace Microsoft.AspNetCore.SignalR ClientTimeoutInterval = _hubOptions.ClientTimeoutInterval ?? _globalHubOptions.ClientTimeoutInterval ?? HubOptionsSetup.DefaultClientTimeoutInterval, StreamBufferCapacity = _hubOptions.StreamBufferCapacity ?? _globalHubOptions.StreamBufferCapacity ?? HubOptionsSetup.DefaultStreamBufferCapacity, MaximumReceiveMessageSize = _maximumMessageSize, + SystemClock = SystemClock, }; Log.ConnectedStarting(_logger); @@ -223,8 +228,6 @@ namespace Microsoft.AspNetCore.SignalR var result = await input.ReadAsync(); var buffer = result.Buffer; - connection.ResetClientTimeout(); - try { if (result.IsCanceled) diff --git a/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj b/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj index 9a18d8ac74..385a449849 100644 --- a/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj +++ b/src/SignalR/server/Core/src/Microsoft.AspNetCore.SignalR.Core.csproj @@ -14,6 +14,8 @@ + + diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/MockSystemClock.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/MockSystemClock.cs new file mode 100644 index 0000000000..2cd2eb617d --- /dev/null +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/MockSystemClock.cs @@ -0,0 +1,47 @@ +// 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.Threading; +using Microsoft.AspNetCore.Internal; + +namespace Microsoft.AspNetCore.SignalR.Tests +{ + public class MockSystemClock : ISystemClock + { + private static Random _random = new Random(); + + private long _utcNowTicks; + + public MockSystemClock() + { + // Use a random DateTimeOffset to ensure tests that incorrectly use the current DateTimeOffset fail always instead of only rarely. + // Pick a date between the min DateTimeOffset and a day before the max DateTimeOffset so there's room to advance the clock. + _utcNowTicks = NextLong(DateTimeOffset.MinValue.Ticks, DateTimeOffset.MaxValue.Ticks - TimeSpan.FromDays(1).Ticks); + } + + public DateTimeOffset UtcNow + { + get + { + UtcNowCalled++; + return new DateTimeOffset(Interlocked.Read(ref _utcNowTicks), TimeSpan.Zero); + } + set + { + Interlocked.Exchange(ref _utcNowTicks, value.Ticks); + } + } + + public long UtcNowTicks => UtcNow.Ticks; + + public DateTimeOffset UtcNowUnsynchronized => UtcNow; + + public int UtcNowCalled { get; private set; } + + private long NextLong(long minValue, long maxValue) + { + return (long)(_random.NextDouble() * (maxValue - minValue) + minValue); + } + } +} diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs index aaf4cfa582..4439cf3bb9 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs @@ -18,6 +18,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Connections.Features; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Protocol; using Microsoft.AspNetCore.Testing; @@ -2659,24 +2660,25 @@ namespace Microsoft.AspNetCore.SignalR.Tests { using (StartVerifiableLog()) { + var interval = 100; + var clock = new MockSystemClock(); var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => services.Configure(options => - options.KeepAliveInterval = TimeSpan.FromMilliseconds(100)), LoggerFactory); + options.KeepAliveInterval = TimeSpan.FromMilliseconds(interval)), LoggerFactory); var connectionHandler = serviceProvider.GetService>(); + connectionHandler.SystemClock = clock; using (var client = new TestClient(new NewtonsoftJsonHubProtocol())) { var connectionHandlerTask = await client.ConnectAsync(connectionHandler); await client.Connected.OrTimeout(); - // Wait 500 ms, but make sure to yield some time up to unblock concurrent threads - // This is useful on AppVeyor because it's slow enough to end up with no time - // being available for the endpoint to run. - for (var i = 0; i < 50; i += 1) + // Trigger multiple keep alives + var heartbeatCount = 5; + for (var i = 0; i < heartbeatCount; i++) { + clock.UtcNow = clock.UtcNow.AddMilliseconds(interval + 1); client.TickHeartbeat(); - await Task.Yield(); - await Task.Delay(10); } // Shut down @@ -2710,7 +2712,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests break; } } - Assert.InRange(pingCounter, 1, Int32.MaxValue); + Assert.Equal(heartbeatCount, pingCounter); } } } @@ -2720,10 +2722,13 @@ namespace Microsoft.AspNetCore.SignalR.Tests { using (StartVerifiableLog()) { + var timeout = 100; + var clock = new MockSystemClock(); var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => services.Configure(options => - options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(100)), LoggerFactory); + options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(timeout)), LoggerFactory); var connectionHandler = serviceProvider.GetService>(); + connectionHandler.SystemClock = clock; using (var client = new TestClient(new NewtonsoftJsonHubProtocol())) { @@ -2731,9 +2736,16 @@ namespace Microsoft.AspNetCore.SignalR.Tests await client.Connected.OrTimeout(); // This is a fake client -- it doesn't auto-ping to signal - // We go over the 100 ms timeout interval... - await Task.Delay(120); - client.TickHeartbeat(); + // We go over the 100 ms timeout interval multiple times + for (var i = 0; i < 3; i++) + { + clock.UtcNow = clock.UtcNow.AddMilliseconds(timeout + 1); + client.TickHeartbeat(); + } + + // Invoke a Hub method and wait for the result to reliably test if the connection is still active + var id = await client.SendInvocationAsync(nameof(MethodHub.ValueMethod)).OrTimeout(); + var result = await client.ReadAsync().OrTimeout(); // but client should still be open, since it never pinged to activate the timeout checking Assert.False(connectionHandlerTask.IsCompleted); @@ -2746,10 +2758,13 @@ namespace Microsoft.AspNetCore.SignalR.Tests { using (StartVerifiableLog()) { + var timeout = 100; + var clock = new MockSystemClock(); var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => services.Configure(options => - options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(100)), LoggerFactory); + options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(timeout)), LoggerFactory); var connectionHandler = serviceProvider.GetService>(); + connectionHandler.SystemClock = clock; using (var client = new TestClient(new NewtonsoftJsonHubProtocol())) { @@ -2757,10 +2772,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests await client.Connected.OrTimeout(); await client.SendHubMessageAsync(PingMessage.Instance); - await Task.Delay(300); - client.TickHeartbeat(); - - await Task.Delay(300); + clock.UtcNow = clock.UtcNow.AddMilliseconds(timeout + 1); client.TickHeartbeat(); await connectionHandlerTask.OrTimeout(); @@ -2774,10 +2786,13 @@ namespace Microsoft.AspNetCore.SignalR.Tests { using (StartVerifiableLog()) { + var timeout = 300; + var clock = new MockSystemClock(); var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services => services.Configure(options => - options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(300)), LoggerFactory); + options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(timeout)), LoggerFactory); var connectionHandler = serviceProvider.GetService>(); + connectionHandler.SystemClock = clock; using (var client = new TestClient(new NewtonsoftJsonHubProtocol())) { @@ -2787,11 +2802,17 @@ namespace Microsoft.AspNetCore.SignalR.Tests for (int i = 0; i < 10; i++) { - await Task.Delay(100); + clock.UtcNow = clock.UtcNow.AddMilliseconds(timeout - 1); client.TickHeartbeat(); await client.SendHubMessageAsync(PingMessage.Instance); } + // Invoke a Hub method and wait for the result to reliably test if the connection is still active + var id = await client.SendInvocationAsync(nameof(MethodHub.ValueMethod)).OrTimeout(); + var result = await client.ReadAsync().OrTimeout(); + + Assert.IsType(result); + Assert.False(connectionHandlerTask.IsCompleted); } }