c# - Why is calling await completing the parent Task prematurely?

Question

I'm trying to create a control that exposes a DoLoading event that consumers can subscribe to in order to perform loading operations. For convenience, event handlers should be called from the UI thread allowing consumers to update the UI at will, but they will also be able to use async/await to perform long-running tasks without blocking the UI thread.

For this, I have declared the following delegate:

public delegate Task AsyncEventHandler<TEventArgs>(object sender, TEventArgs e);

That allows consumers to subscribe to the event:

public event AsyncEventHandler<bool> DoLoading;

The idea is that consumers will subscribe to the event as so (this line is executed in the UI thread):

loader.DoLoading += async (s, e) =>
            {
                for (var i = 5; i > 0; i--)
                {
                    loader.Text = i.ToString(); // UI update
                    await Task.Delay(1000); // long-running task doesn't block UI
                }
            };

At an appropriate point in time, I'm getting a TaskScheduler for the UI thread and storing it in _uiScheduler.

The event is triggered when appropriate by the loader with the following line (this happens in a random thread):

this.PerformLoadingActionAsync().ContinueWith(
            _ =>
            {
                // Other operations that must happen on UI thread
            },
            _uiScheduler);

Notice that this line is not called from the UI thread but needs to update the UI when loading is completed, so I'm using ContinueWith to execute code on the UI task scheduler when the loading task completes.

I've tried several variations of the following methods, none of which have worked, so here's where I'm at:

private async Task<Task> PerformLoadingActionAsync()
{
    TaskFactory uiFactory = new TaskFactory(_uiScheduler);

    // Trigger event on the UI thread and await its execution
    Task evenHandlerTask = await uiFactory.StartNew(async () => await this.OnDoLoading(_mustLoadPreviousRunningState));

    // This can be ignored for now as it completes immediately
    Task commandTask = Task.Run(() => this.ExecuteCommand());

    return Task.WhenAll(evenHandlerTask, commandTask);
}

private async Task OnDoLoading(bool mustLoadPreviousRunningState)
{
    var handler = this.DoLoading;

    if (handler != null)
    {
        await handler(this, mustLoadPreviousRunningState);
    }
}

As you can see, I'm starting two tasks and expect my ContinueWith from before to execute one all of them complete.

The commandTask completes immediately, so it can be ignored for the moment. The eventHandlerTask, as I see it, should only complete one the event handler completes, given that I'm awaiting the call to the method that calls the event handler and I'm awaiting the event handler itself.

However, what's actually happening, is that the tasks are being completed as soon as the line await Task.Delay(1000) in my event handler is executed.

Why is this and how can I get the behaviour I expect?

Answer 1

You correctly realized that StartNew() returns Task<Task> in this case, and you care about the inner Task (though I'm not sure why are you waiting for the outer Task before starting commandTask).

But then you return Task<Task> and ignore the inner Task. What you should do is to use await instead of return and change the return type of PerformLoadingActionAsync() to just Task:

await Task.WhenAll(evenHandlerTask, commandTask);

Few more notes:

1. Using event handlers this way is quite dangerous, because you care about the Task returned from the handler, but if there are more handlers, only the last Task will be returned if you raise the event normally. If you really want to do this, you should call GetInvocationList(), which lets you invoke and await each handler separately:

   private async Task OnDoLoading(bool mustLoadPreviousRunningState)
   {
       var handler = this.DoLoading;
   
       if (handler != null)
       {
           var handlers = handler.GetInvocationList();
   
           foreach (AsyncEventHandler<bool> innerHandler in handlers)
           {
               await innerHandler(this, mustLoadPreviousRunningState);
           }
       }
   }
   

   If you know that you'll never have more than one handler, you could use a delegate property that can be directly set instead of an event.

2. If you have an async method or lambda that has the only await just before its return (and no finallys), then you don't need to make it async, just return the Task directly:

   Task.Factory.StartNew(() => this.OnDoLoading(true))