20

I like the new System.Net.Http.HttpClient class. It has a nice simple API, it doesn't throw on normal errors. But its async only.

I need code that goes (deep inside a server)

foo();
bar();
// compute stuff
var x = GetThingFromOtherServerViaHttp();
// compute more stuff
wiz(x);

classic sequential synchronous code. I saw several SO question that were similar but never actually ended up saying 'do this'. I looked at

client.PostAsync.Wait()

the world screams 'dont do this'. How about:

client.PostAsync.Result()

isnt this just Wait in disguise?

In the end I ended up passing in a lambda callback that processed the result and then awoke the calling thread that was explicitly waiting on a EventWaitHandle. A lot of plumbing. Is there something a little simpler or shoul I just go back to using the old http clients

EDIT:After further reading I suspect that this code has the same issues as Wait and Result, its just a more longwinded deadlock

EDIT: I had MS PM confirm to me recently that there is a mandate 'any API that might take > X ms (I forgot X) must be async', many PMs interpret this as 'async only' (not clear if this is what is intended). Hence the Document DB api being only async.

pm100
  • 48,078
  • 23
  • 82
  • 145
  • 1
    Just use `Result`, for example: http://stackoverflow.com/a/14435574/1663001 – DavidG Aug 12 '15 at 00:39
  • 4
    isnt .Result just .Wait in disguise? What different. The linked SO has the same question with links to articales saying 'dont do it' – pm100 Aug 12 '15 at 00:55
  • reading the code or result and wait, they both end up calling internalwait – pm100 Aug 12 '15 at 01:02
  • @DavidG if you want to make a deadlock, sure go right ahead. – Aron Aug 12 '15 at 01:06
  • What I dont understand is why there are methods (wait) that you should not call. They must work sometimes – pm100 Aug 12 '15 at 01:12
  • I believe the "real" answer can be found in a [MSDN blog post](http://blogs.msdn.com/b/pfxteam/archive/2012/04/13/10293638.aspx) that is referenced in the comments of the answer pointed out by @DavidG . This is far from perfect but at least it allows to avoid deadlock: `int Sync() { return Task.Run(() => Library.FooAsync()).Result; }`. – Philippe Aug 12 '15 at 01:34
  • @pm100 they do work sometimes. The issue is the vast majority of times async await calls are made to produce single threaded async. But when you mix that with blocking synchronous calls, you get a deadlock. The point IS that async await makes single threaded async easy (for UI threading). So in most cases it will deadlock. In a multithreaded scenario, `Task.Wait()` will work. But most people THINK async await automagically makes your code multithreaded, when its primary use case is to AVOID threading. – Aron Aug 12 '15 at 01:38
  • Overall I think that the synchronous calls on `Task` should not have been explicit public methods. – Aron Aug 12 '15 at 01:39
  • 1
    However. I noticed that `HttpClient` isn't a .net 4.0 class. It is a .net 4.5 class. Which means that you have access to async await. So why don't you use it? – Aron Aug 12 '15 at 01:43
  • If you're not concerned about scalability (blocking a thread waiting for the result) or about a possibility of synchronization context-related deadlocks, I suggest you'd use `task.GetAwaiter().GetResult()` instead of `task.Wait` or `task.Result`. That'd make exception handling a bit less obtrusive. – noseratio Aug 12 '15 at 02:24
  • @Aron - I meant 4.5 - was a typo – pm100 Aug 12 '15 at 15:51
  • @Noseratio - of course I am concerned about deadlocks :-) – pm100 Aug 12 '15 at 15:52
  • 2
    @pm100: Why not use `WebClient`? – Stephen Cleary Aug 13 '15 at 12:29
  • because webclient throws on 404. – pm100 Aug 13 '15 at 15:35
  • Honestly, I think this is the kind of "how do I" question that really needs more attention on the "why". It could even lead others in the wrong direction. HttpClient was designed async only for very good reason. Would you say you are beyond convincing that async/await is the right way to go here? You understand that your code would still behave sequentially, that in your example `wiz` would not be called until `x` is available (if awaited properly), and that you won't end up with node.js-like callback soup in your code? – Todd Menier Aug 13 '15 at 18:44
  • 1
    @ToddMenier I said in my question that there are features of HttpClient I like that no other client offers. In particular it doesnt throw. I having a nagging fear that MS will start making new features that are only async, and yet for many reasons many apps are not async eveywhere - this being one case – pm100 Aug 13 '15 at 20:00
  • In confirmation of my concerns. DocumentDB API is *only* available as async – pm100 May 12 '17 at 15:46

