1

Any issue with this C# method (.Net Framework 4.8) with not having await? I am using ReadAsStringAsync() with no await. Caller of RetrieveContent() method can't call it asynchronously. So, I need to avoid making it async method. This is a Windows Console app that can also be installed as a Windows Service- with an endpoint listening for requests. It is supposed to do Synchronous processing- one request at a time.

public string RetrieveContent()
{
  return new HttpClient().GetAsync("https://www.google.com").Result.Content.ReadAsStringAsync().Result;
}

OR

public string RetrieveContent()
{
  var response = new HttpClient().GetAsync("https://www.google.com").Result;
  return response.Content.ReadAsStringAsync().Result;
}

Updates: I can change like this. Thus, caller to this method doesn't need an information from Async method. Will this cause a dead-lock too?

public void RetrieveContent()  //this is just logging content
{
  var response = new HttpClient().GetAsync("https://www.google.com").Result;

  if (response.StatusCode == HttpStatusCode.OK)
    _logger.LogInformation($"content: {response.Content.ReadAsStringAsync().Result} ");  //_logger is logging to local disk I/O
  else 
    _logger.LogError($"HttpStatusCode: {response.StatusCode} ");
}
'''
ritikaadit2
  • 171
  • 1
  • 10
  • 5
    `Any issue` - https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html – GSerg Jul 10 '22 at 18:59
  • 1
    Does this answer your question? [Using await versus Result](https://stackoverflow.com/questions/27992435/using-await-versus-result) – gunr2171 Jul 10 '22 at 19:02
  • @GSerg, when .Result is used, I think, .Net might be blocking (I think). – ritikaadit2 Jul 10 '22 at 19:04
  • 2
    When `.Result` is used, .Net will certainly be blocking, and will likely to be deadlocking, too. Did you read the article? – GSerg Jul 10 '22 at 19:05
  • 1
    Use WebClient which is synchronous. – Wiktor Zychla Jul 10 '22 at 19:05
  • @GSerg it's not very likely to be deadlocking. These are built-in methods, not methods written by some random guy who is not familiar with `.ConfigureAwait(false)`. – Theodor Zoulias Jul 10 '22 at 19:31
  • 2
    @TheodorZoulias that's irrelevant; the outermost awaitable (the one being accessed badly) by itself is enough to deadlock in the scenarios where deadlock is an issue – Marc Gravell Jul 10 '22 at 20:48
  • 1
    @MarcGravell could you demonstrate a consistent deadlocking behavior, caused by blocking synchronously on asynchronous `HttpClient` methods, in an otherwise completely synchronous application? [Here](https://dotnetfiddle.net/8WysB6) is a playground to get you started. It's a console app that has a proper `SynchronizationContext` installed on its main thread. – Theodor Zoulias Jul 10 '22 at 21:10
  • @TheodorZoulias, following up with your post.. – ritikaadit2 Jul 10 '22 at 21:19
  • @GSerg Meanwhile, I changed the code (see "Updates:" section in my post). Thank you for the article, though. I briefly went through it. I will read it in a day or two. But, my caller is not an UI thread. Its a Windows Service Timer wakeup thread – ritikaadit2 Jul 10 '22 at 21:23
  • 1
    @TheodorZoulias that's really not how I plan on spending my Sunday evening. Let's just keep this simple: any time you do sync over async, you're basically in undefined behaviour territory. Rather than arguing minutiae, how about we all just step away from the minefield and stop lying to folks by pretending that there exists scenarios where this is just fine and dandy. – Marc Gravell Jul 10 '22 at 21:27
  • 1
    @MarcGravell hmmm, honestly I would appreciate more a simple example, around 10 lines of code, that would demonstrate beyond any doubt that blocking on asynchronous `HttpClient` methods can cause a deadlock. If you know the mechanism that creates the deadlock, you should be able to demonstrate it in no more than 10 minutes. It won't be a waste of your time. It will be a useful lesson for me, the OP and anyone who is in doubt, and it will also prove that you know what you are talking about. So, no async/await, only `HttpClient`+`Result`. Please show the deadlock. – Theodor Zoulias Jul 10 '22 at 21:47
  • @TheodorZoulias 99.999% of code I've seen which does sync over async does not use `AsyncContext`. Note that `AsyncContext` is a third-party library, so basically out of scope anyway. Try run that code (without the `AsyncContext`) in a WinForms or WPF app, you will see an immediate deadlock. – Charlieface Jul 11 '22 at 14:55
  • @Charlieface the `Nito.AsyncEx.AsyncContext` is a pretty good simulation of the `WindowsFormsSynchronizationContext`. The `AsyncContext.Run` method is similar to the `Application.Run`. Both methods are blocking. They hijack the current thread, and install a message loop on it. The `AsyncContext` uses a `BlockingCollection>` internally as the message loop. Whatever pattern causes a deadlock on the one context, will cause it on the other too. Nevertheless I'll copy-paste the same code in a WinForms app, and I'll post a screenshot here. – Theodor Zoulias Jul 11 '22 at 15:10
  • @Charlieface [here](https://prnt.sc/hOEJqoebrLEl) is the screenshot from my experiment. I copy-pasted the same code in the `Form_Shown` event handler of a Windows Application (.NET 6). A message box appeared, that I can grab with my mouse and drag all over the screen. This experiment didn't produce a deadlock. Please try to create one, and come up with your findings. – Theodor Zoulias Jul 11 '22 at 15:31
  • ritikaadit2 what is the type of your .NET Framework 4.8 application? Is it WinForms? ASP.NET? Windows Service? Also why have you tagged the question with the `async-await` tag? This tag is about the `async` and `await` keywords. For questions about asynchrony in general, the correct tag is the `asynchronous`. – Theodor Zoulias Jul 11 '22 at 17:15
  • 1
    @TheodorZoulias: Historically, it has been possible to deadlock on `HttpClient` asynchronous methods. IIRC, the `HttpClient` for Windows Phone Apps was missing a `ConfigureAwait(false)` and could deadlock. AFAIK, no *modern* `HttpClient` implementations have this limitation, but I would be wary of declaring any such usage "safe". It's the kind of thing where a bug would be really easy to slip in. – Stephen Cleary Jul 12 '22 at 00:03
  • @StephenCleary after that failure, I would expect that Microsoft has put some tests in place, that would prevent a regression to this old bug. It's definitely their responsibility to ensure that their asynchronous APIs don't deadlock when used in a synchronous manner. Although I could envision a future where Microsoft would mess up again, and then [throw the blame to the developers](https://github.com/dotnet/runtime/issues/66255#issuecomment-1060214075) for the bugs! – Theodor Zoulias Jul 12 '22 at 05:05
  • @TheodorZoulias, my app is a Windows Console application that can also be installed as a Windows Service. It does Synchronous processing per request from an entrypoint. – ritikaadit2 Jul 12 '22 at 13:37
  • How does it work? Does it start a new thread for each new entrypoint? How many entrypoints can be processed concurrently at maximum? – Theodor Zoulias Jul 12 '22 at 14:25

1 Answers1

3

Calling Result from synchronous code blocks is considered unsafe and may cause deadlocks because the task might depend on other incomplete tasks. Instead, you should usually be striving to make caller methods async as much as possible. In this specific case, however, it may be okay with a Task.Run wrapper.

See this question for more details: What's the "right way" to use HttpClient synchronously?

Sedat Kapanoglu
  • 46,641
  • 25
  • 114
  • 148