Minimize blocking threads to improve test reliability

test/Kestrel.Core.Tests/HttpConnectionTests.cs

@@ -418,10 +418,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         }
 
         [Fact]
-        public void WriteTimingAbortsConnectionWhenWriteDoesNotCompleteWithMinimumDataRate()
+        public async Task WriteTimingAbortsConnectionWhenWriteDoesNotCompleteWithMinimumDataRate()
         {
             var systemClock = new MockSystemClock();
-            var aborted = new ManualResetEventSlim();
+            var aborted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate =
                 new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
@@ -434,7 +434,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _httpConnection.Http1Connection.Reset();
             _httpConnection.Http1Connection.RequestAborted.Register(() =>
             {
-                aborted.Set();
+                aborted.SetResult(null);
             });
 
             // Initialize timestamp
@@ -448,15 +448,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _httpConnection.Tick(systemClock.UtcNow);
 
             Assert.True(_httpConnection.RequestTimedOut);
-            Assert.True(aborted.Wait(TimeSpan.FromSeconds(10)));
+            await aborted.Task.DefaultTimeout();
         }
 
         [Fact]
-        public void WriteTimingAbortsConnectionWhenSmallWriteDoesNotCompleteWithinGracePeriod()
+        public async Task WriteTimingAbortsConnectionWhenSmallWriteDoesNotCompleteWithinGracePeriod()
         {
             var systemClock = new MockSystemClock();
             var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5));
-            var aborted = new ManualResetEventSlim();
+            var aborted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = minResponseDataRate;
             _httpConnectionContext.ServiceContext.SystemClock = systemClock;
@@ -468,7 +468,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _httpConnection.Http1Connection.Reset();
             _httpConnection.Http1Connection.RequestAborted.Register(() =>
             {
-                aborted.Set();
+                aborted.SetResult(null);
             });
 
             // Initialize timestamp
@@ -490,14 +490,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _httpConnection.Tick(systemClock.UtcNow);
 
             Assert.True(_httpConnection.RequestTimedOut);
-            Assert.True(aborted.Wait(TimeSpan.FromSeconds(10)));
+            await aborted.Task.DefaultTimeout();
         }
 
         [Fact]
-        public void WriteTimingTimeoutPushedOnConcurrentWrite()
+        public async Task WriteTimingTimeoutPushedOnConcurrentWrite()
         {
             var systemClock = new MockSystemClock();
-            var aborted = new ManualResetEventSlim();
+            var aborted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate =
                 new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(2));
@@ -510,7 +510,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _httpConnection.Http1Connection.Reset();
             _httpConnection.Http1Connection.RequestAborted.Register(() =>
             {
-                aborted.Set();
+                aborted.SetResult(null);
             });
 
             // Initialize timestamp
@@ -537,7 +537,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _httpConnection.Tick(systemClock.UtcNow);
 
             Assert.True(_httpConnection.RequestTimedOut);
-            Assert.True(aborted.Wait(TimeSpan.FromSeconds(10)));
+            await aborted.Task.DefaultTimeout();
         }
 
         [Fact]
@@ -547,7 +547,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             var minResponseDataRate = new MinDataRate(bytesPerSecond: 100, gracePeriod: TimeSpan.FromSeconds(5));
             var numWrites = 5;
             var writeSize = 100;
-            var aborted = new TaskCompletionSource<object>();
+            var aborted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             _httpConnectionContext.ServiceContext.ServerOptions.Limits.MinResponseDataRate = minResponseDataRate;
             _httpConnectionContext.ServiceContext.SystemClock = systemClock;

test/Kestrel.Core.Tests/MessageBodyTests.cs

