Skip to content

Commit 35e1129

Browse files
BrennanConroyBrennanConroy
committed
Merged PR 36023: Fix time scale
We converted UtcNow.Ticks usage to Environment.TickCount64 usage. TickCount64 is in milliseconds and UtcNow.Ticks is in the 100-nanosecond scale. When we made this change we didn't convert TickCount64 to the same scale so comparisons were wrong.
1 parent fbe258e commit 35e1129

File tree

4 files changed

+41
-19
lines changed

4 files changed

+41
-19
lines changed

‎src/SignalR/common/Http.Connections/src/HttpConnectionDispatcherOptions.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ public TimeSpan TransportSendTimeout
112112
ArgumentOutOfRangeException.ThrowIfEqual(value, TimeSpan.Zero);
113113

114114
_transportSendTimeout = value;
115-
TransportSendTimeoutTicks = value.Ticks;
116115
}
117116
}
118117

@@ -133,7 +132,6 @@ public TimeSpan TransportSendTimeout
133132
/// </remarks>
134133
public bool AllowStatefulReconnects { get; set; }
135134

136-
internal long TransportSendTimeoutTicks { get; private set; }
137135
internal bool TransportSendTimeoutEnabled => _transportSendTimeout != Timeout.InfiniteTimeSpan;
138136

139137
// We initialize these lazily based on the state of the options specified here.

‎src/SignalR/common/Http.Connections/src/Internal/HttpConnectionContext.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ internal sealed partial class HttpConnectionContext : ConnectionContext,
5050

5151
private CancellationTokenSource? _sendCts;
5252
private bool _activeSend;
53-
private long _startedSendTime;
53+
private TimeSpan _startedSendTime;
5454
private bool _useStatefulReconnect;
5555
private readonly object _sendingLock = new object();
5656
internal CancellationToken SendingToken { get; private set; }
@@ -74,7 +74,7 @@ public HttpConnectionContext(string connectionId, string connectionToken, ILogge
7474

7575
ConnectionId = connectionId;
7676
ConnectionToken = connectionToken;
77-
LastSeenTicks = Environment.TickCount64;
77+
LastSeenTicks = TimeSpan.FromMilliseconds(Environment.TickCount64);
7878
_options = options;
7979

8080
// The default behavior is that both formats are supported.
@@ -140,9 +140,9 @@ public HttpConnectionContext(string connectionId, string connectionToken, ILogge
140140

141141
public Task? ApplicationTask { get; set; }
142142

143-
public long LastSeenTicks { get; set; }
143+
public TimeSpan LastSeenTicks { get; set; }
144144

145-
public long? LastSeenTicksIfInactive
145+
public TimeSpan? LastSeenTicksIfInactive
146146
{
147147
get
148148
{
@@ -618,7 +618,7 @@ public void MarkInactive()
618618
if (Status == HttpConnectionStatus.Active)
619619
{
620620
Status = HttpConnectionStatus.Inactive;
621-
LastSeenTicks = Environment.TickCount64;
621+
LastSeenTicks = TimeSpan.FromMilliseconds(Environment.TickCount64);
622622
}
623623
}
624624
}
@@ -650,12 +650,12 @@ internal void StartSendCancellation()
650650
_sendCts = new CancellationTokenSource();
651651
SendingToken = _sendCts.Token;
652652
}
653-
_startedSendTime = Environment.TickCount64;
653+
_startedSendTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
654654
_activeSend = true;
655655
}
656656
}
657657

