Skip to content

Commit 961d1ec

Browse files
BrennanConroyBrennanConroy
committed
Merged PR 36022: 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 ecd58f3 commit 961d1ec

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
@@ -120,7 +120,6 @@ public TimeSpan TransportSendTimeout
120120
}
121121

122122
_transportSendTimeout = value;
123-
TransportSendTimeoutTicks = value.Ticks;
124123
}
125124
}
126125

@@ -133,7 +132,6 @@ public TimeSpan TransportSendTimeout
133132
/// </remarks>
134133
public bool CloseOnAuthenticationExpiration { 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
@@ -45,7 +45,7 @@ internal sealed partial class HttpConnectionContext : ConnectionContext,
4545

4646
private CancellationTokenSource? _sendCts;
4747
private bool _activeSend;
48-
private long _startedSendTime;
48+
private TimeSpan _startedSendTime;
4949
private readonly object _sendingLock = new object();
5050
internal CancellationToken SendingToken { get; private set; }
5151

@@ -65,7 +65,7 @@ public HttpConnectionContext(string connectionId, string connectionToken, ILogge
6565

6666
ConnectionId = connectionId;
6767
ConnectionToken = connectionToken;
68-
LastSeenTicks = Environment.TickCount64;
68+
LastSeenTicks = TimeSpan.FromMilliseconds(Environment.TickCount64);
6969
_options = options;
7070

7171
// The default behavior is that both formats are supported.
@@ -118,9 +118,9 @@ public HttpConnectionContext(string connectionId, string connectionToken, ILogge
118118

119119
public Task? ApplicationTask { get; set; }
120120

121-
public long LastSeenTicks { get; set; }
121+
public TimeSpan LastSeenTicks { get; set; }
122122

123-
public long? LastSeenTicksIfInactive
123+
public TimeSpan? LastSeenTicksIfInactive
124124
{
125125
get
126126
{
@@ -541,7 +541,7 @@ public void MarkInactive()
541541
if (Status == HttpConnectionStatus.Active)
542542
{
543543
Status = HttpConnectionStatus.Inactive;
544-
LastSeenTicks = Environment.TickCount64;
544+
LastSeenTicks = TimeSpan.FromMilliseconds(Environment.TickCount64);
545545
}
546546
}
547547
}
@@ -573,12 +573,12 @@ internal void StartSendCancellation()
573573
_sendCts = new CancellationTokenSource();
574574
SendingToken = _sendCts.Token;
575575
}
576-
_startedSendTime = Environment.TickCount64;
576+
_startedSendTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
577577
_activeSend = true;
578578
}
579579
}
580580

