Issues with "void async" in Winform event handlers - and maybe a solution?

Question

As far as I can see when an "async void" method (such as an event handler) is called, the caller can never know when it has completed (as it can't wait for the Task to complete). So effectively its a fire and forget call.

This is demonstrated with this code (i've put a Button and TabControl onto a form and hooked up the 2 events). When the button is clicked it changes the tab, this causes the SelectedIndexChanged event to be raised which is async.

    private void button1_Click(object sender, EventArgs e)
    {
        Debug.WriteLine("Started button1_Click");
        tabControl1.SelectedTab = tabControl1.SelectedIndex == 0 ? tabPage2 : tabPage1;
        Debug.WriteLine("Ended button1_Click");
    }

    private async void tabControl1_SelectedIndexChanged(object sender, EventArgs e)
    {
        Debug.WriteLine("Started tabControl1_SelectedIndexChanged");
        await Task.Delay(1000);
        Debug.WriteLine("Ended tabControl1_SelectedIndexChanged");
    }

The resulting output is

Started button1_Click
Started tabControl1_SelectedIndexChanged
Ended button1_Click
Ended tabControl1_SelectedIndexChanged

As you can see the SelectedIndexChanged event handler was fired, but the caller did not wait not wait for it to complete (it couldn't as it had no Task to await).

My Proposed solution

Instead of using async the event handler waits on any Async methods it uses, then everything seems to work... It waits by polling the Task.IsCompleted property while calling DoEvents in order to keep the async task alive and processing (in this case Task.Delay).

    private void button1_Click(object sender, EventArgs e)
    {
        Debug.WriteLine("Started button1_Click");
        tabControl1.SelectedTab = tabControl1.SelectedIndex == 0 ? tabPage2 : tabPage1;
        Debug.WriteLine("Ended button1_Click");
    }

    private void tabControl1_SelectedIndexChanged(object sender, EventArgs e)
    {
        Debug.WriteLine("Started tabControl1_SelectedIndexChanged");
        Await(Task.Delay(1000));
        Debug.WriteLine("Ended tabControl1_SelectedIndexChanged");
    }

    public static void Await(Task task)
    {
        while (task.IsCompleted == false)
        {
            System.Windows.Forms.Application.DoEvents();
        }

        if (task.IsFaulted && task.Exception != null)
            throw task.Exception;
        else
            return;
    }

This now gives the expected result

Started button1_Click
Started tabControl1_SelectedIndexChanged
Ended tabControl1_SelectedIndexChanged
Ended button1_Click

Can anyone see any issues with taking this approach???

Answer 1

It waits by polling the Task.IsCompleted property while calling DoEvents

This is one form of blocking on asynchronous code which I call the nested message loop hack.

Can anyone see any issues with taking this approach?

Yes. This kind of solution suffers from what I call "unexpected reentrancy".

Unexpected reentrancy historically has been the cause of many, many bugs, because some code inevitably ends up being called from within other code (on the same stack). In your example, tabControl1_SelectedIndexChanged (or more specifically, Await) may execute any other UI code directly. Including another invocation of tabControl1_SelectedIndexChanged. This causes problems as soon as your code becomes non-trivial, and even worse, it's timing-dependent so you get "heisenbugs".

There's a long-standing phrase: "DoEvents is evil". That bears careful consideration.

why aren't all the issues linked with DoEvents also linked with await?

That's a good question; a lot of developers were initially skeptical of await because they had been so badly burned by DoEvents. The reason await doesn't fall into this trap is because there's only one message loop. await returns to that singular message loop. There's no nested message loop, and thus no unexpected re-entrancy.

I would never recommend DoEvents as a solution. There are always better solutions.

The order of processing is not preserved. For example I programmatically change the Tab, and expect the event handler to load the tab data, I then want to do something to the loaded data, Currently I set the tab, the event fires - it starts loading tab data and returns immediately, the event handler call returns and I then try to interact with a partially loaded tab. This kind of issue happens anywhere you inadvertently trigger an async event handler

So, here's the actual problem: C# events (at least the void-returning events which are the vast majority of events) are insufficiently powerful to act as anything other than notifications. In design terms, C# events allow you to implement the Observer pattern, but are the wrong choice to implement the Strategy pattern.

To apply to your example, the tabControl1_SelectedIndexChanged is just a notification (Observer pattern) to inform your code that the tab index changed. It's not a hook to provide data loading (Strategy pattern), and attempting to use it that way is what is causing the actual problem here.

The solution then is not to depend on event handlers to drive your logic. Take a look at some of the Model-View-ViewModel (MVVM) concepts out there for some inspiration. Using a ViewModel kind of approach would probably help in this situation. That way your code would not update the tab control at all; instead, it would update the ViewModel, and your VM can do the work asynchronously (with no async void at all) and then your code can update the UI when the work is complete.

A simple approach (with no data binding and no explicit VM) may look something like:

private async void button1_Click(object sender, EventArgs e)
{
  Debug.WriteLine("Started button1_Click");
  var newTab = tabControl1.SelectedIndex == 0 ? tabPage2 : tabPage1;
  ShowLoadingState(newTab);
  tabControl1.SelectedTab = newTab;
  await LoadDataForTab(newTab); 
  Debug.WriteLine("Ended button1_Click");
}

private async Task LoadDataForTab(TabPage tab)
{
  await Task.Delay(1000);
  // load data into tab
}