Skip to content

Replace ISystemClock in SignalR #47895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions src/SignalR/common/Shared/ISystemClock.cs

This file was deleted.

13 changes: 0 additions & 13 deletions src/SignalR/common/Shared/SystemClock.cs

This file was deleted.

32 changes: 16 additions & 16 deletions src/SignalR/server/Core/src/HubConnectionContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
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;
Expand All @@ -29,11 +28,11 @@ public partial class HubConnectionContext
private readonly ILogger _logger;
private readonly CancellationTokenSource _connectionAbortedTokenSource = new CancellationTokenSource();
private readonly TaskCompletionSource _abortCompletedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
private readonly long _keepAliveInterval;
private readonly long _clientTimeoutInterval;
private readonly TimeSpan _keepAliveInterval;
private readonly TimeSpan _clientTimeoutInterval;
private readonly SemaphoreSlim _writeLock = new SemaphoreSlim(1);
private readonly object _receiveMessageTimeoutLock = new object();
private readonly ISystemClock _systemClock;
private readonly TimeProvider _timeProvider;
private readonly CancellationTokenRegistration _closedRegistration;
private readonly CancellationTokenRegistration? _closedRequestedRegistration;

Expand All @@ -46,7 +45,7 @@ public partial class HubConnectionContext
private readonly int _streamBufferCapacity;
private readonly long? _maxMessageSize;
private bool _receivedMessageTimeoutEnabled;
private long _receivedMessageElapsedTicks;
private TimeSpan _receivedMessageElapsed;
private long _receivedMessageTick;
private ClaimsPrincipal? _user;

