Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to trigger (NOT avoid!) an HttpClient deadlock

There are a number of questions on SO about how to avoid deadlocks in async code (for example, HttpClient methods) being called from sync code, like this. I'm aware of the various ways to avoid these deadlocks.

In contrast, I'd like to learn about strategies to aggravate or trigger these deadlocks in faulty code during testing.

enter image description here

Here's an example bit of bad code that recently caused problems for us:

public static string DeadlockingGet(Uri uri)
{
    using (var http = new HttpClient())
    {
        var response = http.GetAsync(uri).Result;
        response.EnsureSuccessStatusCode();
        return response.Content.ReadAsStringAsync().Result;
    }
}

It was being called from an ASP.NET app, and thus had a non-null value of SynchronizationContext.Current, which provided the fuel for a potential deadlock fire.

Aside from blatantly misusing HttpClient, this code deadlocked in one of our company's servers... but only sporadically.

My attempt to repro deadlock

I work in QA, so I tried to repro the deadlock via a unit test that hits a local instance of Fiddler's listener port:

public class DeadlockTest
{
    [Test]
    [TestCase("http://localhost:8888")]
    public void GetTests(string uri)
    {
        SynchronizationContext.SetSynchronizationContext(new SynchronizationContext());
        var context = SynchronizationContext.Current;
        var thread = Thread.CurrentThread.ManagedThreadId;
        var result = DeadlockingGet(new Uri(uri));
        var thread2 = Thread.CurrentThread.ManagedThreadId;
    }
}

A couple things to note:

  • By default, a unit test has a null SynchronizationContext.Current, and so .Result captures the context of TaskScheduler, which is the thread pool context. Therefore I use SetSynchronizationContext to set it to a specific context, to more closely emulate what happens in an ASP.NET or UI context.

  • I've configured Fiddler to wait a while (~1 minute) before responding back. I've heard from coworkers that this may help repro the deadlock (but I have no hard evidence this is the case).

  • I've ran it with debugger to make sure that context is non-null and thread == thread2.

Unfortunately, I've had no luck triggering deadlocks with this unit test. It always finishes, no matter how long the delay in Fiddler is, unless the delay exceeds the 100-second default Timeout of HttpClient (in which case it just blows up with an exception).

Am I missing an ingredient to ignite a deadlock fire? I'd like to repro the deadlocks, just to be positive that our eventual fix actually works.

like image 515
DumpsterDoofus Avatar asked Dec 23 '16 19:12

DumpsterDoofus


2 Answers

It seems you think that setting any synchronization context might cause deadlock with async code - that is not true. It is dangerous to block on async code in asp.net and UI applications because they have special, single, main thread. In UI applications that is, well, main UI thread, in ASP.NET applications there are many such threads, but for given request there is one - request thread.

Synchronization contexts of ASP.NET and UI applications are special in that they basically send callbacks to that one special thread. So when:

  1. you execute some code on this thread
  2. from that code you execute some async Task and block on it's Result.
  3. That Task has await statement.

Deadlock will occur. Why this happens? Because continuation of async method is Posted to current synchronization context. Those special contexts we discuss above will send those continuations to the special main thread. You already execute code on this same thread and it is already blocked - hence deadlock.

So what are you doing wrong? First, SynchronizationContext is not special context we discussed above - it just posts continuations to thread pool thread. You need another one for tests. You can either use existing (like WindowsFormsSynchronizationContext), or create simple context which behaves the same (sample code, ONLY for demonstration purposes):

class QueueSynchronizationContext : SynchronizationContext {
    private readonly BlockingCollection<Tuple<SendOrPostCallback, object>> _queue = new BlockingCollection<Tuple<SendOrPostCallback, object>>(new ConcurrentQueue<Tuple<SendOrPostCallback, object>>());
    public QueueSynchronizationContext() {
        new Thread(() =>
        {
            foreach (var item in _queue.GetConsumingEnumerable()) {
                item.Item1(item.Item2);
            }
        }).Start();
    }        

    public override void Post(SendOrPostCallback d, object state) {
        _queue.Add(new Tuple<SendOrPostCallback, object>(d, state));
    }

    public override void Send(SendOrPostCallback d, object state) {
        // Send should be synchronous, so we should block here, but we won't bother
        // because for this question it does not matter
        _queue.Add(new Tuple<SendOrPostCallback, object>(d, state));
    }
}

All it does is puts all callbacks to single queue and executes them one by one on separate, single thread.

Simulating deadlock with this context is easy:

class Program {
    static void Main(string[] args)
    {
        var ctx = new QueueSynchronizationContext();
        ctx.Send((state) =>
        {
            // first, execute code on this context
            // so imagine you are in ASP.NET request thread,
            // or in WPF UI thread now                
            SynchronizationContext.SetSynchronizationContext(ctx);
            Deadlock(new Uri("http://google.com"));   
            Console.WriteLine("No deadlock if got here");
        }, null);
        Console.ReadKey();
    }

    public static void NoDeadlock(Uri uri) {
        DeadlockingGet(uri).ContinueWith(t =>
        {
            Console.WriteLine(t.Result);
        });
    }

    public static string Deadlock(Uri uri)
    {
        // we are on "main" thread, doing blocking operation
        return DeadlockingGet(uri).Result;
    }

    public static async Task<string> DeadlockingGet(Uri uri) {
        using (var http = new HttpClient()) {
            // await in async method
            var response = await http.GetAsync(uri);
            // this is continuation of async method
            // it will be posted to our context (you can see in debugger), and will deadlock
            response.EnsureSuccessStatusCode();
            return response.Content.ReadAsStringAsync().Result;
        }
    }
}
like image 168
Evk Avatar answered Oct 23 '22 11:10

Evk


You have not been able to reproduce the issue because SynchronizationContext itself does not mimic the context installed by ASP.NET. The base SynchronizationContext does no locking or synchronization, but the ASP.NET context does: Because HttpContext.Current is not thread-safe nor is it stored in the LogicalCallContext to be passed between threads, the AspNetSynchronizationContext does a bit of work to a. restore HttpContext.Current when resuming a task and b. lock to ensure that only one task is running for a given context.

A similar problem exists with MVC: http://btburnett.com/2016/04/testing-an-sdk-for-asyncawait-synchronizationcontext-deadlocks.html

The approach given there is to test your code with a context which ensures that Send or Post is never called on the context. If it is, this is an indication of the deadlocking behavior. To resolve, either make the method tree async all the way up or use ConfigureAwait(false) somewhere, which essentially detaches the task completion from the sync context. For more information, this article details when you should use ConfigureAwait(false)

like image 25
Dark Falcon Avatar answered Oct 23 '22 09:10

Dark Falcon