-
Notifications
You must be signed in to change notification settings - Fork 783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Async void is not always bad #86
Comments
Here is an article from years ago explaining how unobserved Task exceptions can cause crashes: https://devblogs.microsoft.com/pfxteam/task-exception-handling-in-net-4-5/ And here is a more recent conversation about it (with a comment from the same author) explaining that unobserved exceptions on Tasks will cause the Also see:
And here is an example of it crashing the process: try
{
Console.WriteLine("Testing async void method");
Test();
}
catch
{
Console.WriteLine("caught exception"); // never hit
}
// if we get this far, the loop should never exit right?
while (true)
{
await Task.Delay(10);
Console.WriteLine("waiting for task to be GCed");
}
async void Test()
{
await Task.Yield();
throw new Exception("oops");
} When you call an async void method, there is a Task involved, but not one that you can try to handle exceptions for. |
I did not finish studiying but this is really interesting. I still find it hard to believe that async void is generally evil since it is explicetly said (see referenced article) to be fine in some scenarios. What is it now? |
I've also seen similar issues where a developer might accidentally cause the process to crash by not realizing they were creating unobserved Task exceptions in writing essentially
myList.ForEach(async element =>
{
await SomeMethodThatThrows(element);
}); This will also crash the process - and can be difficult to debug if the crash might not be immediately when this code executes, but instead some time later when the task is finalized. This is what is mentioned in this repo here: https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#implicit-async-void-delegates |
It's more just about the fact that it is hard or impossible to properly handle exceptions when you are calling an The linked article doesn't specifically say it is "fine" to use |
I just wrote a very simple test that basically proofs you right:
This test throws an exception. To workaround that one could either catch exceptions or wrap the lambda in Task.Run()
|
Fine but https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#async-void states |
Ah so this guidance is specific to ASP.NET CORE:
In ASP.NET Core there shouldn't ever be a place where you have to write The other examples are for things like UWP where you have no other choice. (I haven't written UWP in a while so that may have changed, not positive) |
@dferretti So, in this case, we need to use async void TimerCallback(object? state)
{
try
{
await TimerCallbackCore(state);
}
catch (Exception ex)
{
}
}
async Task TimerCallbackCore(object? state)
{
// do something
}
new Timer(TimerCallback) |
PeriodicTimer for this case is better choice for async code since .net 6 |
PeriodicTimer.WaitForNextTickAsync will block the caller, it's not what I want. |
It's what you want, and if you want to ignore backpressure then you can always fire and forget. |
I wouldn't agree on the statement that async void is evil. There are definitely exceptions where you're fine to use async void.
Moreover I'd really appreciate more details on how async void can cause process crashes when exceptions occur.
To summarize this first guideline, you should prefer async Task to async void. Async Task methods enable easier error-handling, composability and testability. The exception to this guideline is asynchronous event handlers, which must return void. This exception includes methods that are logically event handlers even if they’re not literally event handlers (for example, ICommand.Execute implementations).
from
https://docs.microsoft.com/en-us/archive/msdn-magazine/2013/march/async-await-best-practices-in-asynchronous-programming
The text was updated successfully, but these errors were encountered: