Fix flaky heartbeat test (#1794)

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/Heartbeat.cs

@@ -12,29 +12,21 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
         public static readonly TimeSpan Interval = TimeSpan.FromSeconds(1);
 
         private readonly IHeartbeatHandler[] _callbacks;
-        private readonly TimeSpan _interval;
         private readonly ISystemClock _systemClock;
         private readonly IKestrelTrace _trace;
         private Timer _timer;
         private int _executingOnHeartbeat;
 
         public Heartbeat(IHeartbeatHandler[] callbacks, ISystemClock systemClock, IKestrelTrace trace)
-            : this(callbacks, systemClock, trace, Interval)
-        {
-        }
-
-        // For testing
-        internal Heartbeat(IHeartbeatHandler[] callbacks, ISystemClock systemClock, IKestrelTrace trace, TimeSpan interval)
         {
             _callbacks = callbacks;
-            _interval = interval;
             _systemClock = systemClock;
             _trace = trace;
         }
 
         public void Start()
         {
-            _timer = new Timer(OnHeartbeat, state: this, dueTime: _interval, period: _interval);
+            _timer = new Timer(OnHeartbeat, state: this, dueTime: Interval, period: Interval);
         }
 
         private static void OnHeartbeat(object state)
@@ -67,7 +59,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
             }
             else
             {
-                _trace.TimerSlow(_interval, now);
+                _trace.HeartbeatSlow(Interval, now);
             }
         }
 

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/IKestrelTrace.cs

@@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure
 
         void NotAllConnectionsAborted();
 
-        void TimerSlow(TimeSpan interval, DateTimeOffset now);
+        void HeartbeatSlow(TimeSpan interval, DateTimeOffset now);
 
         void ApplicationNeverCompleted(string connectionId);
     }

src/Microsoft.AspNetCore.Server.Kestrel.Core/Internal/Infrastructure/KestrelTrace.cs

@@ -48,7 +48,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
         private static readonly Action<ILogger, Exception> _notAllConnectionsAborted =
             LoggerMessage.Define(LogLevel.Debug, 21, "Some connections failed to abort during server shutdown.");
 
-        private static readonly Action<ILogger, TimeSpan, DateTimeOffset, Exception> _timerSlow =
+        private static readonly Action<ILogger, TimeSpan, DateTimeOffset, Exception> _heartbeatSlow =
             LoggerMessage.Define<TimeSpan, DateTimeOffset>(LogLevel.Warning, 22, @"Heartbeat took longer than ""{interval}"" at ""{now}"".");
 
         private static readonly Action<ILogger, string, Exception> _applicationNeverCompleted =
@@ -121,9 +121,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
             _notAllConnectionsAborted(_logger, null);
         }
 
-        public virtual void TimerSlow(TimeSpan interval, DateTimeOffset now)
+        public virtual void HeartbeatSlow(TimeSpan interval, DateTimeOffset now)
         {
-            _timerSlow(_logger, interval, now, null);
+            _heartbeatSlow(_logger, interval, now, null);
         }
 
         public virtual void ApplicationNeverCompleted(string connectionId)

test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/DateHeaderValueManagerTests.cs

@@ -43,11 +43,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 UtcNow = now
             };
 
-            var timerInterval = TimeSpan.FromSeconds(10);
             var dateHeaderValueManager = new DateHeaderValueManager(systemClock);
             var testKestrelTrace = new TestKestrelTrace();
 
-            using (var heartbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace, timerInterval))
+            using (var heartbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace))
             {
                 Assert.Equal(now.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String);
                 systemClock.UtcNow = future;
@@ -67,13 +66,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 UtcNow = now
             };
 
-            var timerInterval = TimeSpan.FromMilliseconds(100);
             var dateHeaderValueManager = new DateHeaderValueManager(systemClock);
             var testKestrelTrace = new TestKestrelTrace();
 
             var mockHeartbeatHandler = new Mock<IHeartbeatHandler>();
 
