0

This is my code, I'm using AsyncEx in my library to try to get around potential deadlocks, but I ended up in there anyway:

return AsyncContext.Run(async () =>
{
   var get = await httpClient.GetAsync(url);
   if (get.IsSuccessStatusCode && (get.StatusCode == HttpStatusCode.OK))
   {
      var content = await get.Content.ReadAsStringAsync();
      return content;
   }

   return "";
});

I'm running this from a command line app, calling it with multiple different url values in a row, but synchronously in one big for loop. Given enough calls, it will eventually stop dead in its tracks. Am I doing something wrong?

f00f
  • 123
  • 1
  • 11
  • `AsyncContext.Run` does use a thread from the thread pool. It's not normally called in a loop. So my first guess would be thread pool exhaustion? I'm not sure why your code uses `AsyncContext.Run` in the first place. – Stephen Cleary Jan 18 '20 at 00:30
  • @StephenCleary we have a requirement that we can't work around to create a sync call, and `HttpClient` is both newer and faster than its .Net sync alternative, `System.Net.WebRequest`. I'm not sure what you mean by "not normally called in a loop", it's a function that takes a `Task` and returns `T`, why would the manner in which it is called matter? Also, thread pool exhaustion makes sense if `AsyncContext.Run` is will return before the Task passed in is completed (if so, what does it return?) such that the next iteration of the loop is invoked and causes a new thread to be acquired. – f00f Jan 22 '20 at 21:00
  • If the way it works is that it 1) grabs a thread which 2) spins waiting for the `Task` to finish, 3) returns the thread, and then 4) the result is returned to the caller, then I'm not following how it might grab multiple threads even if called in an infinite loop. – f00f Jan 22 '20 at 21:03
  • `AsyncContext` provides a single-threaded context for asynchronous operations. It's most commonly used once in a Console app for its main thread, or sometimes in unit tests. I've never written (or seen) an application use more than one. If you're on a Console app and you're doing sync-over-async, why not just use `GetAwaiter().GetResult()`? – Stephen Cleary Jan 23 '20 at 02:37
  • I'm using only one at a time, in a Console app. `GetAwaiter().GetResult()` will also freeze, and that was the impetus to go looking for something that takes a `Task` and returns a `T`. – f00f Jan 23 '20 at 13:38

1 Answers1

2

What I think happens is that after calling GetAsync the continuation cannot switch back to the same thread since the other thread is waiting for the continuation to start. Does the following code work?

return AsyncContext.Run(async () =>
{
   var get = await httpClient.GetAsync(url).ConfigureAwait(false);
   if (get.IsSuccessStatusCode && (get.StatusCode == HttpStatusCode.OK))
   {
      var content = await get.Content.ReadAsStringAsync().ConfigureAwait(false);
      return content;
   }

   return "";
});
jkulhanek
  • 177
  • 8
  • Okay, I'm still getting... something... very weird. I put some logging in `async ()` function to try to see if it's the always the same place where it gets stuck. Before `await httpClient.GetAsync...` (1), then after (2), then after the `await get.Content.Read...` (3). I run it and it looks like 1, 2, 3, 1, 2, 3, 1, 2, 3, 1 and it stops there. Then, a little while later, I get a few more 1s in a row, which shouldn't be possible. The whole point is that this is blocking, being called in a loop. Eventually, actually, it unblocks and finishes. – f00f Jan 20 '20 at 01:24
  • I'm at a place that doesn't like jsfiddle/pastebin, so I have to use [this](https://ctxt.io/2/AABADhW4EA). Let me know if its expired, I'll put a new one up. – f00f Jan 21 '20 at 14:21
  • I meant the whole code including the main function. It is important how you call this code. – jkulhanek Jan 21 '20 at 15:32
  • Fair enough, I included more code going all the way up the stack [here](https://ctxt.io/2/AABAgeR9Fw). There's slightly more to it, but cut it out since it's un-executed noise in this context. – f00f Jan 21 '20 at 16:22
  • Ok, sorry to say that, but your code is very poorly written. You cannot call asynchronous code synchronously in the middle of the stack. There is a rule, that you should "async all the way up". You should replace any results types T with Task and change method signatures to async. The reason your code does not work is that you are synchronously invoking code (in a blocking fashion) so that the continuation from the awaited code does not have a thread to return to. – jkulhanek Jan 21 '20 at 19:17
  • Two things: one, there's no way to asynchronously invoke AsyncContext.Run unless the suggestion is that I do: Task.Factory.StartNew(() => AsyncContext.Run(...))) ? But, more importantly, I have a dilemma: I have to expose a synchronous call which does an Get call, so the only way for me to do it using .Net is with System.Net.WebRequest, which is very old. (There is, btw, another code path which does that exactly. But, it is, slightly slower than the HttpClient path.) – f00f Jan 22 '20 at 13:21
  • 1) Why would you ever need to expose a synchronous version of a method? You should always prefer async one if there is at least one async-enabled method call. 2) Why again do you need AsyncContext? And if you use that, there should be one call to AsyncContext.Run per thread (look at the example of the library). You should avoid calling that in the middle of the stack (async all way up). 3) That is the point of new .net api, since synchronous versions are obsolete and you should not use them anymore. If you don't care about the quality of your code, I suggest using older api. (WebClient) – jkulhanek Jan 22 '20 at 14:16
  • Also note, that the main function can also be async https://stackoverflow.com/questions/9208921/cant-specify-the-async-modifier-on-the-main-method-of-a-console-app . – jkulhanek Jan 22 '20 at 14:17
  • 1) The requirement comes from the out of our group, from consumers of the library. We do prefer async (actually originally only implemented those getters), but requirements are funny like that sometimes. 2) I don't understand what you mean; in the example I posted, there is only one call to AsyncContext.Run per thread. Also, the only way to use AysncContext.Run will turn an asynchronous method into a synchronous one, right? So, if that's true, then either of these needs to be true: turning an async method into sync can only be done in an async method or deadlocks are inevitable. – f00f Jan 22 '20 at 15:23
  • The following is true: async can never be turned into sync (safely). On some contexts, you can turn your async code into sync by calling .Result property or .Wait() method. This, however, is not safe and may cause deadlocks (e.g., in IIS applications). I don't know the exact conditions under which the deadlock occurs, but I think it is safe to call Wait (.Result) in a console app if you use ConfigureAwait(false) on every async call. https://stackoverflow.com/questions/9343594/how-to-call-asynchronous-method-from-synchronous-method-in-c – jkulhanek Jan 22 '20 at 15:41
  • Also look at the following link, where there is more insight into the problem. https://learn.microsoft.com/en-us/archive/msdn-magazine/2015/july/async-programming-brownfield-async-development – jkulhanek Jan 22 '20 at 15:43
  • If aysnc can never be turned into sync, then how is this function supposed to be used `TResult AsyncContext.Run(Func> action)`? How is this sync function invoked with `await`? – f00f Jan 22 '20 at 21:06
  • 1
    A suggest to avoid any prerelease libraries in your project: https://github.com/StephenCleary/AsyncEx/issues/107 it is supposed to work under some assumptions which are not always fulfilled. – jkulhanek Jan 24 '20 at 17:29
  • Thanks for that link. I think that's probably what's going on with my code as well. I just switched back to using WebRequest and/or RestSharp and will eat the performance loss on that code path for now. – f00f Jan 24 '20 at 20:11