3 Answers3

9

From http://blogs.msdn.com/b/pfxteam/archive/2012/04/13/10293638.aspx:

return Task.Run(() => Client.PostAsync()).Result;
Jeff Dunlop
  • 893
  • 1
  • 7
  • 20
  • 6
    The article actually says *don't* do this unless you absolutely have to. I'm not sure the OP's usage qualifies under the narrow path of when you would want to do this that is described in the article. – Matt Johnson-Pint May 24 '16 at 00:09
  • 4
    That's fine, leave him a comment to that effect, but "you don't want to do that" really isn't an answer. I did answer his question with the way that sucks least. – Jeff Dunlop Jul 18 '16 at 09:25
  • 1
    Can this cause deadlocks? If it can't, I would gladly use it. The thing is, I've read that calling .Result(), .Wait() or other things on HttpClient directly could cause deadlocks. – Dexter Feb 06 '17 at 15:57
  • 3
    **Don't do that!** What's the point of using a task to run an *asynchronous* method only to block on it? Either use the method properbly with `var result=await Client.PostAsync();` or block directly with `Client.PostAsync().Result`. If you fear that awaiting can block, eg because the UI is blocked by another operation, use `ConfigureAwait(false)`, ie `var result=await Client.PostAsync().ConfigureAwait(false);`; – Panagiotis Kanavos Feb 09 '17 at 12:37
  • 2
    @PanagiotisKanavos `Task.Run(() => Client.PostAsync())` is pretty much what `Client.PostAsync().ConfigureAwait(false)` does. It ensures that the continuation of `Client.PostAsync()` will be scheduled in the ThreadPool and not in the context of the initial call. What `Task.Run` does is wrapping the async call of the `Client` inside an async call that will be executed in the ThreadPool. As a result, the continuation of `Client` async call will be scheduled to the ThreadPool context. `ConfigureAwait` was introduced pretty much to do the same think in a framework supported way. – Thanasis Ioannidis Mar 22 '17 at 10:28
7

@Noseratio - of course I am concerned about deadlocks :-)

You only would be concerned about deadlocks if you're on a thread with synchronization context.

That would normally be either the main thread of a UI application (client-side), or random ASP.NET thread processing an HTTP request (server-side). In either case you really shouldn't be blocking it.

The accepted answer might help mitigating the deadlock, but blocking like this would still hurt the end user experience of your UI app (the UI would be frozen), or the server-side app scalability (a thread wasted with blocking, while it could be serving another request). Just don't block and use async/await all the way through.

You mentioned "deep inside a server" but provided no details on what kind of server-side app is that. Most of the modern server-side frameworks have a good plumbing for async/await, so it shouldn't be a problem embracing it.

Updated to address the comment:

I still dont like the fact that the same code written in different places will deadlock. I would expect the wait call in a deadlock-prone environment to throw

This is not particularly a problem of async/await but is that of synchronization context concept in general, when it's used with blocking code. Here is the deadlock in a nutshell:

private void Form1_Load(object sender, EventArgs e)
{
    var mre = new System.Threading.ManualResetEvent(initialState: false);
    System.Threading.SynchronizationContext.Current.Post(_ => 
        mre.Set(), null);
    mre.WaitOne();
    MessageBox.Show("We never get here");
}

In theory, it might be possible to try to mitigate potential deadlocks inside SynchronizationContext.Post, say by checking Thread.ThreadState == System.Threading.ThreadState.WaitSleepJoin. That would not however be a 100% reliable solution.

