Web Api + HttpClient: An asynchronous module or handler completed while an asynchronous operation was still pending

Question

I'm writing an application that proxies some HTTP requests using the ASP.NET Web API and I am struggling to identify the source of an intermittent error. It seems like a race condition... but I'm not entirely sure.

Before I go into detail here is the general communication flow of the application:

• Client makes a HTTP request to Proxy 1.
• Proxy 1 relays the contents of the HTTP request to Proxy 2
• Proxy 2 relays the contents of the HTTP request to the Target Web Application
• Target Web App responds to the HTTP request and the response is streamed (chunked transfer) to Proxy 2
• Proxy 2 returns the response to Proxy 1 which in turn responds to the original calling Client.

The Proxy applications are written in ASP.NET Web API RTM using .NET 4.5. The code to perform the relay looks like so:

//Controller entry point.
public HttpResponseMessage Post()
{
    using (var client = new HttpClient())
    {
        var request = BuildRelayHttpRequest(this.Request);

        //HttpCompletionOption.ResponseHeadersRead - so that I can start streaming the response as soon
        //As it begins to filter in.
        var relayResult = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead).Result;

        var returnMessage = BuildResponse(relayResult);
        return returnMessage;
    }
}

private static HttpRequestMessage BuildRelayHttpRequest(HttpRequestMessage incomingRequest)
{
    var requestUri = BuildRequestUri();
    var relayRequest = new HttpRequestMessage(incomingRequest.Method, requestUri);
    if (incomingRequest.Method != HttpMethod.Get && incomingRequest.Content != null)
    {
       relayRequest.Content = incomingRequest.Content;
    }

    //Copies all safe HTTP headers (mainly content) to the relay request
    CopyHeaders(relayRequest, incomingRequest);
    return relayRequest;
}

private static HttpRequestMessage BuildResponse(HttpResponseMessage responseMessage)
{
    var returnMessage = Request.CreateResponse(responseMessage.StatusCode);
    returnMessage.ReasonPhrase = responseMessage.ReasonPhrase;
    returnMessage.Content = CopyContentStream(responseMessage);

    //Copies all safe HTTP headers (mainly content) to the response
    CopyHeaders(returnMessage, responseMessage);
}

private static PushStreamContent CopyContentStream(HttpResponseMessage sourceContent)
{
    var content = new PushStreamContent(async (stream, context, transport) =>
            await sourceContent.Content.ReadAsStreamAsync()
                            .ContinueWith(t1 => t1.Result.CopyToAsync(stream)
                                .ContinueWith(t2 => stream.Dispose())));
    return content;
}

The error that occurs intermittently is:

An asynchronous module or handler completed while an asynchronous operation was still pending.

This error usually occurs on the first few requests to the proxy applications after which the error is not seen again.

Visual Studio never catches the Exception when thrown. But the error can be caught in the Global.asax Application_Error event. Unfortunately the Exception has no Stack Trace.

The proxy applications are hosted in Azure Web Roles.

Any help identifying the culprit would be appreciated.

Solution 1

Your problem is a subtle one: the async lambda you're passing to PushStreamContent is being interpreted as an async void (because the PushStreamContent constructor only takes Actions as parameters). So there's a race condition between your module/handler completing and the completion of that async void lambda.

PostStreamContent detects the stream closing and treats that as the end of its Task (completing the module/handler), so you just need to be sure there's no async void methods that could still run after the stream is closed. async Task methods are OK, so this should fix it:

private static PushStreamContent CopyContentStream(HttpResponseMessage sourceContent)
{
  Func<Stream, Task> copyStreamAsync = async stream =>
  {
    using (stream)
    using (var sourceStream = await sourceContent.Content.ReadAsStreamAsync())
    {
      await sourceStream.CopyToAsync(stream);
    }
  };
  var content = new PushStreamContent(stream => { var _ = copyStreamAsync(stream); });
  return content;
}

