Improve reliability of RequestTimesOutWhenRequestBodyNotReceivedAtSpecifiedMinimumRate (#2589)
- Fix race condition in test code - Addresses https://github.com/aspnet/KestrelHttpServer/issues/2539
This commit is contained in:
parent
1951ddf6ea
commit
99a661edd6
|
|
@ -7,7 +7,6 @@ using System.Threading.Tasks;
|
|||
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 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
|
|||
context.Features.Get<IHttpMinRequestBodyDataRateFeature>().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())
|
||||
|
|
|
|||
Loading…
Reference in New Issue