658-
internal void TryCancelSend(long currentTicks)
658+
internal void TryCancelSend(TimeSpan currentTicks)
659659
{
660660
if (!_options.TransportSendTimeoutEnabled)
661661
{
@@ -666,7 +666,7 @@ internal void TryCancelSend(long currentTicks)
666666
{
667667
if (_activeSend)
668668
{
669-
if (currentTicks - _startedSendTime > _options.TransportSendTimeoutTicks)
669+
if (currentTicks - _startedSendTime > _options.TransportSendTimeout)
670670
{
671671
_sendCts!.Cancel();
672672

‎src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,15 @@ internal sealed partial class HttpConnectionManager
2424
private readonly PeriodicTimer _nextHeartbeat;
2525
private readonly ILogger<HttpConnectionManager> _logger;
2626
private readonly ILogger<HttpConnectionContext> _connectionLogger;
27-
private readonly long _disconnectTimeoutTicks;
27+
private readonly TimeSpan _disconnectTimeout;
2828
private readonly HttpConnectionsMetrics _metrics;
2929

3030
public HttpConnectionManager(ILoggerFactory loggerFactory, IHostApplicationLifetime appLifetime, IOptions<ConnectionOptions> connectionOptions, HttpConnectionsMetrics metrics)
3131
{
3232
_logger = loggerFactory.CreateLogger<HttpConnectionManager>();
3333
_connectionLogger = loggerFactory.CreateLogger<HttpConnectionContext>();
3434
_nextHeartbeat = new PeriodicTimer(_heartbeatTickRate);
35-
_disconnectTimeoutTicks = (long)(connectionOptions.Value.DisconnectTimeout ?? ConnectionOptionsSetup.DefaultDisconectTimeout).TotalMilliseconds;
35+
_disconnectTimeout = connectionOptions.Value.DisconnectTimeout ?? ConnectionOptionsSetup.DefaultDisconectTimeout;
3636
_metrics = metrics;
3737

3838
// Register these last as the callbacks could run immediately
@@ -141,7 +141,7 @@ private async Task ExecuteTimerLoop()
141141
public void Scan()
142142
{
143143
var now = DateTimeOffset.UtcNow;
144-
var ticks = Environment.TickCount64;
144+
var ticks = TimeSpan.FromMilliseconds(Environment.TickCount64);
145145

146146
// Scan the registered connections looking for ones that have timed out
147147
foreach (var c in _connections)
@@ -152,7 +152,7 @@ public void Scan()
152152

153153
// Once the decision has been made to dispose we don't check the status again
154154
// But don't clean up connections while the debugger is attached.
155-
if (!Debugger.IsAttached && lastSeenTick.HasValue && (ticks - lastSeenTick.Value) > _disconnectTimeoutTicks)
155+
if (!Debugger.IsAttached && lastSeenTick.HasValue && (ticks - lastSeenTick.Value) > _disconnectTimeout)
156156
{
157157
Log.ConnectionTimedOut(_logger, connection.ConnectionId);
158158
HttpConnectionsEventSource.Log.ConnectionTimedOut(connection.ConnectionId);

‎src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ public async Task TransportEndingGracefullyWaitsOnApplicationLongPolling()
555555
await task.DefaultTimeout();
556556

557557
// We've been gone longer than the expiration time
558-
connection.LastSeenTicks = Environment.TickCount64 - (long)disconnectTimeout.TotalMilliseconds - 1;
558+
connection.LastSeenTicks = TimeSpan.FromMilliseconds(Environment.TickCount64) - disconnectTimeout - TimeSpan.FromTicks(1);
559559

560560
// The application is still running here because the poll is only killed
561561
// by the heartbeat so we pretend to do a scan and this should force the application task to complete
@@ -1277,6 +1277,7 @@ bool ExpectedErrors(WriteContext writeContext)
12771277

12781278
using (StartVerifiableLog(expectedErrorsFilter: ExpectedErrors))
12791279
{
1280+
var initialTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
12801281
var manager = CreateConnectionManager(LoggerFactory);
12811282
var connection = manager.CreateConnection();
12821283
connection.TransportType = HttpTransportType.LongPolling;
@@ -1287,16 +1288,23 @@ bool ExpectedErrors(WriteContext writeContext)
12871288
var builder = new ConnectionBuilder(services.BuildServiceProvider());
12881289
builder.UseConnectionHandler<TestConnectionHandler>();
12891290
var app = builder.Build();
1290-
var options = new HttpConnectionDispatcherOptions();
12911291
// First poll completes immediately
1292+
var options = new HttpConnectionDispatcherOptions();
12921293
await dispatcher.ExecuteAsync(context, options, app).DefaultTimeout();
12931294
var sync = new SyncPoint();
12941295
context.Response.Body = new BlockingStream(sync);
12951296
var dispatcherTask = dispatcher.ExecuteAsync(context, options, app);
12961297
await connection.Transport.Output.WriteAsync(new byte[] { 1 }).DefaultTimeout();
12971298
await sync.WaitForSyncPoint().DefaultTimeout();
1299+
1300+
// Try cancel before cancellation should occur
1301+
connection.TryCancelSend(initialTime + options.TransportSendTimeout);
1302+
Assert.False(connection.SendingToken.IsCancellationRequested);
1303+
12981304
// Cancel write to response body
1299-
connection.TryCancelSend(long.MaxValue);
1305+
connection.TryCancelSend(TimeSpan.FromMilliseconds(Environment.TickCount64) + options.TransportSendTimeout + TimeSpan.FromTicks(1));
1306+
Assert.True(connection.SendingToken.IsCancellationRequested);
1307+
13001308
sync.Continue();
13011309
await dispatcherTask.DefaultTimeout();
13021310
// Connection should be removed on canceled write
@@ -1310,6 +1318,7 @@ public async Task SSEConnectionClosesWhenSendTimeoutReached()
13101318
{
13111319
using (StartVerifiableLog())
13121320
{
1321+
var initialTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
13131322
var manager = CreateConnectionManager(LoggerFactory);
13141323
var connection = manager.CreateConnection();
13151324
var dispatcher = CreateDispatcher(manager, LoggerFactory);
@@ -1326,8 +1335,15 @@ public async Task SSEConnectionClosesWhenSendTimeoutReached()
13261335
var dispatcherTask = dispatcher.ExecuteAsync(context, options, app);
13271336
await connection.Transport.Output.WriteAsync(new byte[] { 1 }).DefaultTimeout();
13281337
await sync.WaitForSyncPoint().DefaultTimeout();
1338+
1339+
// Try cancel before cancellation should occur
1340+
connection.TryCancelSend(initialTime + options.TransportSendTimeout);
1341+
Assert.False(connection.SendingToken.IsCancellationRequested);
1342+
13291343
// Cancel write to response body
1330-
connection.TryCancelSend(long.MaxValue);
1344+
connection.TryCancelSend(TimeSpan.FromMilliseconds(Environment.TickCount64) + options.TransportSendTimeout + TimeSpan.FromTicks(1));
1345+
Assert.True(connection.SendingToken.IsCancellationRequested);
1346+
13311347
sync.Continue();
13321348
await dispatcherTask.DefaultTimeout();
13331349
// Connection should be removed on canceled write
@@ -1346,6 +1362,7 @@ bool ExpectedErrors(WriteContext writeContext)
13461362
}
13471363
using (StartVerifiableLog(expectedErrorsFilter: ExpectedErrors))
13481364
{
1365+
var initialTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
13491366
var manager = CreateConnectionManager(LoggerFactory);
13501367
var connection = manager.CreateConnection();
13511368
var dispatcher = CreateDispatcher(manager, LoggerFactory);
@@ -1362,8 +1379,15 @@ bool ExpectedErrors(WriteContext writeContext)
13621379
var dispatcherTask = dispatcher.ExecuteAsync(context, options, app);
13631380
await connection.Transport.Output.WriteAsync(new byte[] { 1 }).DefaultTimeout();
13641381
await sync.WaitForSyncPoint().DefaultTimeout();
1382+
1383+
// Try cancel before cancellation should occur
1384+
connection.TryCancelSend(initialTime + options.TransportSendTimeout);
1385+
Assert.False(connection.SendingToken.IsCancellationRequested);
1386+
13651387
// Cancel write to response body
1366-
connection.TryCancelSend(long.MaxValue);
1388+
connection.TryCancelSend(TimeSpan.FromMilliseconds(Environment.TickCount64) + options.TransportSendTimeout + TimeSpan.FromTicks(1));
1389+
Assert.True(connection.SendingToken.IsCancellationRequested);
1390+
13671391
sync.Continue();
13681392
await dispatcherTask.DefaultTimeout();
13691393
// Connection should be removed on canceled write

0 commit comments

Comments
 (0)