If you want your proxies to scale a bit better, I also recommend getting rid of all the Result calls:

//Controller entry point.
public async Task<HttpResponseMessage> PostAsync()
{
  using (var client = new HttpClient())
  {
    var request = BuildRelayHttpRequest(this.Request);

    //HttpCompletionOption.ResponseHeadersRead - so that I can start streaming the response as soon
    //As it begins to filter in.
    var relayResult = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead);

    var returnMessage = BuildResponse(relayResult);
    return returnMessage;
  }
}

Your former code would block one thread for each request (until the headers are received); by using async all the way up to your controller level, you won't block a thread during that time.

Solution 2

I would like to add some wisdom for anyone else who landed here with the same error, but all of your code seems fine. Look for any lambda expressions passed into functions across the call-tree from where this occurs.

I was getting this error on a JavaScript JSON call to an MVC 5.x controller action. Everything I was doing up and down the stack was defined async Task and called using await.

However, using Visual Studio's "Set next statement" feature I systematically skipped over lines to determine which one caused it. I kept drilling down into local methods until I got to a call into an external NuGet package. The called method took an Action as a parameter and the lambda expression passed in for this Action was preceded by the async keyword. As Stephen Cleary points out above in his answer, this is treated as an async void, which MVC does not like. Luckily said package had *Async versions of the same methods. Switching to using those, along with some downstream calls to the same package fixed the problem.

I realize this is not a novel solution to the problem, but I passed over this thread a few times in my searches trying to resolve the issue because I thought I didn't have any async void or async <Action> calls, and I wanted to help someone else avoid that.

Solution 3

A slightly simpler model is that you can actually just use the HttpContents directly and pass them around inside the relay. I just uploaded a sample illustrating how you can rely both requests and responses asynchronously and without buffering the content in a relatively simple manner:

http://aspnet.codeplex.com/SourceControl/changeset/view/7ce67a547fd0#Samples/WebApi/RelaySample/ReadMe.txt

It is also beneficial to reuse the same HttpClient instance as this allows you to reuse connections where appropriate.

Comments

[Gavin Osborn]

Interesting - I will investigate your solution and come back to you. I'm aware that I have some perf improvements that I could make with Async - That was my next challenge once I eradicated this peculiar issue.

[Gavin Osborn]

Hi Stephen, doesn't your solution close the wrong stream? It closes (via dispose) the incoming stream and not the outgoing content stream and thus the request would never end (until timeout).

[Gavin Osborn]

Thanks Henrik, that is actually the solution I came up with in the end. Due to some of the particulars of our relay I can use that in 90% of the routes through our system - but I must rely on PushContent responses for the rest.

[Dan Friedman]

@Stephen The execution of copyToStreamAsync returns a Task, so var a is actually Task a, correct? I don't see any assurances, that that Task completes. Instead, I would change the construction to var content = new PushStreamContent((stream, h, t) => copyToStreamAsync(stream).Wait()); (Obviously, .Wait is to be avoided, but it seems sometimes necessary when mixing async and sync.) So why does yours work? How does my posted code compare?

[Stephen Cleary]

@DanFriedman: The _ is a Task, yes. _ completes after CopyToAsync completes. The key to this is that PushStreamContent will be considered "complete" when the stream is closed. So your code will only work if copyToStreamAsync(stream) closes stream. The main difference with Wait is that it will block a thread (and potentially cause deadlocks).

[Jonathan Wood]

This helped me. I haven't worked with async all too much but the lesson here seems to be that you should always set the return type of an async method to Task or Task<>, and it makes me think the language should require this. My code did not look like the OP's code, but I had a async void and changing it to async Task solved the issue.

[jokab]

@StephenCleary copyToStreamAsync is supposed to be copyStreamAsync. correct?

[Bern]

Excellent answer. This issue really had me scratching my head and this answer helped point me in the direction of the issue. In my case I was using MVC Web API and calling an async method from a constructor. Making the constructor call to a non-async method resolved the issue.