Expand All @@ -58,8 +57,9 @@ public partial class HubConnectionContext
/// <param name="contextOptions">The options to configure the HubConnectionContext.</param>
public HubConnectionContext(ConnectionContext connectionContext, HubConnectionContextOptions contextOptions, ILoggerFactory loggerFactory)
{
_keepAliveInterval = (long)contextOptions.KeepAliveInterval.TotalMilliseconds;
_clientTimeoutInterval = (long)contextOptions.ClientTimeoutInterval.TotalMilliseconds;
_timeProvider = contextOptions.TimeProvider ?? TimeProvider.System;
_keepAliveInterval = contextOptions.KeepAliveInterval;
_clientTimeoutInterval = contextOptions.ClientTimeoutInterval;
_streamBufferCapacity = contextOptions.StreamBufferCapacity;
_maxMessageSize = contextOptions.MaximumReceiveMessageSize;

Expand All @@ -76,8 +76,7 @@ public HubConnectionContext(ConnectionContext connectionContext, HubConnectionCo

HubCallerContext = new DefaultHubCallerContext(this);

_systemClock = contextOptions.SystemClock ?? new SystemClock();
_lastSendTick = _systemClock.CurrentTicks;
_lastSendTick = _timeProvider.GetTimestamp();

var maxInvokeLimit = contextOptions.MaximumParallelInvocations;
ActiveInvocationLimit = new ChannelBasedSemaphore(maxInvokeLimit);
Expand Down Expand Up @@ -614,15 +613,16 @@ private async Task AbortAsyncSlow()

private void KeepAliveTick()
{
var currentTime = _systemClock.CurrentTicks;
var currentTime = _timeProvider.GetTimestamp();
var elapsed = _timeProvider.GetElapsedTime(Volatile.Read(ref _lastSendTick), currentTime);

// 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).
// If it is, we send a ping frame, if not, we no-op on this tick. This means that in the worst case, the
// true "ping rate" of the server could be (_hubOptions.KeepAliveInterval + HubEndPoint.KeepAliveTimerInterval),
// because if the interval elapses right after the last tick of this timer, it won't be detected until the next tick.

if (currentTime - Volatile.Read(ref _lastSendTick) > _keepAliveInterval)
if (elapsed > _keepAliveInterval)
{
// Haven't sent a message for the entire keep-alive duration, so send a ping.
// If the transport channel is full, this will fail, but that's OK because
Expand Down Expand Up @@ -657,11 +657,11 @@ private void CheckClientTimeout()
{
if (_receivedMessageTimeoutEnabled)
{
_receivedMessageElapsedTicks = _systemClock.CurrentTicks - _receivedMessageTick;
_receivedMessageElapsed = _timeProvider.GetElapsedTime(_receivedMessageTick);

if (_receivedMessageElapsedTicks >= _clientTimeoutInterval)
if (_receivedMessageElapsed >= _clientTimeoutInterval)
{
Log.ClientTimeout(_logger, TimeSpan.FromMilliseconds(_clientTimeoutInterval));
Log.ClientTimeout(_logger, _clientTimeoutInterval);
AbortAllowReconnect();
}
}
Expand Down Expand Up @@ -707,7 +707,7 @@ internal void BeginClientTimeout()
lock (_receiveMessageTimeoutLock)
{
_receivedMessageTimeoutEnabled = true;
_receivedMessageTick = _systemClock.CurrentTicks;
_receivedMessageTick = _timeProvider.GetTimestamp();
}
}

Expand All @@ -717,7 +717,7 @@ internal void StopClientTimeout()
{
// we received a message so stop the timer and reset it
// it will resume after the message has been processed
_receivedMessageElapsedTicks = 0;
_receivedMessageElapsed = TimeSpan.Zero;
_receivedMessageTick = 0;
_receivedMessageTimeoutEnabled = false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Internal;

namespace Microsoft.AspNetCore.SignalR;

/// <summary>
Expand Down Expand Up @@ -30,7 +28,7 @@ public class HubConnectionContextOptions
/// </summary>
public long? MaximumReceiveMessageSize { get; set; }

internal ISystemClock SystemClock { get; set; } = default!;
internal TimeProvider TimeProvider { get; set; } = default!;

/// <summary>
/// Gets or sets the maximum parallel hub method invocations.
Expand Down
5 changes: 2 additions & 3 deletions src/SignalR/server/Core/src/HubConnectionHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Linq;
using Microsoft.AspNetCore.Connections;
using Microsoft.AspNetCore.Internal;
using Microsoft.AspNetCore.SignalR.Internal;
using Microsoft.AspNetCore.SignalR.Protocol;
using Microsoft.Extensions.DependencyInjection;
Expand Down Expand Up @@ -31,7 +30,7 @@ public class HubConnectionHandler<THub> : ConnectionHandler where THub : Hub
private readonly int _maxParallelInvokes;

// Internal for testing
internal ISystemClock SystemClock { get; set; } = new SystemClock();
internal TimeProvider TimeProvider { get; set; } = TimeProvider.System;

/// <summary>
/// Initializes a new instance of the <see cref="HubConnectionHandler{THub}"/> class.
Expand Down Expand Up @@ -120,7 +119,7 @@ public override async Task OnConnectedAsync(ConnectionContext connection)
ClientTimeoutInterval = _hubOptions.ClientTimeoutInterval ?? _globalHubOptions.ClientTimeoutInterval ?? HubOptionsSetup.DefaultClientTimeoutInterval,
StreamBufferCapacity = _hubOptions.StreamBufferCapacity ?? _globalHubOptions.StreamBufferCapacity ?? HubOptionsSetup.DefaultStreamBufferCapacity,
MaximumReceiveMessageSize = _maximumMessageSize,
SystemClock = SystemClock,
TimeProvider = TimeProvider,
MaximumParallelInvocations = _maxParallelInvokes,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
<Compile Include="$(SharedSourceRoot)ObjectMethodExecutor\*.cs" />
<Compile Include="$(SignalRSharedSourceRoot)AsyncEnumerableAdapters.cs" Link="Internal\AsyncEnumerableAdapters.cs" />
<Compile Include="$(SignalRSharedSourceRoot)TaskCache.cs" Link="Internal\TaskCache.cs" />
<Compile Include="$(SignalRSharedSourceRoot)ISystemClock.cs" Link="Internal\ISystemClock.cs" />
<Compile Include="$(SignalRSharedSourceRoot)SystemClock.cs" Link="Internal\SystemClock.cs" />
<Compile Include="$(SignalRSharedSourceRoot)ClientResultsManager.cs" Link="Internal\ClientResultsManager.cs" />
<Compile Include="$(SignalRSharedSourceRoot)CreateLinkedToken.cs" Link="Internal\CreateLinkedToken.cs" />
<Compile Include="$(SharedSourceRoot)ThrowHelpers\ArgumentNullThrowHelper.cs" LinkBase="Shared" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,27 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.AspNetCore.Internal;

namespace Microsoft.AspNetCore.SignalR.Tests;

public class MockSystemClock : ISystemClock
public class TestTimeProvider : TimeProvider
{
private long _nowTicks;

public MockSystemClock()
public TestTimeProvider()
{
// 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.
_nowTicks = NextLong(0, long.MaxValue - (long)TimeSpan.FromDays(1).TotalMilliseconds);
_nowTicks = NextLong(0, long.MaxValue - (long)TimeSpan.FromDays(1).TotalSeconds) * TimestampFrequency;
}

public long CurrentTicks
public override long GetTimestamp() => _nowTicks;

public void Advance(TimeSpan offset)
{
get => _nowTicks;
set
{
Interlocked.Exchange(ref _nowTicks, value);
}
Interlocked.Add(ref _nowTicks, (long)(offset.TotalSeconds * TimestampFrequency));
}

private long NextLong(long minValue, long maxValue)
private static long NextLong(long minValue, long maxValue)
{
return (long)(Random.Shared.NextDouble() * (maxValue - minValue) + minValue);
}
Expand Down
40 changes: 20 additions & 20 deletions src/SignalR/server/SignalR/test/HubConnectionHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2735,13 +2735,13 @@ public async Task WritesPingMessageIfNothingWrittenWhenKeepAliveIntervalElapses(
{
using (StartVerifiableLog())
{
var intervalInMS = 100;
var clock = new MockSystemClock();
var interval = TimeSpan.FromMilliseconds(100);
var timeProvider = new TestTimeProvider();
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services =>
services.Configure<HubOptions>(options =>
options.KeepAliveInterval = TimeSpan.FromMilliseconds(intervalInMS)), LoggerFactory);
options.KeepAliveInterval = interval), LoggerFactory);
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<MethodHub>>();
connectionHandler.SystemClock = clock;
connectionHandler.TimeProvider = timeProvider;

using (var client = new TestClient(new NewtonsoftJsonHubProtocol()))
{
Expand All @@ -2752,7 +2752,7 @@ public async Task WritesPingMessageIfNothingWrittenWhenKeepAliveIntervalElapses(
var heartbeatCount = 5;
for (var i = 0; i < heartbeatCount; i++)
{
clock.CurrentTicks = clock.CurrentTicks + intervalInMS + 1;
timeProvider.Advance(interval + TimeSpan.FromMilliseconds(1));
client.TickHeartbeat();
}

Expand Down Expand Up @@ -2797,13 +2797,13 @@ public async Task ConnectionNotTimedOutIfClientNeverPings()
{
using (StartVerifiableLog())
{
var timeoutInMS = 100;
var clock = new MockSystemClock();
var timeout = TimeSpan.FromMilliseconds(100);
var timeProvider = new TestTimeProvider();
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services =>
services.Configure<HubOptions>(options =>
options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(timeoutInMS)), LoggerFactory);
options.ClientTimeoutInterval = timeout), LoggerFactory);
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<MethodHub>>();
connectionHandler.SystemClock = clock;
connectionHandler.TimeProvider = timeProvider;

using (var client = new TestClient(new NewtonsoftJsonHubProtocol()))
{
Expand All @@ -2814,7 +2814,7 @@ public async Task ConnectionNotTimedOutIfClientNeverPings()
// We go over the 100 ms timeout interval multiple times
for (var i = 0; i < 3; i++)
{
clock.CurrentTicks = clock.CurrentTicks + timeoutInMS + 1;
timeProvider.Advance(timeout + TimeSpan.FromMilliseconds(1));
client.TickHeartbeat();
}

Expand All @@ -2833,21 +2833,21 @@ public async Task ConnectionTimesOutIfInitialPingAndThenNoMessages()
{
using (StartVerifiableLog())
{
var timeoutInMS = 100;
var clock = new MockSystemClock();
var timeout = TimeSpan.FromMilliseconds(100);
var timeProvider = new TestTimeProvider();
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services =>
services.Configure<HubOptions>(options =>
options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(timeoutInMS)), LoggerFactory);
options.ClientTimeoutInterval = timeout), LoggerFactory);
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<MethodHub>>();
connectionHandler.SystemClock = clock;
connectionHandler.TimeProvider = timeProvider;

using (var client = new TestClient(new NewtonsoftJsonHubProtocol()))
{
var connectionHandlerTask = await client.ConnectAsync(connectionHandler);
await client.Connected.DefaultTimeout();
await client.SendHubMessageAsync(PingMessage.Instance);

clock.CurrentTicks = clock.CurrentTicks + timeoutInMS + 1;
timeProvider.Advance(timeout + TimeSpan.FromMilliseconds(1));
client.TickHeartbeat();

await connectionHandlerTask.DefaultTimeout();
Expand All @@ -2860,13 +2860,13 @@ public async Task ReceivingMessagesPreventsConnectionTimeoutFromOccuring()
{
using (StartVerifiableLog())
{
var timeoutInMS = 300;
var clock = new MockSystemClock();
var timeout = TimeSpan.FromMilliseconds(300);
var timeProvider = new TestTimeProvider();
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(services =>
services.Configure<HubOptions>(options =>
options.ClientTimeoutInterval = TimeSpan.FromMilliseconds(timeoutInMS)), LoggerFactory);
options.ClientTimeoutInterval = timeout), LoggerFactory);
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<MethodHub>>();
connectionHandler.SystemClock = clock;
connectionHandler.TimeProvider = timeProvider;

using (var client = new TestClient(new NewtonsoftJsonHubProtocol()))
{
Expand All @@ -2876,7 +2876,7 @@ public async Task ReceivingMessagesPreventsConnectionTimeoutFromOccuring()

for (int i = 0; i < 10; i++)
{
clock.CurrentTicks = clock.CurrentTicks + timeoutInMS - 1;
timeProvider.Advance(timeout - TimeSpan.FromMilliseconds(1));
client.TickHeartbeat();
await client.SendHubMessageAsync(PingMessage.Instance);
}
Expand Down