Leonardo
Leonardo

Reputation: 11401

ASP.NET Core 5 - Timeout middleware outputting phantom data?

I created a timeout middleware that works basically like this:

public async Task InvokeAsync(HttpContext httpContext)
{
    var stopwatch = Stopwatch.StartNew();    

    using (var timeoutTS = CancellationTokenSource.CreateLinkedTokenSource(httpContext.RequestAborted))
    {
        var delayTask = Task.Delay(config.Timeout);
                
        var res = await Task.WhenAny(delayTask, _next(httpContext));
        Trace.WriteLine("Time taken = " + stopwatch.ElapsedMilliseconds);

        if (res == delayTask)
        {
            timeoutTS.Cancel();
            httpContext.Response.StatusCode = 408;
        }
    }
}

In order to test it, I created a controller action:

[HttpGet]
public async Task<string> Get(string timeout)
{
    var result = DateTime.Now.ToString("mm:ss.fff");

    if (timeout != null)
    {
        await Task.Delay(2000);
    }

    var rng = new Random();

    result = result + " - " + DateTime.Now.ToString("mm:ss.fff");
    return result;
}

The configured timeout to 500ms and the Time Taken reported is usually 501-504 ms (which is a very acceptable skid).

The problem is that every now and then I was seeing an error on the output windows saying that the response had already started. And I thought to myself: this cant be! this is happening 1 second earlier than the end of the Task.Delay on the corresponding controller.

So I opened up fiddler and (to my surprise) several requests are returning in 1.3-1.7 seconds WITH A FULL RESPONSE BODY.

By comparing the reported time written on the response body with the timestamp on fiddler "statistic" tab I can guarantee that the response im looking at does not belong to that request at hand!

Does anyone knows what's going on? Why is this "jumbling" happening?

Upvotes: 0

Views: 1417

Answers (1)

weichch
weichch

Reputation: 10055

Frankly, you're not using middleware in the way it is designed for.

You might want to read this middleware docs.

The ASP.NET Core request pipeline consists of a sequence of request delegates, called one after the other.

In your case, your middleware is running in parallel with the next middleware.

When a middleware short-circuits, it's called a terminal middleware because it prevents further middleware from processing the request.

If I understand you correctly, you might want to create such terminal middleware, but clearly your current one is not.

In your case, you have invoked the _next middleware, which means the request has already handed off to the next middleware in the request pipeline. The subsequent middleware components can start the response before the timeout has elapsed. i.e. a race condition between your middleware and a subsequent middleware.

To avoid the race condition, you should always check HasStarted before assigning the status code. And if the response has started, all you can do might only be aborting the request if you don't want the client to wait for too long.

static void ResetOrAbort(HttpContext httpContext)
{
    var resetFeature = httpContext.Features.Get<IHttpResetFeature>();
    if (resetFeature is not null)
    {
        resetFeature.Reset(2);
    }
    else
    {
        httpContext.Abort();
    }
}

app.Use(next =>
{
    return async context =>
    {
        var nextTask = next(context);
        var t = await Task.WhenAny(nextTask, Task.Delay(100));

        if (t != nextTask)
        {
            var response = context.Response;

            // If response has not started, return 408
            if (!response.HasStarted)
            {
                // NOTE: you will still get same exception
                // because the above check does not eliminate
                // any race condition
                try
                {
                    response.StatusCode = StatusCodes.Status408RequestTimeout;
                    await response.StartAsync();
                }
                catch
                {
                    ResetOrAbort(context);
                }
            }
            // Otherwise, abort the request
            else
            {
                ResetOrAbort(context);
            }
        }
    };
});

Upvotes: 1

Related Questions