Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Deadlock with ContinueWiths in WebAPI

We've been running into a lot of deadlocks as part of exposing some of existing code over Web API. I've been able to distill the problem down to this very simple example that will hang forever:

public class MyController : ApiController
{
    public Task Get()
    {
        var context = TaskScheduler.FromCurrentSynchronizationContext();

        return Task.FromResult(1)
            .ContinueWith(_ => { }, context)
            .ContinueWith(_ => Ok(DateTime.Now.ToLongTimeString()), context);
    }
}

To me, that code seems simple enough. This might seem a bit contrived but that's only because I've tried simplifying the problem as much as possible. It seems having two ContinueWiths chained like this will cause the deadlock - if I comment out the first ContinueWith (that isn't really doing anything anyway), it will work just fine. I can also 'fix' it by not giving the specific scheduler (but that isn't a workable solution for us since our real code needs to be on the correct/original thread). Here, I've put the two ContinueWiths right next to each other but in our real application there is a lot of logic that is happening and the ContinueWiths end up coming from different methods.

I know I could re-write this particular example using async/await and it would simplify things and seems to fix the deadlock. However, we have a ton of legacy code that has been written over the past few years - and most of it was written before async/await came out so it uses ContinueWith's heavily. Re-writing all that logic isn't something we'd like to do right now if we can avoid it. Code like this has worked fine in all the other scenarios we've run into (Desktop App, Silverlight App, Command Line App, etc.) - it's just Web API that is giving us these problems.

Is there anything that can be done generically that can solve this kind of deadlock? I'm looking for a solution that would hopefully not involve re-writing all ContinueWith's to use async/await.

Update:

The code above is the entire code in my controller. I've tried to make this reproducible with the minimal amount of code. I've even done this in a brand new solution. The full steps that I did:

  1. From Visual Studio 2013 Update 1 on Windows 7 (with .NET Framework 4.5.1), create a new Project using the ASP.NET Web Application Template
  2. Select Web API as the template (on the next screen)
  3. Replace the Get() method in the auto-created ValuesController with the example given in my original code
  4. Hit F5 to start the app and navigate to ./api/values - the request will hang forever
  5. I've tried hosting the web site in IIS as well (instead of using IIS Express)
  6. I also tried updating all the various Nuget packages so I was on the latest of everything

The web.config is untouched from what the template created. Specifically, it has this:

<system.web>
   <compilation debug="true" targetFramework="4.5" />
   <httpRuntime targetFramework="4.5" />
</system.web>
like image 945
Stephen McDaniel Avatar asked Oct 01 '22 19:10

Stephen McDaniel


1 Answers

Try the following (untested). It's based on the idea that AspNetSynchronizationContext.Send executes the callback synchronously and thus should not result in the same deadlock. This way, we enter the AspNetSynchronizationContext on a random pool thread:

public class MyController : ApiController
{
    public Task Get()
    {
        // should be AspNetSynchronizationContext
        var context = SynchronizationContext.Current;

        return Task.FromResult(1)
            .ContinueWith(_ => { }, TaskScheduler.Default)
            .ContinueWith(_ =>
            {
                object result = null;
                context.Send(__ => { result = Ok(DateTime.Now.ToLongTimeString()); }, 
                    null);
                return result;
            }, TaskScheduler.Default);
    }
}

Updated, based on the comments, apparently it works and eliminates the deadlock. Further, I'd build a custom task scheduler on top of this solution, and use it instead of TaskScheduler.FromCurrentSynchronizationContext(), with very minimal changes to the existing code base.

like image 176
noseratio Avatar answered Oct 03 '22 08:10

noseratio