11

I am trying to use Task.WhenAll to await completion of multiple tasks.

My code is below - it is supposed to launch multiple async tasks, each of which retrieves a bus route and then adds them to a local array. However, Task.WhenAll(...) returns immediately, and the count of the local routes array is zero. This seems strange, since I would expect the various await statements within each Task to mean that the flow is suspended, and the Task does not return until it's finished.

List<Task> monitoredTasks = new List<Task>();
foreach (BusRouteIdentifier bri in stop.services)
{
    BusRouteRequest req = new BusRouteRequest(bri.id);

    // Start a new task to fetch the route for each stop
    Task getRouteTask = Task.Factory.StartNew(async () =>
    {
        var route = await BusDataProviderManager.DataProvider.DataBroker.getRoute(req);

            // Add the route to our array (on UI thread as it's observed)
            await dispatcher.RunAsync(Windows.UI.Core.CoreDispatcherPriority.Normal, delegate
            {
                this.routes.Add(route);
            });
    });

    // Store the task in our monitoring list
    monitoredTasks .Add(getRouteTask);
}

Debug.WriteLine("Awaiting WHENALL");
await Task.WhenAll(monitoredTasks );
Debug.WriteLine(string.Format("WHENALL returned (routes count is {0} ", this.routes.Count));

this.OnWillEndFetchingRoutes(new EventArgs());

I'm obviously doing something wrong - but what?

Boiethios
  • 38,438
  • 19
  • 134
  • 183
Carlos P
  • 3,928
  • 2
  • 34
  • 50
  • have you tried to check whether the final `Task` is not in the faulted state? – ie. Aug 14 '12 at 10:33
  • I think it could be something wrong `await dispatcher` inside foreach loop. UI thread will observed and display immediately ? – cat916 Aug 14 '12 at 10:34
  • @ie. Yes, its state is RanToCompletion. The state of all the Tasks in the array are RanToCompletion too, although when I actually inspect them individually, the Result field of each one is WaitingForActivation – Carlos P Aug 14 '12 at 10:46
  • @user861114 Not sure what you mean, the UI will observe and display, but shouldn't the 'await' keyword mean that execution doesn't return until the method has finished? – Carlos P Aug 14 '12 at 10:48

3 Answers3

9

This was down to a basic lack of understanding of how async-await really works.

The inner task was returning flow to the outer task, which then finished before the await ever returned.

To achieve what I wanted, I needed to refactor as follows:

List<Task<BusRoute>> routeRetrievalTasks = new List<Task<BusRoute>>();
foreach (BusRouteIdentifier bri in stop.services)
{
    BusRouteRequest req = new BusRouteRequest(bri.id);
    routeRetrievalTasks.Add(BusDataProviderManager.DataProvider.DataBroker.getRoute(req));
}

foreach (var task in routeRetrievalTasks)
{
    var route = await task;
    this.routes.Add(route); // triggers events
}

Thanks to Dave Smits

Boiethios
  • 38,438
  • 19
  • 134
  • 183
Carlos P
  • 3,928
  • 2
  • 34
  • 50
6

I suspect the problem is your call to Task.Factory.StartNew(). I suspect you're ending up with a Task<Task>, and you're only finding out when it's effectively started the task.

Try this instead:

Func<Task> taskFunc = async () =>
{
    var route = await BusDataProviderManager.DataProvider.DataBroker.getRoute(req);

    // Add the route to our array (on UI thread as it's observed)
    await dispatcher.RunAsync(Windows.UI.Core.CoreDispatcherPriority.Normal, delegate
    {
        this.routes.Add(route);
    });

}

Task getRouteTask = Task.Run(taskFunc);
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Thanks. I assume that the rest of my method would continue as before, i.e. add the Task to the array of tasks, then call Task.WhenAll(...) on the array. In which case, how does this approach compare to the one in my Answer? Are there any advantages? – Carlos P Aug 14 '12 at 11:05
  • @CarlosP: To be honest I don't really have enough context to comment on which is a better approach. But yes, the rest of your method would continue as before. – Jon Skeet Aug 14 '12 at 11:13
-2

So the answer to "how to use WhenAll() correctly ?" is actually : "Don't and just successively await your tasks one after an other" ? :D

Your solution works but optimization wise it is suboptimal. You are executing all your process, await for the first, then continue your loop. Other process continues in the same time, most of them have probably end at this point, but you will continue to loop await after await instead of awaiting all of them at the same time

Task shall be awaited has late as possible : Async and await

List<Task> monitoredTasks = new List<Task>();
        foreach (BusRouteIdentifier bri in stop.services)
        {
            BusRouteRequest req = new BusRouteRequest(bri.id);

            // Start a new task to fetch the route for each stop
            var route = await BusDataProviderManager.DataProvider.DataBroker.getRoute(req);

            // Add the route to our array (on UI thread as it's observed) 
            //BUT no point using a factory here, it would only create a Task<Task> while dispatcher.RunAsync is already returning you the Task you need
            // Do not await if you don't need to!!!
            Task getRouteTask = dispatcher.RunAsync(Windows.UI.Core.CoreDispatcherPriority.Normal, delegate
            {
                this.routes.Add(route);
            });

            // Store the task in our monitoring list
            monitoredTasks.Add(getRouteTask);
        }

        Debug.WriteLine("Awaiting WHENALL");
        // And now Task.WhenAll is apply directly on your tasks and not on a list of Task<Task> returning your tasks 
        await Task.WhenAll(monitoredTasks);
        Debug.WriteLine(string.Format("WHENALL returned (routes count is {0} ", this.routes.Count));

        this.OnWillEndFetchingRoutes(new EventArgs());
Ashijo
  • 90
  • 8