Encapsulating asynchronous method in Task.Factory.StartNew

Question

I have a good understanding of the task and the async / await pattern, but recently someone fixed a deadlock that he said was caused by:

public async Task<HttpResponseMessage> GetHttpResponse() { using (var client = new HttpClient()) { return await client.SendAsync(new HttpRequestMessage()).ConfigureAwait(false); } } 

He says that he fixed it in some way Task.Factory.StartNew .

 public HttpResponseMessage GetHttpResponse() { using (var client = new HttpClient()) { return Task.Factory.StartNew(() => client.SendAsync(new HttpRequestMessage()).Result).Result; } } 

First, the problems are why I have to change the return statement as HttpResponseMessage and not Task<HttpResponseMessage> .

My second question is why this code will solve the deadlock error. In my opinion, the fact that he calls .Result makes GetHttpResponse Thread wait (and freeze) until client.SendAsync completes.

Can someone try to explain to me how this code affects any TaskScheduler and SynchronizationContext .

Thank you for your help.

EDIT: Here is the caller method to provide more context for the problem.

 public IWebRequestResult ExecuteQuery(ITwitterQuery twitterQuery, ITwitterClientHandler handler = null) { HttpResponseMessage httpResponseMessage = null; try { httpResponseMessage = _httpClientWebHelper.GetHttpResponse(twitterQuery, handler).Result; var result = GetWebResultFromResponse(twitterQuery.QueryURL, httpResponseMessage); if (!result.IsSuccessStatusCode) { throw _exceptionHandler.TryLogFailedWebRequestResult(result); } var stream = result.ResultStream; if (stream != null) { var responseReader = new StreamReader(stream); result.Response = responseReader.ReadLine(); } return result; } // ... 

Answer 1

Modified code to eliminate the deadlock is a very poor use of Task APIs , I see so many problems:

• He converted Async call to Sync call . This is a blocking call due to the use of the result in Task
• Task.Factory.StartNew , Big No, check out Stephen Cleary's StartNew Dangerous

As for your source code, the following most likely reason for deadlock :

• The reason ConfigureAwait(false) does not work, you need to use it in the full call stack in all Async calls , so it leads to deadlock . The client.SendAsync point, and the complete call chain must have ConfigureAwait(false) so that it ignores the Synchronization context . This does not help to be in one place, and there is no guarantee for calls that are not under your control.

Possible Solution:

• Here, a simple deletion should work, you don't even need Task.WhenAll , since there are no multiple tasks

    using (var client = new HttpClient()) { return await client.SendAsync(new HttpRequestMessage()); } 

Another preferred option would be:

• Make your code synchronous as follows (now you do not need ConfigureAwait(false) in the whole chain):

   public HttpResponseMessage GetHttpResponse() { using (var client = new HttpClient()) { return await client.SendAsync(new HttpRequestMessage()).ConfigureAwait(false).GetAwaiter().GetResult(); } } 

Answer 2

The deadlock was not caused by return await client.SendAsync(new HttpRequestMessage()).ConfigureAwait(false); . This was caused by a blocking call to the stack. Since it is very unlikely that the MS version of HttpClient.SendAsync() has a built-in lock code (which can be blocked), it must be one of the callers of the public async Task<HttpResponseMessage> GetHttpResponse() that use .Wait() or .Result on the returned Task. Your whole colleague did this to move the blocking call onto the stack, where it is more visible. Moreover, this “fix” does not even resolve the classic deadlock , as it uses the same synchronization context (!). You can deduce that somewhere else on the stack another function unloads a new task without the asp.net synchronization context, and in this new (possibly by default) context, your GetHttpResponse() lock is GetHttpResponse() , otherwise the "fix" will also have dead end!

Since in the real world it is not always possible that obsolescence code for refactoring will be completely asynchronous, you should use your own async HttpClient interface and make sure that .ConfigureAwait(false) used in the implementation, since all infrastructure libraries should,

Answer 3

@ mrinal-kamboj @ panagiotis-kanavos @shay

Thank you all for your help. As already mentioned, I started reading Async in C# 5.0 from Alex Davies .

What i found

In chapter 8: Which thread runs my code, it mentions our case, and I think I found an interesting solution there:

You can work around the deadlock issue by going to the thread pool before running asynchronous code so that the captured SynchronizationContext is a thread pool and not a user interface thread.

 var result = Task.Run(() => MethodAsync()).Result; 

By Task.Run , it actually causes the asynchronous call to SynchronizationContext be SynchronizationContext ThreadPool (which makes sense). However, it also ensures that MethodAsync code MethodAsync not start and does not return to the main thread.

My decision

Given this, I changed my code as follows:

 public HttpResponseMessage GetHttpResponse() { using (var client = GetHttpClient()) { return TaskEx.Run(() => client.SendAsync(new HttpRequestMessage())).Result; } } 

This code works correctly for Console, WPF, WinRT, and ASP.NET. I will conduct further testing and update this post.

Questions

• Do you think this new code makes sense?
• Do you think this will prevent any potential impasse?

Note

In this book, I found out that .ConfigureAwait(false) only prevents the callback to call the SynchronizationContext.Post() method, which must be run on the caller's thread. In order to determine the thread to which the callback should be executed, it is important that the SynchronizationContext checks if the thread with which it is associated is associated. If so, then he chooses a different thread.

From my understanding, this means that the callback can be launched in any thread (UI-Thread or ThreadPool). Therefore, it does not guarantee non-execution in the UI-Thread, but makes it very unlikely.

NOTE (2)

It is interesting to note that the following code does not work:

 public async Task<HttpResponseMessage> GetHttpResponse() { using (var client = GetHttpClient()) { return await TaskEx.Run(() => client.SendAsync(new HttpRequestMessage())); 

When I tried to get this code, I meant that .Result can be used outside the ThreadPool area expected by .Result . To some extent, this makes sense to me, but if someone wants to comment on this, he will be happy :)