From f3b6430aabe561ca4a5876d3fe80c19ff3c3c910 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 17 Sep 2018 16:43:30 -0700 Subject: [PATCH] Initialize Heartbeat in KestrelServer.StartAsync (#2939) Fixes #2850 again --- .../Http1WritingBenchmark.cs | 1 + .../ResponseHeaderCollectionBenchmark.cs | 4 +- .../ResponseHeadersWritingBenchmark.cs | 1 + .../Internal/Http/DateHeaderValueManager.cs | 14 ------- .../Internal/Infrastructure/Heartbeat.cs | 1 + .../Infrastructure/HeartbeatManager.cs | 3 +- src/Kestrel.Core/KestrelServer.cs | 5 +-- .../DateHeaderValueManagerTests.cs | 16 ++++++-- test/Kestrel.Core.Tests/KestrelServerTests.cs | 39 +++++++++++++++++++ .../HttpsTests.cs | 2 +- .../KeepAliveTimeoutTests.cs | 12 +++--- .../RequestBodyTimeoutTests.cs | 9 +++-- .../RequestHeadersTimeoutTests.cs | 8 ++-- .../RequestTests.cs | 2 +- test/shared/TestServiceContext.cs | 9 +++-- 15 files changed, 83 insertions(+), 43 deletions(-) diff --git a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs index acc57a23d6..1125eb47b2 100644 --- a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs @@ -118,6 +118,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance http1Connection.Reset(); http1Connection.InitializeStreams(MessageBody.ZeroContentLengthKeepAlive); + serviceContext.DateHeaderValueManager.OnHeartbeat(DateTimeOffset.UtcNow); return http1Connection; } diff --git a/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs b/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs index abd19bc359..84ee6f477a 100644 --- a/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs +++ b/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Buffers; using System.IO.Pipelines; using System.Runtime.CompilerServices; @@ -177,7 +178,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance var serviceContext = new ServiceContext { - DateHeaderValueManager = new DateHeaderValueManager(), + DateHeaderValueManager = _dateHeaderValueManager, ServerOptions = new KestrelServerOptions(), Log = new MockTrace(), HttpParser = new HttpParser() @@ -192,6 +193,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance }); http1Connection.Reset(); + serviceContext.DateHeaderValueManager.OnHeartbeat(DateTimeOffset.UtcNow); _responseHeadersDirect = (HttpResponseHeaders)http1Connection.ResponseHeaders; var context = new DefaultHttpContext(http1Connection); diff --git a/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs b/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs index 6eea85d72d..1bb5f54258 100644 --- a/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs @@ -137,6 +137,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance }); http1Connection.Reset(); + serviceContext.DateHeaderValueManager.OnHeartbeat(DateTimeOffset.UtcNow); _http1Connection = http1Connection; } diff --git a/src/Kestrel.Core/Internal/Http/DateHeaderValueManager.cs b/src/Kestrel.Core/Internal/Http/DateHeaderValueManager.cs index ee82fd1bc1..58dcf88841 100644 --- a/src/Kestrel.Core/Internal/Http/DateHeaderValueManager.cs +++ b/src/Kestrel.Core/Internal/Http/DateHeaderValueManager.cs @@ -18,20 +18,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http private DateHeaderValues _dateValues; - /// - /// Initializes a new instance of the class. - /// - public DateHeaderValueManager() - : this(DateTimeOffset.UtcNow) - { - } - - // Internal for testing - internal DateHeaderValueManager(DateTimeOffset initialUtcNow) - { - SetDateValues(initialUtcNow); - } - /// /// Returns a value representing the current server date/time for use in the HTTP "Date" response header /// in accordance with http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.18 diff --git a/src/Kestrel.Core/Internal/Infrastructure/Heartbeat.cs b/src/Kestrel.Core/Internal/Infrastructure/Heartbeat.cs index 5937002304..fdf5e75010 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/Heartbeat.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/Heartbeat.cs @@ -30,6 +30,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure public void Start() { + OnHeartbeat(); _timer = new Timer(OnHeartbeat, state: this, dueTime: _interval, period: _interval); } diff --git a/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs b/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs index 98d3585921..bc54e73385 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/HeartbeatManager.cs @@ -11,10 +11,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure private readonly Action _walkCallback; private DateTimeOffset _now; - public HeartbeatManager(ConnectionManager connectionManager, DateTimeOffset initialUtcNow) + public HeartbeatManager(ConnectionManager connectionManager) { _connectionManager = connectionManager; - _now = initialUtcNow; _walkCallback = WalkCallback; } diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index e90188a6eb..51b444ada2 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -71,9 +71,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core trace, serverOptions.Limits.MaxConcurrentUpgradedConnections); - var now = DateTimeOffset.UtcNow; - var heartbeatManager = new HeartbeatManager(connectionManager, now); - var dateHeaderValueManager = new DateHeaderValueManager(now); + var heartbeatManager = new HeartbeatManager(connectionManager); + var dateHeaderValueManager = new DateHeaderValueManager(); var heartbeat = new Heartbeat( new IHeartbeatHandler[] { dateHeaderValueManager, heartbeatManager }, new SystemClock(), diff --git a/test/Kestrel.Core.Tests/DateHeaderValueManagerTests.cs b/test/Kestrel.Core.Tests/DateHeaderValueManagerTests.cs index adf2971c2c..719d82752c 100644 --- a/test/Kestrel.Core.Tests/DateHeaderValueManagerTests.cs +++ b/test/Kestrel.Core.Tests/DateHeaderValueManagerTests.cs @@ -25,7 +25,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests { var now = DateTimeOffset.UtcNow; - var dateHeaderValueManager = new DateHeaderValueManager(now); + var dateHeaderValueManager = new DateHeaderValueManager(); + dateHeaderValueManager.OnHeartbeat(now); + Assert.Equal(now.ToString(Rfc1123DateFormat), dateHeaderValueManager.GetDateHeaderValues().String); } @@ -39,7 +41,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests UtcNow = now }; - var dateHeaderValueManager = new DateHeaderValueManager(now); + var dateHeaderValueManager = new DateHeaderValueManager(); + dateHeaderValueManager.OnHeartbeat(now); + var testKestrelTrace = new TestKestrelTrace(); using (var heartbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, DebuggerWrapper.Singleton, testKestrelTrace)) @@ -62,7 +66,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests UtcNow = now }; - var dateHeaderValueManager = new DateHeaderValueManager(now); + var dateHeaderValueManager = new DateHeaderValueManager(); + dateHeaderValueManager.OnHeartbeat(now); + var testKestrelTrace = new TestKestrelTrace(); var mockHeartbeatHandler = new Mock(); @@ -93,7 +99,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests UtcNow = now }; - var dateHeaderValueManager = new DateHeaderValueManager(now); + var dateHeaderValueManager = new DateHeaderValueManager(); + dateHeaderValueManager.OnHeartbeat(now); + var testKestrelTrace = new TestKestrelTrace(); using (var heatbeat = new Heartbeat(new IHeartbeatHandler[] { dateHeaderValueManager }, systemClock, DebuggerWrapper.Singleton, testKestrelTrace)) diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 5ac0fbd5cd..0a8ad65938 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -9,11 +9,14 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Net.Http.Headers; using Moq; using Xunit; @@ -384,6 +387,42 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests mockTransport.Verify(transport => transport.UnbindAsync(), Times.Once); } + [Fact] + public void StartingServerInitializesHeartbeat() + { + var testContext = new TestServiceContext() + { + ServerOptions = + { + ListenOptions = + { + new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + } + }, + DateHeaderValueManager = new DateHeaderValueManager() + }; + + testContext.Heartbeat = new Heartbeat( + new IHeartbeatHandler[] { testContext.DateHeaderValueManager }, + testContext.MockSystemClock, + DebuggerWrapper.Singleton, + testContext.Log); + + using (var server = new KestrelServer(new MockTransportFactory(), testContext)) + { + Assert.Null(testContext.DateHeaderValueManager.GetDateHeaderValues()); + + // Ensure KestrelServer is started at a different time than when it was constructed, since we're + // verifying the heartbeat is initialized during KestrelServer.StartAsync(). + testContext.MockSystemClock.UtcNow += TimeSpan.FromDays(1); + + StartDummyApplication(server); + + Assert.Equal(HeaderUtilities.FormatDate(testContext.MockSystemClock.UtcNow), + testContext.DateHeaderValueManager.GetDateHeaderValues().String); + } + } + private static KestrelServer CreateServer(KestrelServerOptions options, ILogger testLogger) { return new KestrelServer(Options.Create(options), new MockTransportFactory(), new LoggerFactory(new[] { new KestrelTestLoggerProvider(testLogger) })); diff --git a/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs b/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs index 51980f6977..da2df11053 100644 --- a/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/HttpsTests.cs @@ -312,7 +312,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests LoggerFactory.AddProvider(loggerProvider); var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var handshakeStartedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); TimeSpan handshakeTimeout = default; diff --git a/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs index f462b0716c..716ff9583a 100644 --- a/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/KeepAliveTimeoutTests.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionClosedWhenKeepAliveTimeoutExpires() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionKeptAliveBetweenRequests() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionNotTimedOutWhileRequestBeingSent() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -113,7 +113,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task ConnectionNotTimedOutWhileAppIsRunning() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var cts = new CancellationTokenSource(); using (var server = CreateServer(testContext, longRunningCt: cts.Token)) @@ -150,7 +150,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task ConnectionTimesOutWhenOpenedButNoRequestSent() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -167,7 +167,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests private async Task KeepAliveTimeoutDoesNotApplyToUpgradedConnections() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); var cts = new CancellationTokenSource(); using (var server = CreateServer(testContext, upgradeCt: cts.Token)) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs index e40f387e97..75d55da8fb 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestBodyTimeoutTests.cs @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { var gracePeriod = TimeSpan.FromSeconds(5); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager, serviceContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); var appRunningEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -95,7 +95,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests serviceContext.InitializeHeartbeat(); // Ensure there's still a constant date header value. - serviceContext.DateHeaderValueManager = new DateHeaderValueManager(); + var clock = new MockSystemClock(); + var date = new DateHeaderValueManager(); + date.OnHeartbeat(clock.UtcNow); + serviceContext.DateHeaderValueManager = date; var appRunningEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); @@ -137,7 +140,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { var gracePeriod = TimeSpan.FromSeconds(5); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager, serviceContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); var appRunningTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var exceptionSwallowedTcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs index 5bd803a6dc..e5fd714179 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestHeadersTimeoutTests.cs @@ -26,7 +26,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionAbortedWhenRequestHeadersNotReceivedInTime(string headers) { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task RequestHeadersTimeoutCanceledAfterHeadersReceived() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task ConnectionAbortedWhenRequestLineNotReceivedInTime(string requestLine) { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests public async Task TimeoutNotResetOnEachRequestLineCharacterReceived() { var testContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager, testContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(testContext.ConnectionManager); using (var server = CreateServer(testContext)) using (var connection = server.CreateConnection()) diff --git a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs index 0e64566c6e..1b3473a60f 100644 --- a/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/RequestTests.cs @@ -945,7 +945,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests var appEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var delayEvent = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var serviceContext = new TestServiceContext(LoggerFactory); - var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager, serviceContext.SystemClock.UtcNow); + var heartbeatManager = new HeartbeatManager(serviceContext.ConnectionManager); using (var server = new TestServer(async context => { diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index 3e833a9e5e..5944eece8b 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -41,9 +41,8 @@ namespace Microsoft.AspNetCore.Testing public void InitializeHeartbeat() { - var now = DateTimeOffset.UtcNow; - var heartbeatManager = new HeartbeatManager(ConnectionManager, now); - DateHeaderValueManager = new DateHeaderValueManager(now); + var heartbeatManager = new HeartbeatManager(ConnectionManager); + DateHeaderValueManager = new DateHeaderValueManager(); Heartbeat = new Heartbeat( new IHeartbeatHandler[] { DateHeaderValueManager, heartbeatManager }, new SystemClock(), @@ -61,13 +60,15 @@ namespace Microsoft.AspNetCore.Testing Scheduler = PipeScheduler.ThreadPool; MockSystemClock = new MockSystemClock(); SystemClock = MockSystemClock; - DateHeaderValueManager = new DateHeaderValueManager(MockSystemClock.UtcNow); + DateHeaderValueManager = new DateHeaderValueManager(); ConnectionManager = new ConnectionManager(Log, ResourceCounter.Unlimited); HttpParser = new HttpParser(Log.IsEnabled(LogLevel.Information)); ServerOptions = new KestrelServerOptions { AddServerHeader = false }; + + DateHeaderValueManager.OnHeartbeat(SystemClock.UtcNow); } public ILoggerFactory LoggerFactory { get; set; }