30

So I understand why returning void from async would normally make no sense, but I've ran into a situation where I think it would be perfectly valid. Consider the following contrived example:

protected override void OnLoad(EventArgs e)
{
    if (CustomTask == null)
        // Do not await anything, let OnLoad return.
        PrimeCustomTask();
}
private TaskCompletionSource<int> CustomTask;

// I DO NOT care about the return value from this. So why is void bad?
private async void PrimeCustomTask()
{
    CustomTask = new TaskCompletionSource<int>();
    int result = 0;
    try
    {
        // Wait for button click to set the value, but do not block the UI.
        result = await CustomTask.Task;
    }
    catch
    {
        // Handle exceptions
    }
    CustomTask = null;

    // Show the value
    MessageBox.Show(result.ToString());
}

private void button1_Click(object sender, EventArgs e)
{
    if (CustomTask != null)
        CustomTask.SetResult(500);
}

I realize this is an unusual example, but I tried to make it simple and more generalized. Could someone explain to me why this is horrible code, and also how I could modify it to follow conventions correctly?

Thanks for any help.

AnotherProgrammer
  • 535
  • 1
  • 6
  • 13
  • 1
    Read this. https://msdn.microsoft.com/en-us/magazine/jj991977.aspx – Nkosi Aug 01 '17 at 21:33
  • @Nkosi I understand why it's a bad practice, but why is my code in particular a symptom of any of those bad practices? And how else could I do this if not using void async? – AnotherProgrammer Aug 01 '17 at 21:35
  • @AnotherProgrammer you can't catch error from your async void's isn't that enough to avoid them – johnny 5 Aug 01 '17 at 21:36
  • I can handle the exceptions within the async function, they don't need to be thrown. And even if they are they still can be caught in Main(), it's just ugly. You'd get the same problem with using a BackgroundWorker. – AnotherProgrammer Aug 01 '17 at 21:38
  • @AnotherProgrammer but why would you introduce a potential problem to your code that all other developers need to know about why not just return Task.From(0) – johnny 5 Aug 01 '17 at 21:39
  • 5
    @AnotherProgrammer This appears to be an [XY problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem). What is the ultimate goal you are trying to achieve? This example is unclear. – Nkosi Aug 01 '17 at 21:42
  • @johnny 5 And what shall I do with that result? I'm not going to await it, so I'm just going to get a compiler warning. – AnotherProgrammer Aug 01 '17 at 21:42
  • @Nkosi I have a WinForms control, and within a function I want to wait for another developer (this is a library) to call a method on my control to set a value. Once the value is set, I need to continue with the execution, all asynchronously. You're right this is an XY problem, but every "solution" leads to problems like this. – AnotherProgrammer Aug 01 '17 at 21:48
  • 4
    @AnotherProgrammer I suggest providing a [mcve] of the problem and what solutions were tried so that the problem can be better understood and viable solutions provided. – Nkosi Aug 01 '17 at 21:54
  • @Nkosi I'd normally adhere to that, but I don't really have an example to start with beyond a description of my problem. – AnotherProgrammer Aug 01 '17 at 21:55
  • 1
    @AnotherProgrammer: No, `async void` exceptions *cannot* be caught in `Main`. And `BackgroundWorker` doesn't have the same problem (though it has other problems). – Stephen Cleary Aug 01 '17 at 23:53
  • I was able to catch it without any issue in Main, it just throws a TargetInvocationException wrapping the real exception. It's the same as if you threw an exception from the RunWorkerCompleted event handler of a background worker. – AnotherProgrammer Aug 02 '17 at 14:22

2 Answers2

38

Well, walking through the reasons in the "avoid async void" article:

  • Async void methods have different error-handling semantics. Exceptions escaping from PrimeCustomTask will be very awkward to handle.
  • Async void methods have different composing semantics. This is an argument centered around code maintainability and reuse. Essentially, the logic in PrimeCustomTask is there and that's it - it can't be composed into a higher-level async method.
  • Async void methods are difficult to test. Following naturally from the first two points, it's very difficult to write a unit test covering PrimeCustomTask (or anything that calls it).

It's also important to note that async Task is the natural approach. Of the several languages that have adopted async/await, C#/VB are the only ones AFAIK that support async void at all. F# doesn't, Python doesn't, JavaScript and TypeScript don't. async void is unnatural from a language design perspective.

The reason async void was added to C#/VB was to enable asynchronous event handlers. If you change your code to use async void event handlers:

protected override async void OnLoad(EventArgs e)
{
  if (CustomTask == null)
    await PrimeCustomTask();
}

private async Task PrimeCustomTask()

Then the async void disadvantages are restricted to your event handler. In particular, exceptions from PrimeCustomTask are propagated naturally to its (asynchronous) callers (OnLoad), PrimeCustomTask can be composed (called naturally from other asynchronous methods), and PrimeCustomTask is much easier to include in a unit test.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
29

Using void async is only generally seen as "bad" because:

  • You can’t wait for its completion (as mentioned in this post already
  • Is especially painful if it is called by a parent thread that exits before it has completed
  • Any unhandled exceptions will terminate your process (ouch!)

There are plenty of cases (like yours) where using it is fine. Just be cautious when using it.

Cleptus
  • 3,446
  • 4
  • 28
  • 34
liam
  • 1,918
  • 3
  • 22
  • 28