-2

I know that there are similar questions, but I tried every best practice explained there, but the code still deadlocks.

The async method is called from a System.Timers.Timer Elapsed event with SynchronizingObject set to null, so the events will fire on the ThreadPool. So actually with Task.Run().Result run on the ThreadPool, I could also get deadlocks because Task.Run fires threads on the ThreadPool...

I need to do a REST call using HttpClient from a Windows Service which fires some System.Timers.Timer event every few seconds on the ThreadPool. Because HttpClient only has Async methods, I need to call async methods from the ThreadPool.

I tried every best practice that is being told here: https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html

At the moment it's just not an option to make the whole app async compatible because it would require a lot of refactoring. I need to make it work from synchronous code.

So in fact, I await the HttpClient method like this:

await HttpClient.PostAsync().ConfigureAwait(false);

And every await has also ConfigureAwait(false); until the place where I have to get the result of the Task.

I tried .Result and .GetAwaiter().GetResult() but these cause deadlocks and code hangs infinitely. Then I have to kill the service process from Task Manager...

Any help on how I can get the result from the Task without deadlocking the service?

I didn't try Task.Run<>(async () => await HttpClient.PostAsync()); yet, would this solve my issue? But it looks like a hack...

And please don't tell me to use some external Nuget Package, there should be a way without depending on an external package.

Ozkan
  • 3,880
  • 9
  • 47
  • 78
  • 2
    This doesn't make sense. Deadlocks occur when waiting synchronously on asynchronous code, but only when there is a synchronization context, and Windows services don't have a synchronization context. – Gabriel Luci Jan 20 '22 at 19:52
  • Then why is my code hanging infinitely if I do `.Result`. if I don't do it, then I get the task itself without issue – Ozkan Jan 20 '22 at 19:54
  • 1
    As much as it doesn't make sense why this happens at all, have you tried just not using `await` at all anywhere? e.g. `HttpClient.PostAsync().GetAwaiter().GetResult()` – Gabriel Luci Jan 20 '22 at 19:54
  • That's initially how it was... that deadlocks too. It's why I tried doing async/await. But at some point I need to do .Result anyway and that make my app lock too... – Ozkan Jan 20 '22 at 19:57
  • 1
    Can you post a short, but complete repro of some code that causes a deadlock? It's not clear from your question. – David Browne - Microsoft Jan 20 '22 at 19:58
  • You could make everything async too. See this question and answer: https://stackoverflow.com/questions/20585493/calling-async-methods-from-a-windows-service – Gabriel Luci Jan 20 '22 at 20:00
  • 1
    Ideally, you can figure out why there appears to be a synchronous context. Is it possible that your windows service has code somewhere that manually changes the task scheduler to use a synchronization context? Barring that, the next best thing is to go "async all the way." Since you've said that would require too much refactoring, you're stuck with a "hacky" option that involves using a separate thread, such as the Task.Run approach. With Task.Run You could probably even block and get the result synchronously inside that lambda so you don't have to change all the rest of your code to be async. – StriplingWarrior Jan 20 '22 at 20:09
  • How do you know it is the task that is deadlocked, and not the http post that tries to connect or waits for response? – Frank Nielsen Jan 20 '22 at 20:34
  • 2
    @Gabriel using .GetAwaiter().GetResult() should almost never be suggested - it is a *cause* of problems, not a fix (it is fundamentally similar to .Result); `await` should be preferred in almost all cases – Marc Gravell Jan 20 '22 at 23:38
  • GetAwaiter().GetResult() is advised because it propagates the exception and gives a readable error message, instead of `a error occurred` – Ozkan Jan 21 '22 at 07:20
  • @frank, I call the api with postman and it works – Ozkan Jan 21 '22 at 07:21
  • @All please reread my post, I added some context. Async method is called from a System.Timers.Timer event which works on the ThreadPool – Ozkan Jan 21 '22 at 08:22
  • `.GetAwaiter().GetResult()` is advised **over** just calling `.Result`, it is not advised over doing proper async. It has better problems than `.Result`, but it's not good. – Lasse V. Karlsen Jan 21 '22 at 08:34
  • Have you tried making the Elapsed event async? It will be `async void`, but that's fine. Just make sure you are using `try`/`catch` blocks to make sure there are no unhandled exceptions. – Gabriel Luci Jan 21 '22 at 15:23
  • It would be helpful if you show the code for your Elapsed event. – Gabriel Luci Jan 21 '22 at 15:25
  • @GabrielLuci gonna do. I see a `lock` statement on a static object in the Elapsed event. And multiple elapsed events come into the same method. Really weird code, but can't change it in short term unfortunately. And problem is I can't make awaits within `lock` statements. So making the Elapsed event async is not an option either – Ozkan Jan 21 '22 at 16:00

1 Answers1

3

Make your Elapsed event async. In your comment, you mentioned that you don't want to do this because the Elapsed event can be triggered again before the first one is finished. To avoid that, simply set AutoReset to false when you create your timer so that it doesn't automatically restart the timer. Then restart it at the end of your Elapsed event:

private async void OnTimedEvent(object source, ElapsedEventArgs e) {
    try {
        // Do stuff
    } finally {
        // Restart the timer
        timer.Enabled = true;
    }
}

The try/finally block ensures that the timer is restarted even if there is an exception. You can add a catch block in there too if needed.

This way, you don't need to use lock in your Elapsed event.

Gabriel Luci
  • 38,328
  • 4
  • 55
  • 84