@@ -687,11 +687,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
         {
             using (var input = new TestInput())
             {
-                var logEvent = new ManualResetEventSlim();
+                var logEvent = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
                 var mockLogger = new Mock<IKestrelTrace>();
                 mockLogger
                     .Setup(logger => logger.RequestBodyDone("ConnectionId", "RequestId"))
-                    .Callback(() => logEvent.Set());
+                    .Callback(() => logEvent.SetResult(null));
                 input.Http1Connection.ServiceContext.Log = mockLogger.Object;
                 input.Http1Connection.ConnectionIdFeature = "ConnectionId";
                 input.Http1Connection.TraceIdentifier = "RequestId";
@@ -706,7 +706,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
 
                 input.Fin();
 
-                Assert.True(logEvent.Wait(TestConstants.DefaultTimeout));
+                await logEvent.Task.DefaultTimeout();
 
                 await body.StopAsync();
             }

test/Kestrel.FunctionalTests/ChunkedResponseTests.cs

@@ -350,7 +350,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         {
             var testContext = new TestServiceContext(LoggerFactory);
 
-            var flushWh = new ManualResetEventSlim();
+            var flushWh = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             using (var server = new TestServer(async httpContext =>
             {
@@ -358,7 +358,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello "), 0, 6);
 
                 // Don't complete response until client has received the first chunk.
-                flushWh.Wait();
+                await flushWh.Task.DefaultTimeout();
 
                 await response.Body.WriteAsync(Encoding.ASCII.GetBytes("World!"), 0, 6);
             }, testContext, listenOptions))
@@ -379,7 +379,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "Hello ",
                         "");
 
-                    flushWh.Set();
+                    flushWh.SetResult(null);
 
                     await connection.ReceiveEnd(
                         "6",

test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs

@@ -30,15 +30,39 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 DateHeaderValueManager = new DateHeaderValueManager(systemClock)
             };
 
-            var appRunningEvent = new ManualResetEventSlim();
+            var appRunningEvent = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             using (var server = new TestServer(context =>
             {
                 context.Features.Get<IHttpMinRequestBodyDataRateFeature>().MinDataRate =
                     new MinDataRate(bytesPerSecond: 1, gracePeriod: gracePeriod);
 
-                appRunningEvent.Set();
+                // The server must call Request.Body.ReadAsync() *before* the test sets systemClock.UtcNow (which is triggered by the
-                return context.Request.Body.ReadAsync(new byte[1], 0, 1);
+                // server calling appRunningEvent.SetResult(null)).  If systemClock.UtcNow is set first, it's possible for the test to fail
+                // due to the following race condition:
+                //
+                // 1. [test]    systemClock.UtcNow += gracePeriod + TimeSpan.FromSeconds(1);
+                // 2. [server]  Heartbeat._timer is triggered, which calls HttpConnection.Tick()
+                // 3. [server]  HttpConnection.Tick() calls HttpConnection.CheckForReadDataRateTimeout()
+                // 4. [server]  HttpConnection.CheckForReadDataRateTimeout() is a no-op, since _readTimingEnabled is false,
+                //              since Request.Body.ReadAsync() has not been called yet
+                // 5. [server]  HttpConnection.Tick() sets _lastTimestamp = timestamp
+                // 6. [server]  Request.Body.ReadAsync() is called
+                // 6. [test]    systemClock.UtcNow is never updated again, so server timestamp is never updated,
+                //              so HttpConnection.CheckForReadDataRateTimeout() is always a no-op until test fails
+                //
+                // This is a pretty tight race, since the once-per-second Heartbeat._timer needs to fire between the test updating
+                // systemClock.UtcNow and the server calling Request.Body.ReadAsync().  But it happened often enough to cause
+                // test flakiness in our CI (https://github.com/aspnet/KestrelHttpServer/issues/2539).
+                //
+                // For verification, I was able to induce the race by adding a sleep in the RequestDelegate:
+                //     appRunningEvent.SetResult(null);
+                //     Thread.Sleep(5000);
+                //     return context.Request.Body.ReadAsync(new byte[1], 0, 1);
+
+                var readTask = context.Request.Body.ReadAsync(new byte[1], 0, 1);
+                appRunningEvent.SetResult(null);
+                return readTask;
             }, serviceContext))
             {
                 using (var connection = server.CreateConnection())
@@ -50,7 +74,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "",
                         "");
 
-                    Assert.True(appRunningEvent.Wait(TestConstants.DefaultTimeout));
+                    await appRunningEvent.Task.DefaultTimeout();
                     systemClock.UtcNow += gracePeriod + TimeSpan.FromSeconds(1);
 
                     await connection.Receive(
@@ -77,13 +101,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 DateHeaderValueManager = new DateHeaderValueManager(systemClock),
             };
 
-            var appRunningEvent = new ManualResetEventSlim();
+            var appRunningEvent = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             using (var server = new TestServer(context =>
             {
                 context.Features.Get<IHttpMinRequestBodyDataRateFeature>().MinDataRate = null;
 
-                appRunningEvent.Set();
+                appRunningEvent.SetResult(null);
                 return Task.CompletedTask;
             }, serviceContext))
             {
@@ -96,7 +120,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "",
                         "");
 
-                    Assert.True(appRunningEvent.Wait(TestConstants.DefaultTimeout));
+                    await appRunningEvent.Task.DefaultTimeout();
 
                     await connection.Receive(
                         "HTTP/1.1 200 OK",
@@ -115,7 +139,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             Assert.Contains(TestSink.Writes, w => w.EventId.Id == 33 && w.LogLevel == LogLevel.Information);
         }
 
-        [Fact(Skip="https://github.com/aspnet/KestrelHttpServer/issues/2464")]
+        [Fact]
         public async Task ConnectionClosedEvenIfAppSwallowsException()
         {
             var gracePeriod = TimeSpan.FromSeconds(5);
@@ -126,23 +150,30 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 DateHeaderValueManager = new DateHeaderValueManager(systemClock)
             };
 
-            var appRunningEvent = new ManualResetEventSlim();
+            var appRunningTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-            var exceptionSwallowedEvent = new ManualResetEventSlim();
+            var exceptionSwallowedTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             using (var server = new TestServer(async context =>
             {
                 context.Features.Get<IHttpMinRequestBodyDataRateFeature>().MinDataRate =
                     new MinDataRate(bytesPerSecond: 1, gracePeriod: gracePeriod);
 
-                appRunningEvent.Set();
+                // See comment in RequestTimesOutWhenRequestBodyNotReceivedAtSpecifiedMinimumRate for
+                // why we call ReadAsync before setting the appRunningEvent.
+                var readTask = context.Request.Body.ReadAsync(new byte[1], 0, 1);
+                appRunningTcs.SetResult(null);
 
                 try
                 {
-                    await context.Request.Body.ReadAsync(new byte[1], 0, 1);
+                    await readTask;
                 }
                 catch (BadHttpRequestException ex) when (ex.StatusCode == 408)
                 {
-                    exceptionSwallowedEvent.Set();
+                    exceptionSwallowedTcs.SetResult(null);
+                }
+                catch (Exception ex)
+                {
+                    exceptionSwallowedTcs.SetException(ex);
                 }
 
                 var response = "hello, world";
@@ -159,9 +190,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "",
                         "");
 
-                    Assert.True(appRunningEvent.Wait(TestConstants.DefaultTimeout), "AppRunningEvent timed out.");
+                    await appRunningTcs.Task.DefaultTimeout();
                     systemClock.UtcNow += gracePeriod + TimeSpan.FromSeconds(1);
-                    Assert.True(exceptionSwallowedEvent.Wait(TestConstants.DefaultTimeout), "ExceptionSwallowedEvent timed out.");
+                    await exceptionSwallowedTcs.Task.DefaultTimeout();
 
                     await connection.Receive(
                         "HTTP/1.1 200 OK",

test/Kestrel.FunctionalTests/RequestTests.cs

@@ -1696,8 +1696,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         [Fact]
         public async Task DoesNotEnforceRequestBodyMinimumDataRateOnUpgradedRequest()
         {
-            var appEvent = new ManualResetEventSlim();
+            var appEvent = new TaskCompletionSource<object>();
-            var delayEvent = new ManualResetEventSlim();
+            var delayEvent = new TaskCompletionSource<object>();
             var serviceContext = new TestServiceContext(LoggerFactory)
             {
                 SystemClock = new SystemClock()
@@ -1710,12 +1710,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
 
                 using (var stream = await context.Features.Get<IHttpUpgradeFeature>().UpgradeAsync())
                 {
-                    appEvent.Set();
+                    appEvent.SetResult(null);
 
                     // Read once to go through one set of TryPauseTimingReads()/TryResumeTimingReads() calls
                     await stream.ReadAsync(new byte[1], 0, 1);
 
-                    delayEvent.Wait();
+                    await delayEvent.Task.DefaultTimeout();
 
                     // Read again to check that the connection is still alive
                     await stream.ReadAsync(new byte[1], 0, 1);
@@ -1735,11 +1735,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "",
                         "a");
 
-                    Assert.True(appEvent.Wait(TestConstants.DefaultTimeout));
+                    await appEvent.Task.DefaultTimeout();
 
                     await Task.Delay(TimeSpan.FromSeconds(5));
 
-                    delayEvent.Set();
+                    delayEvent.SetResult(null);
 
                     await connection.Send("b");
 

test/Kestrel.FunctionalTests/ResponseTests.cs

@@ -1116,12 +1116,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             var flushed = new SemaphoreSlim(0, 1);
             var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } };
 
-            using (var server = new TestServer(httpContext =>
+            using (var server = new TestServer(async httpContext =>
             {
                 httpContext.Response.ContentLength = 12;
                 httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello, world"), 0, 12);
-                flushed.Wait();
+                await flushed.WaitAsync();
-                return Task.CompletedTask;
             }, serviceContext))
             {
                 using (var connection = server.CreateConnection())
@@ -1152,7 +1151,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             {
                 httpContext.Response.ContentLength = 12;
                 await httpContext.Response.WriteAsync("");
-                flushed.Wait();
+                await flushed.WaitAsync();
                 await httpContext.Response.WriteAsync("hello, world");
             }, new TestServiceContext(LoggerFactory)))
             {
@@ -1180,23 +1179,23 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         [Fact]
         public async Task WriteAfterConnectionCloseNoops()
         {
-            var connectionClosed = new ManualResetEventSlim();
+            var connectionClosed = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-            var requestStarted = new ManualResetEventSlim();
+            var requestStarted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-            var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+            var appCompleted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             using (var server = new TestServer(async httpContext =>
             {
                 try
                 {
-                    requestStarted.Set();
+                    requestStarted.SetResult(null);
-                    connectionClosed.Wait();
+                    await connectionClosed.Task.DefaultTimeout();
                     httpContext.Response.ContentLength = 12;
                     await httpContext.Response.WriteAsync("hello, world");
-                    tcs.TrySetResult(null);
+                    appCompleted.TrySetResult(null);
                 }
                 catch (Exception ex)
                 {
-                    tcs.TrySetException(ex);
+                    appCompleted.TrySetException(ex);
                 }
             }, new TestServiceContext(LoggerFactory)))
             {
@@ -1208,14 +1207,14 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "",
                         "");
 
-                    requestStarted.Wait();
+                    await requestStarted.Task.DefaultTimeout();
                     connection.Shutdown(SocketShutdown.Send);
                     await connection.WaitForConnectionClose().DefaultTimeout();
                 }
 
-                connectionClosed.Set();
+                connectionClosed.SetResult(null);
 
-                await tcs.Task.DefaultTimeout();
+                await appCompleted.Task.DefaultTimeout();
             }
         }
 
@@ -2280,19 +2279,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             var largeString = new string('a', maxBytesPreCompleted + 1);
 
             var writeTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-            var requestAbortedWh = new ManualResetEventSlim();
+            var requestAbortedWh = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
-            var requestStartWh = new ManualResetEventSlim();
+            var requestStartWh = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
 
             using (var server = new TestServer(async httpContext =>
             {
-                requestStartWh.Set();
+                requestStartWh.SetResult(null);
 
                 var response = httpContext.Response;
                 var request = httpContext.Request;
                 var lifetime = httpContext.Features.Get<IHttpRequestLifetimeFeature>();
 
-                lifetime.RequestAborted.Register(() => requestAbortedWh.Set());
+                lifetime.RequestAborted.Register(() => requestAbortedWh.SetResult(null));
-                Assert.True(requestAbortedWh.Wait(TestConstants.DefaultTimeout));
+                await requestAbortedWh.Task.DefaultTimeout();
 
                 try
                 {
@@ -2316,15 +2315,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                         "",
                         "");
 
-                    Assert.True(requestStartWh.Wait(TestConstants.DefaultTimeout));
+                    await requestStartWh.Task.DefaultTimeout();
                 }
 
                 // Write failed - can throw TaskCanceledException or OperationCanceledException,
-                // dependending on how far the canceled write goes.
+                // depending on how far the canceled write goes.
                 await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await writeTcs.Task).DefaultTimeout();
 
                 // RequestAborted tripped
-                Assert.True(requestAbortedWh.Wait(TestConstants.DefaultTimeout));
+                await requestAbortedWh.Task.DefaultTimeout();
             }
         }