Avoid possible tearing in HeartbeatManager.UtcNow (#3092)

src/Kestrel.Core/Internal/Http2/Http2Connection.cs

@@ -1052,8 +1052,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                 {
                     if (stream.IsDraining)
                     {
-                        stream.DrainExpiration =
+                        stream.DrainExpirationTicks =
-                            _context.ServiceContext.SystemClock.UtcNow + Constants.RequestBodyDrainTimeout;
+                            _context.ServiceContext.SystemClock.UtcNowTicks + Constants.RequestBodyDrainTimeout.Ticks;
 
                         _drainingStreams.TryAdd(streamId, stream);
                     }
@@ -1096,7 +1096,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
             foreach (var stream in _drainingStreams)
             {
-                if (now > stream.Value.DrainExpiration)
+                if (now.Ticks > stream.Value.DrainExpirationTicks)
                 {
                     RemoveDrainingStream(stream.Key);
                 }

src/Kestrel.Core/Internal/Http2/Http2Stream.cs

@@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         private readonly StreamInputFlowControl _inputFlowControl;
         private readonly StreamOutputFlowControl _outputFlowControl;
 
-        internal DateTimeOffset DrainExpiration { get; set; }
+        internal long DrainExpirationTicks { get; set; }
 
         private StreamCompletionFlags _completionState;
         private readonly object _completionLock = new object();

src/Kestrel.Core/Internal/HttpConnection.cs

@@ -113,7 +113,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
                 using (connectionLifetimeNotificationFeature?.ConnectionClosedRequested.Register(state => ((HttpConnection)state).StopProcessingNextRequest(), this))
                 {
                     // Ensure TimeoutControl._lastTimestamp is initialized before anything that could set timeouts runs.
-                    _timeoutControl.Initialize(_systemClock.UtcNow);
+                    _timeoutControl.Initialize(_systemClock.UtcNowTicks);
 
                     _context.ConnectionFeatures.Set<IConnectionTimeoutFeature>(_timeoutControl);
 
@@ -356,7 +356,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
                 return;
             }
 
-            var now = _systemClock.UtcNow;
+            // It's safe to use UtcNowUnsynchronized since Tick is called by the Heartbeat.
+            var now = _systemClock.UtcNowUnsynchronized;
             _timeoutControl.Tick(now);
             _requestProcessor?.Tick(now);
         }

src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.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 System.Threading;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
 {
@@ -10,6 +11,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
         private readonly ConnectionManager _connectionManager;
         private readonly Action<KestrelConnection> _walkCallback;
         private DateTimeOffset _now;
+        private long _nowTicks;
 
         public HeartbeatManager(ConnectionManager connectionManager)
         {
@@ -17,11 +19,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
             _walkCallback = WalkCallback;
         }
 
-        public DateTimeOffset UtcNow => _now;
+        public DateTimeOffset UtcNow => new DateTimeOffset(UtcNowTicks, TimeSpan.Zero);
+
+        public long UtcNowTicks => Volatile.Read(ref _nowTicks);
+
+        public DateTimeOffset UtcNowUnsynchronized => _now;
 
         public void OnHeartbeat(DateTimeOffset now)
         {
             _now = now;
+            Volatile.Write(ref _nowTicks, now.Ticks);
+
             _connectionManager.Walk(_walkCallback);
         }
 

src/Kestrel.Core/Internal/Infrastructure/ISystemClock.cs

@@ -11,8 +11,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
     public interface ISystemClock
     {
         /// <summary>
-        /// Retrieves the current system time in UTC.
+        /// Retrieves the current UTC system time.
         /// </summary>
         DateTimeOffset UtcNow { get; }
+        
+        /// <summary>
+        /// Retrieves ticks for the current UTC system time.
+        /// </summary>
+        long UtcNowTicks { get; }
+
+        /// <summary>
+        /// Retrieves the current UTC system time.
+        /// This is only safe to use from code called by the <see cref="Heartbeat"/>.
+        /// </summary>
+        DateTimeOffset UtcNowUnsynchronized { get; }
     }
 }

src/Kestrel.Core/Internal/Infrastructure/SystemClock.cs

@@ -11,14 +11,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
     internal class SystemClock : ISystemClock
     {
         /// <summary>
-        /// Retrieves the current system time in UTC.
+        /// Retrieves the current UTC system time.
         /// </summary>
-        public DateTimeOffset UtcNow
+        public DateTimeOffset UtcNow => DateTimeOffset.UtcNow;
-        {
+        
-            get
+        /// <summary>
-            {
+        /// Retrieves ticks for the current UTC system time.
-                return DateTimeOffset.UtcNow;
+        /// </summary>
-            }
+        public long UtcNowTicks => DateTimeOffset.UtcNow.Ticks;
-        }
+
+        /// <summary>
+        /// Retrieves the current UTC system time.
+        /// </summary>
+        public DateTimeOffset UtcNowUnsynchronized => DateTimeOffset.UtcNow;
     }
 }

src/Kestrel.Core/Internal/Infrastructure/TimeoutControl.cs

@@ -40,9 +40,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
 
         internal IDebugger Debugger { get; set; } = DebuggerWrapper.Singleton;
 
-        public void Initialize(DateTimeOffset now)
+        internal void Initialize(long nowTicks)
         {
-            _lastTimestamp = now.Ticks;
+            _lastTimestamp = nowTicks;
         }
 
         public void Tick(DateTimeOffset now)

test/Kestrel.Core.Tests/TimeoutControlTests.cs

@@ -31,7 +31,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _timeoutControl.Debugger = mockDebugger.Object;
 
             var now = DateTimeOffset.Now;
-            _timeoutControl.Initialize(now);
+            _timeoutControl.Initialize(now.Ticks);
             _timeoutControl.SetTimeout(1, TimeoutReason.RequestHeaders);
             _timeoutControl.Tick(now.AddTicks(2).Add(Heartbeat.Interval));
 
@@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             // Initialize timestamp
             var now = DateTimeOffset.UtcNow;
-            _timeoutControl.Initialize(now);
+            _timeoutControl.Initialize(now.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();
@@ -99,7 +99,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             // Initialize timestamp
             var now = DateTimeOffset.UtcNow;
-            _timeoutControl.Initialize(now);
+            _timeoutControl.Initialize(now.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();
@@ -159,7 +159,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
 
             // Initialize timestamp
-            _timeoutControl.Initialize(_systemClock.UtcNow);
+            _timeoutControl.Initialize(_systemClock.UtcNow.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();
@@ -215,7 +215,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
 
             // Initialize timestamp
-            _timeoutControl.Initialize(_systemClock.UtcNow);
+            _timeoutControl.Initialize(_systemClock.UtcNow.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();
@@ -263,7 +263,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var startTime = _systemClock.UtcNow;
 
             // Initialize timestamp
-            _timeoutControl.Initialize(startTime);
+            _timeoutControl.Initialize(startTime.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();
@@ -295,7 +295,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             // Initialize timestamp
             var now = DateTimeOffset.UtcNow;
-            _timeoutControl.Initialize(now);
+            _timeoutControl.Initialize(now.Ticks);
             _timeoutControl.InitializeHttp2(flowControl);
 
             _timeoutControl.StartRequestBody(minRate);
@@ -341,7 +341,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             // Initialize timestamp
             var now = DateTimeOffset.UtcNow;
-            _timeoutControl.Initialize(now);
+            _timeoutControl.Initialize(now.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();
@@ -379,7 +379,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
 
             // Initialize timestamp
-            _timeoutControl.Initialize(_systemClock.UtcNow);
+            _timeoutControl.Initialize(_systemClock.UtcNow.Ticks);
 
             // Should complete within 4 seconds, but the timeout is adjusted by adding Heartbeat.Interval
             _timeoutControl.BytesWrittenToBuffer(minRate, 400);
@@ -398,7 +398,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             // Initialize timestamp
             var startTime = _systemClock.UtcNow;
-            _timeoutControl.Initialize(startTime);
+            _timeoutControl.Initialize(startTime.Ticks);
 
             // Should complete within 1 second, but the timeout is adjusted by adding Heartbeat.Interval
             _timeoutControl.BytesWrittenToBuffer(minRate, 100);
@@ -422,7 +422,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
 
             // Initialize timestamp
-            _timeoutControl.Initialize(_systemClock.UtcNow);
+            _timeoutControl.Initialize(_systemClock.UtcNow.Ticks);
 
             // Should complete within 5 seconds, but the timeout is adjusted by adding Heartbeat.Interval
             _timeoutControl.BytesWrittenToBuffer(minRate, 500);
@@ -456,7 +456,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
             // Initialize timestamp
             var startTime = _systemClock.UtcNow;
-            _timeoutControl.Initialize(startTime);
+            _timeoutControl.Initialize(startTime.Ticks);
 
             // 5 consecutive 100 byte writes.
             for (var i = 0; i < numWrites - 1; i++)
@@ -487,7 +487,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
 
             // Initialize timestamp
-            _timeoutControl.Initialize(_systemClock.UtcNow);
+            _timeoutControl.Initialize(_systemClock.UtcNow.Ticks);
 
             // Should complete within 4 seconds, but the timeout is adjusted by adding Heartbeat.Interval
             _timeoutControl.BytesWrittenToBuffer(minRate, 400);
@@ -512,7 +512,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minRate = new MinDataRate(bytesPerSecond, gracePeriod);
 
             // Initialize timestamp
-            _timeoutControl.Initialize(_systemClock.UtcNow);
+            _timeoutControl.Initialize(_systemClock.UtcNow.Ticks);
 
             _timeoutControl.StartRequestBody(minRate);
             _timeoutControl.StartTimingRead();

test/Kestrel.InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs

@@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var mockSystemClock = _serviceContext.MockSystemClock;
             var limits = _serviceContext.ServerOptions.Limits;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             CreateConnection();
 
@@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var mockSystemClock = _serviceContext.MockSystemClock;
             var limits = _serviceContext.ServerOptions.Limits;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_noopApplication);
 
@@ -73,7 +73,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var mockSystemClock = _serviceContext.MockSystemClock;
             var limits = _serviceContext.ServerOptions.Limits;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_noopApplication);
 
@@ -128,7 +128,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var mockSystemClock = _serviceContext.MockSystemClock;
             var limits = _serviceContext.ServerOptions.Limits;;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_noopApplication);
 
@@ -165,7 +165,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var mockSystemClock = _serviceContext.MockSystemClock;
             var limits = _serviceContext.ServerOptions.Limits;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_noopApplication);
 
@@ -272,7 +272,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Disable response buffering so "socket" backpressure is observed immediately.
             limits.MaxResponseBufferSize = 0;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
             
@@ -328,7 +328,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Disable response buffering so "socket" backpressure is observed immediately.
             limits.MaxResponseBufferSize = 0;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -386,7 +386,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // This only affects the stream windows. The connection-level window is always initialized at 64KiB.
             _clientSettings.InitialWindowSize = 6;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -440,7 +440,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // This only affects the stream windows. The connection-level window is always initialized at 64KiB.
             _clientSettings.InitialWindowSize = (uint)_maxData.Length - 1;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -496,7 +496,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // This only affects the stream windows. The connection-level window is always initialized at 64KiB.
             _clientSettings.InitialWindowSize = (uint)_maxData.Length - 1;
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -561,7 +561,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Use non-default value to ensure the min request and response rates aren't mixed up.
             limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5));
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -610,7 +610,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Use non-default value to ensure the min request and response rates aren't mixed up.
             limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5));
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -663,7 +663,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Use non-default value to ensure the min request and response rates aren't mixed up.
             limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5));
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -732,7 +732,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Use non-default value to ensure the min request and response rates aren't mixed up.
             limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5));
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(_echoApplication);
 
@@ -807,7 +807,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             // Use non-default value to ensure the min request and response rates aren't mixed up.
             limits.MinRequestBodyDataRate = new MinDataRate(480, TimeSpan.FromSeconds(2.5));
 
-            _timeoutControl.Initialize(mockSystemClock.UtcNow);
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
 
             await InitializeConnectionAsync(async context =>
             {

test/shared/MockSystemClock.cs

@@ -24,6 +24,10 @@ namespace Microsoft.AspNetCore.Testing
             }
         }
 
+        public long UtcNowTicks => UtcNow.Ticks;
+
+        public DateTimeOffset UtcNowUnsynchronized => UtcNow;
+
         public int UtcNowCalled { get; private set; }
     }
 }