Hon'yuan Lim
Hon'yuan Lim

Reputation: 85

Why Task.WhenAll(taskList) doesn't work?

If the task in foreach isn't async, and make ping.Send instead of ping.SendPingAsync then Task.WhenAll(taskList) will work.

List<Task> taskList = new List<Task>();   

foreach (var host in hostArray)
{
    var aHost = host;
    Task task = new Task(async ()=>        
    {
        Ping ping = new Ping();
        PingResult pingRes = new PingResult { HostNameOrAddress = aHost };
        for (int i = 0; i < _pingCount; i++)
        {
            try
            {
                PingReply reply = await ping.SendPingAsync(aHost,1000);
                if (reply.Status == IPStatus.Success)
                    pingRes.RoundtripTimes.Add(reply.RoundtripTime);
            }
            catch
            {
                // ignored
            }
        }

        Dispatcher.Invoke(() =>
        {
            _pingResultList.Add(pingRes);                                
        });
    });

    taskList.Add(task);
    task.Start();
}

await Task.WhenAll(taskList); //Never wait.

Upvotes: 2

Views: 5300

Answers (2)

Douglas
Douglas

Reputation: 54877

When your anonymous method is asynchronous, you should use the Task.Run method instead of the Task constructor.

foreach (var host in hostArray)
{
    Task task = Task.Run(async () =>
    {
        // ...
    });

    taskList.Add(task);
}

await Task.WhenAll(taskList);

The C# compiler converts asynchronous methods to a Func<Task> delegate. Consequently, when you run them through a task, the outer task would actually be a Task<Task> – namely, a task that generates another task when executed.

The Task constructor (like the Task.Factory.StartNew method) is impervious to the fact that your anonymous method itself returns a task that needs to be awaited upon; instead, it just waits on the outer task, which completes as soon as your anonymous method reaches its end.

Task.Run, on the other hand, has overloads that can handle such Func<Task> delegates by unwrapping them and waiting on their inner task. Thus, it will wait on the inner task representing the completion of the SendPingAsync call.

Upvotes: 6

Sriram Sakthivel
Sriram Sakthivel

Reputation: 73442

Don't use Task's constructor. It doesn't support asynchronous lambda. Instead use Task.Run.

Your code at the moment waits only till your SendPingAsync is called in your tasks, not for the whole asynchronous method to finish. Because as soon as first await point hit, asynchronous methods return.

Your async lambda compiles to Task(Action) which means it gets compiled as async void method. Since there is no easy way to wait for async void methods, Task class constructor doesn't support it. Conversely Task.Run supports asynchronous method via Task.Run(Func<Task>) overload. Since this returns a Task, Run method can easily wait for its completion.

Not only with asynchronous methods, always prefer Task.Run or Task.Factory.StartNew(if you need more control). There is almost no reason for one to use Task constructor.

If you decide to use StartNew for the asynchronous methods, you'll have to call Task.UnWrap and wait for the task returned by it. When you're using Task.Run, you get this for free.

Upvotes: 9

Related Questions