diff --git a/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs b/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs index 0ae0e9f0c..61eb7a471 100644 --- a/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs +++ b/test/Kestrel.FunctionalTests/RequestBodyTimeoutTests.cs @@ -7,7 +7,6 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; @@ -37,8 +36,32 @@ public async Task RequestTimesOutWhenRequestBodyNotReceivedAtSpecifiedMinimumRat context.Features.Get().MinDataRate = new MinDataRate(bytesPerSecond: 1, gracePeriod: gracePeriod); + // The server must call Request.Body.ReadAsync() *before* the test sets systemClock.UtcNow (which is triggered by the + // server calling appRunningEvent.Set()). 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.Set(); + // 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.Set(); - return context.Request.Body.ReadAsync(new byte[1], 0, 1); + return readTask; }, serviceContext)) { using (var connection = server.CreateConnection())