Why does Task.Run(() => something) that calls a sync method that awaits deadlock?

Question

I'm trying to figure out where a deadlock is happening in a TPL program that I'm converting from using WebRequest/WebResponse to HttpClient.

I've got a loop that creates 100 tasks and puts them in an array:

var putTasks = new Task[100];
for (var i = 0; i < 100; i++)
{
    putTasks[i] = Task.Run(() => client.Put("something"));
}
Task.WaitAll(putTasks);

The code is hanging on Task.WaitAll and looking at the server side shows that the first request completes, but the rest don't.

The weird part about this (and yes, it should be async all the way through, but it's not right now, and I'm trying to understand why it's deadlocking before I make it entirely async) is that client.Put is a synchronous method that looks like this:

string Put(string thing) { return PutAsync(string thing).GetAwaiter().GetResult(); }

and PutAsync looks like:

async Task<string> PutAsync(string thing)
{
    //httpClient built up here
    HttpContent content = new StringContent(thing);

    var response = await httpClient.PutAsync("http://myuri", content);
    var result = await response.Content.ReadAsStreamAsync();
    // Do some stuff
}

The part it's hanging on inside PutAsync is the response = await httpClient.PutAsync. I've seen a bunch of answers around deadlocking in a UI thread but none of the fixes (e.g. changing the captured context) help.

Ideally I'd just make it all async all the way down and await everything but as I said, I'm trying to figure out why the deadlock is happening and where it's happening before I tear everything apart - the other problem is this is a library used by other people, and the interface is synchronous methods, so I can't just change it to async and update a bunch of hidden code.

Answer 1

Then please post an answer enlightening me on how I should take an API that provides synchronous methods (e.g. a client.Put(string thing) method and hook it up to an async method behind the scenes.

You don't. Doing so doesn't provide any benefit. Only the person who provided the library should be wrapping calls they know for fact call some type of Completion Port. Creating a separate thread (Task.Run()) to wait for a resource is simply wasting a resource (there are exceptions).

I can't just change it to async and update a bunch of hidden code.

And you shouldn't.

The part it's hanging on inside PutAsync is the response = await httpClient.PutAsync

99% of deadlocks are caused because the caller to MethodAsync() is done incorrectly, without that specific code, knowing why is unanswerable.

This is only useful if you know you are using completion ports

I can't demonstrate a PutAsync because the requirements to create a true async method aren't available on HttpClient. Instead I'll create on for SqlCommand for ExecuteNonQuery():

public static Task<int> ExecuteNonQueryAsync(this SqlCommand sqlCommand)
{
  var tcs = new TaskCompletionSource<int>();

  sqlCommand.BeginExecutedNonQuery(result =>
  {
    tcs.SetResult(sqlCommand.EndExecutedNonQuery());
  });

  return await tcs.Task;
}

Granted this is very poorly coded, no try/catch etc, but it doesn't use Task.Run() to start another thread that does nothing.

Now if you want to properly do Parallel Task calling with async tasks then something like this is more appropriate (and doesn't waste threads):

public async Task<IEnumerable<HttpResponseMessage>> GoCrazy()
{
  var putTasks = new List<Task<HttpResponseMessage>>();

  for (var i = 0; i < 100; i++)
  {
    var task = client.PutAsync("something");
    putTasks.Add(task);
  }

  // WhenAll not WaitAll
  var result = await Task.WhenAll(putTasks);

  return result;
}

Answer 2

The problem was all the running tasks to call the sync API filled up the threadpool so that the async calls that the HttpClient had to do had no threads to run on.