noseratio
  • 59,932
  • 34
  • 208
  • 486
  • good answer, but I still dont like the fact that the same code written in different places will deadlock. I would expect the wait call in a deadlock-prone environment to throw. – pm100 Aug 12 '15 at 23:35
  • i have to say that I dont agree about 'async all the way'. Writing every server to look like node.js doesnt have a lot of advantages and has a lot of downside. I hope the .net team doesnt get into the habit of writing new APIs that are 'async only' – pm100 Aug 12 '15 at 23:39
  • 1
    @pm100 firstly, it is a technical problem. It is very hard to write the framework otherwise. But more importantly, I disagree with you that there are disadvantages to async all the way down. On the contrary you should learn about the scalability advantages of async all the way down. It's why a Java webserver can only serve ~1000 concurrent requests, whilst a single thread in node.js can serve millions with a fraction of the resources. – Aron Aug 13 '15 at 02:40
  • @pm100, check my update. As to node.js, there it's common to struggle with callback pyramids of hell (but that problem is coming to to the end with ES6 generators and ES7 `async/await`, either). With C# `async/await` you naturally have pseudo-linear, pseudo-synchronous flow of non-blocking code, which can easily be converted to from existing synchronous code. E.g., with ASP.NET you simply make your controller methods async and replace synchronous blocking calls with their asynchronous equivalents + `await`. Even legacy WebForms have `RegisterAsyncTask, etc. – noseratio Aug 13 '15 at 04:58
  • I note that the documentdb API is async only. – pm100 May 03 '17 at 19:50
  • @Noseratio - then why is it sometimes OK / no deadlock when accessing .Result in a Web Controller? – Don Cheadle May 17 '17 at 01:13
  • @mmcrae, in your controller, what's the value of `System.Threading.SynchronizationContext.Current` right before calling `Client.PostAsync().Result` ? – noseratio May 17 '17 at 02:49
  • Not in my IDE/project right now. But I gather you're saying as long as there's not 2 different contexts, there's no problem? – Don Cheadle May 17 '17 at 03:11
  • @mmcrae, I'd rather expect there might be no context at all, like with ASP.NET Core. – noseratio May 17 '17 at 07:49
  • @Noseratio - I guess I'm really wondering why `.Result` causes a dead lock in certain cases, when obviously SOMETHING must wait/call `.Result` for the result to do anything. – Don Cheadle May 17 '17 at 13:11
  • @mmcrae, I guess that's because the designers of this API don't want us to call `task.Resut`. They want us to use `await task` all the way through, including in a web controller, where controller's actions should be asynchronous. – noseratio May 17 '17 at 18:45
1

Controversially I am going to say for System.Net.Http.HttpClient you probably can just use .Result

Microsoft should have followed their own advice and used .ConfigureAwait(false) all the way down that library. The reference source implies this:

https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpClient.cs

Why take the risk? Because it should be slightly more lightweight than Task.Run(() => Client.PostAsync()).Result.

I wouldn't trust many (any?) other libraries to be coded safely and look forward to seeing if anyone argues against my answer or better yet proves me wrong.

KCD
  • 9,873
  • 5
  • 66
  • 75
  • In some cases, having `.Result` inside of a .NET MVC controller for example can cause deadlock. Usually if something else is doing `await` below the `.Result` – Don Cheadle May 16 '17 at 23:03
  • Correct if you go from async to sync higher up the call chain and did not use `.ConfigureAwait(true)` all the way down to `HttpClient`. In other words your `MyAsyncHttpThing` was not safe from deadlocks and the simplest rule to follow is still "never use `.Result`" – KCD May 18 '17 at 18:20
  • Unless you mean mixing `.Result` on `HttpClient` in a async Controller.... which is very interesting I have not considered that case. But there is no good reason to do that either?? – KCD May 18 '17 at 18:24
  • I think you mean `.ConfigureAwait(false)` btw. Passing false means that when it resumes, it should NOT attempt to resume on the context that it was started by (hence in a Controller, it should NOT try to resume on the UI context. That causes deadlocks) – Don Cheadle May 18 '17 at 21:19
  • http://blog.stephencleary.com/2012/07/dont-block-on-async-code.html this is an example of how .Result inside of a Controller can cause a deadlock (key is that only one thread can be on the UI context at a time, and .Result will wait *inside the UI context* which prevents the await'ed thing from getting back in the context to return) – Don Cheadle May 18 '17 at 21:26