diff --git a/test/Kestrel.Core.Tests/HttpConnectionTests.cs b/test/Kestrel.Core.Tests/HttpConnectionTests.cs
index 091b3cadeaa072bb73d9fffea32f3c2fd0bc475a..c8dbf28ba0c7ed3decb6b9a9ece9486c7d88020b 100644
--- a/test/Kestrel.Core.Tests/HttpConnectionTests.cs
+++ b/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;
diff --git a/test/Kestrel.Core.Tests/MessageBodyTests.cs b/test/Kestrel.Core.Tests/MessageBodyTests.cs
index 703808a400d58f8e94421f24168115a46b64dd34..5f006b12e3cc96ce0183c2d6199b880de5fa9ec4 100644
--- a/test/Kestrel.Core.Tests/MessageBodyTests.cs
+++ b/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();
}
diff --git a/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs b/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs
index c9a7e70747a1a5faedb546cdd840d07578d4dbda..6cb1a9e439284d5b31117595e4e3f347721287bc 100644
--- a/test/Kestrel.FunctionalTests/ChunkedResponseTests.cs
+++ b/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",
diff --git a/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs b/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs
index 0ae0e9f0ce9d6ff3d347124531c9b859f88dd410..9d292e53fce717e2207fd53a299b173b214b9ee2 100644
--- a/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs
+++ b/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();
- return context.Request.Body.ReadAsync(new byte[1], 0, 1);
+ // The server must call Request.Body.ReadAsync() *before* the test sets systemClock.UtcNow (which is triggered by the
+ // 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 exceptionSwallowedEvent = new ManualResetEventSlim();
+ var appRunningTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+ 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",
diff --git a/test/Kestrel.FunctionalTests/RequestTests.cs b/test/Kestrel.FunctionalTests/RequestTests.cs
index eac3aaa369dff23a0b0220e3546cfbd9574bb1f3..32dfac74fcf5000aa0a43a58e93819ad7bfc43b5 100644
--- a/test/Kestrel.FunctionalTests/RequestTests.cs
+++ b/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 delayEvent = new ManualResetEventSlim();
+ var appEvent = new TaskCompletionSource<object>();
+ 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");
diff --git a/test/Kestrel.FunctionalTests/ResponseTests.cs b/test/Kestrel.FunctionalTests/ResponseTests.cs
index 9660dca2f3ed3f59af94ce9c6c43235549d2ce2a..2f22bc1f675ace222aa2bd592d3cb1a1d08f52fa 100644
--- a/test/Kestrel.FunctionalTests/ResponseTests.cs
+++ 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();
- return Task.CompletedTask;
+ await flushed.WaitAsync();
}, 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 requestStarted = new ManualResetEventSlim();
- var tcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+ var connectionClosed = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+ var requestStarted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+ var appCompleted = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
using (var server = new TestServer(async httpContext =>
{
try
{
- requestStarted.Set();
- connectionClosed.Wait();
+ requestStarted.SetResult(null);
+ 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 requestStartWh = new ManualResetEventSlim();
+ var requestAbortedWh = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
+ 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());
- Assert.True(requestAbortedWh.Wait(TestConstants.DefaultTimeout));
+ lifetime.RequestAborted.Register(() => requestAbortedWh.SetResult(null));
+ 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();
}
}