-            using (var heartbeat = new Heartbeat(new[] { dateHeaderValueManager, mockHeartbeatHandler.Object }, systemClock, testKestrelTrace, timerInterval))
+            using (var heartbeat = new Heartbeat(new[] { dateHeaderValueManager, mockHeartbeatHandler.Object }, systemClock, testKestrelTrace))
             {
                 heartbeat.OnHeartbeat();
 
@@ -99,11 +97,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 UtcNow = now
             };
 
-            var timerInterval = TimeSpan.FromSeconds(10);
             var dateHeaderValueManager = new DateHeaderValueManager(systemClock);
             var testKestrelTrace = new TestKestrelTrace();
 
-            using (var heatbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace, timerInterval))
+            using (var heatbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, testKestrelTrace))
             {
                 heatbeat.OnHeartbeat();
                 Assert.Equal(now.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String);

test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests/HeartbeatTests.cs

@@ -19,39 +19,38 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         public void BlockedHeartbeatDoesntCauseOverlapsAndIsLoggedAsError()
         {
             var systemClock = new MockSystemClock();
-            var heartbeatInterval = TimeSpan.FromMilliseconds(10);
             var heartbeatHandler = new Mock<IHeartbeatHandler>();
             var kestrelTrace = new Mock<IKestrelTrace>();
             var handlerMre = new ManualResetEventSlim();
             var traceMre = new ManualResetEventSlim();
 
             heartbeatHandler.Setup(h => h.OnHeartbeat(systemClock.UtcNow)).Callback(() => handlerMre.Wait());
-            kestrelTrace.Setup(t => t.TimerSlow(heartbeatInterval, systemClock.UtcNow)).Callback(() => traceMre.Set());
+            kestrelTrace.Setup(t => t.HeartbeatSlow(Heartbeat.Interval, systemClock.UtcNow)).Callback(() => traceMre.Set());
 
-            using (var heartbeat = new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace.Object, heartbeatInterval))
+            using (var heartbeat = new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace.Object))
             {
-                heartbeat.Start();
+                Task.Run(() => heartbeat.OnHeartbeat());
+                Task.Run(() => heartbeat.OnHeartbeat());
                 Assert.True(traceMre.Wait(TimeSpan.FromSeconds(10)));
             }
 
             handlerMre.Set();
 
             heartbeatHandler.Verify(h => h.OnHeartbeat(systemClock.UtcNow), Times.Once());
-            kestrelTrace.Verify(t => t.TimerSlow(heartbeatInterval, systemClock.UtcNow), Times.AtLeastOnce());
+            kestrelTrace.Verify(t => t.HeartbeatSlow(Heartbeat.Interval, systemClock.UtcNow), Times.Once());
         }
 
         [Fact]
         public void ExceptionFromHeartbeatHandlerIsLoggedAsError()
         {
             var systemClock = new MockSystemClock();
-            var heartbeatInterval = TimeSpan.FromMilliseconds(10);
             var heartbeatHandler = new Mock<IHeartbeatHandler>();
             var kestrelTrace = new TestKestrelTrace();
             var ex = new Exception();
 
             heartbeatHandler.Setup(h => h.OnHeartbeat(systemClock.UtcNow)).Throws(ex);
 
-            using (var heartbeat = new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace, heartbeatInterval))
+            using (var heartbeat = new Heartbeat(new[] { heartbeatHandler.Object }, systemClock, kestrelTrace))
             {
                 heartbeat.OnHeartbeat();
             }

test/Microsoft.AspNetCore.Server.Kestrel.Performance/Mocks/MockTrace.cs

@@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance
         public void NotAllConnectionsAborted() { }
         public void NotAllConnectionsClosedGracefully() { }
         public void RequestProcessingError(string connectionId, Exception ex) { }
-        public void TimerSlow(TimeSpan interval, DateTimeOffset now) { }
+        public void HeartbeatSlow(TimeSpan interval, DateTimeOffset now) { }
         public void ApplicationNeverCompleted(string connectionId) { }
     }
 }