581-
internal void TryCancelSend(long currentTicks)
581+
internal void TryCancelSend(TimeSpan currentTicks)
582582
{
583583
if (!_options.TransportSendTimeoutEnabled)
584584
{
@@ -589,7 +589,7 @@ internal void TryCancelSend(long currentTicks)
589589
{
590590
if (_activeSend)
591591
{
592-
if (currentTicks - _startedSendTime > _options.TransportSendTimeoutTicks)
592+
if (currentTicks - _startedSendTime > _options.TransportSendTimeout)
593593
{
594594
_sendCts!.Cancel();
595595

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ 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

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

3636
// Register these last as the callbacks could run immediately
3737
appLifetime.ApplicationStarted.Register(() => Start());
@@ -134,7 +134,7 @@ private async Task ExecuteTimerLoop()
134134
public void Scan()
135135
{
136136
var now = DateTimeOffset.UtcNow;
137-
var ticks = Environment.TickCount64;
137+
var ticks = TimeSpan.FromMilliseconds(Environment.TickCount64);
138138

139139
// Scan the registered connections looking for ones that have timed out
140140
foreach (var c in _connections)
@@ -145,7 +145,7 @@ public void Scan()
145145

146146
// Once the decision has been made to dispose we don't check the status again
147147
// But don't clean up connections while the debugger is attached.
148-
if (!Debugger.IsAttached && lastSeenTick.HasValue && (ticks - lastSeenTick.Value) > _disconnectTimeoutTicks)
148+
if (!Debugger.IsAttached && lastSeenTick.HasValue && (ticks - lastSeenTick.Value) > _disconnectTimeout)
149149
{
150150
Log.ConnectionTimedOut(_logger, connection.ConnectionId);
151151
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
@@ -557,7 +557,7 @@ public async Task TransportEndingGracefullyWaitsOnApplicationLongPolling()
557557
await task.DefaultTimeout();
558558

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

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

11911191
using (StartVerifiableLog(expectedErrorsFilter: ExpectedErrors))
11921192
{
1193+
var initialTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
11931194
var manager = CreateConnectionManager(LoggerFactory);
11941195
var connection = manager.CreateConnection();
11951196
connection.TransportType = HttpTransportType.LongPolling;
@@ -1200,16 +1201,23 @@ bool ExpectedErrors(WriteContext writeContext)
12001201
var builder = new ConnectionBuilder(services.BuildServiceProvider());
12011202
builder.UseConnectionHandler<TestConnectionHandler>();
12021203
var app = builder.Build();
1203-
var options = new HttpConnectionDispatcherOptions();
12041204
// First poll completes immediately
1205+
var options = new HttpConnectionDispatcherOptions();
12051206
await dispatcher.ExecuteAsync(context, options, app).DefaultTimeout();
12061207
var sync = new SyncPoint();
12071208
context.Response.Body = new BlockingStream(sync);
12081209
var dispatcherTask = dispatcher.ExecuteAsync(context, options, app);
12091210
await connection.Transport.Output.WriteAsync(new byte[] { 1 }).DefaultTimeout();
12101211
await sync.WaitForSyncPoint().DefaultTimeout();
1212+
1213+
// Try cancel before cancellation should occur
1214+
connection.TryCancelSend(initialTime + options.TransportSendTimeout);
1215+
Assert.False(connection.SendingToken.IsCancellationRequested);
1216+
12111217
// Cancel write to response body
1212-
connection.TryCancelSend(long.MaxValue);
1218+
connection.TryCancelSend(TimeSpan.FromMilliseconds(Environment.TickCount64) + options.TransportSendTimeout + TimeSpan.FromTicks(1));
1219+
Assert.True(connection.SendingToken.IsCancellationRequested);
1220+
12131221
sync.Continue();
12141222
await dispatcherTask.DefaultTimeout();
12151223
// Connection should be removed on canceled write
@@ -1223,6 +1231,7 @@ public async Task SSEConnectionClosesWhenSendTimeoutReached()
12231231
{
12241232
using (StartVerifiableLog())
12251233
{
1234+
var initialTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
12261235
var manager = CreateConnectionManager(LoggerFactory);
12271236
var connection = manager.CreateConnection();
12281237
connection.TransportType = HttpTransportType.ServerSentEvents;
@@ -1240,8 +1249,15 @@ public async Task SSEConnectionClosesWhenSendTimeoutReached()
12401249
var dispatcherTask = dispatcher.ExecuteAsync(context, options, app);
12411250
await connection.Transport.Output.WriteAsync(new byte[] { 1 }).DefaultTimeout();
12421251
await sync.WaitForSyncPoint().DefaultTimeout();
1252+
1253+
// Try cancel before cancellation should occur
1254+
connection.TryCancelSend(initialTime + options.TransportSendTimeout);
1255+
Assert.False(connection.SendingToken.IsCancellationRequested);
1256+
12431257
// Cancel write to response body
1244-
connection.TryCancelSend(long.MaxValue);
1258+
connection.TryCancelSend(TimeSpan.FromMilliseconds(Environment.TickCount64) + options.TransportSendTimeout + TimeSpan.FromTicks(1));
1259+
Assert.True(connection.SendingToken.IsCancellationRequested);
1260+
12451261
sync.Continue();
12461262
await dispatcherTask.DefaultTimeout();
12471263
// Connection should be removed on canceled write
@@ -1260,6 +1276,7 @@ bool ExpectedErrors(WriteContext writeContext)
12601276
}
12611277
using (StartVerifiableLog(expectedErrorsFilter: ExpectedErrors))
12621278
{
1279+
var initialTime = TimeSpan.FromMilliseconds(Environment.TickCount64);
12631280
var manager = CreateConnectionManager(LoggerFactory);
12641281
var connection = manager.CreateConnection();
12651282
connection.TransportType = HttpTransportType.WebSockets;
@@ -1277,8 +1294,15 @@ bool ExpectedErrors(WriteContext writeContext)
12771294
var dispatcherTask = dispatcher.ExecuteAsync(context, options, app);
12781295
await connection.Transport.Output.WriteAsync(new byte[] { 1 }).DefaultTimeout();
12791296
await sync.WaitForSyncPoint().DefaultTimeout();
1297+
1298+
// Try cancel before cancellation should occur
1299+
connection.TryCancelSend(initialTime + options.TransportSendTimeout);
1300+
Assert.False(connection.SendingToken.IsCancellationRequested);
1301+
12801302
// Cancel write to response body
1281-
connection.TryCancelSend(long.MaxValue);
1303+
connection.TryCancelSend(TimeSpan.FromMilliseconds(Environment.TickCount64) + options.TransportSendTimeout + TimeSpan.FromTicks(1));
1304+
Assert.True(connection.SendingToken.IsCancellationRequested);
1305+
12821306
sync.Continue();
12831307
await dispatcherTask.DefaultTimeout();
12841308
// Connection should be removed on canceled write

0 commit comments

Comments
 (0)