CODEWITHSUNDEEP
c#task-parallel-libraryasync-await
KOLANICH
KOLANICH

Reputation: 3072

Task.WhenAll have finished BEFORE the Tasks it contains were finished

for (int i = 1; i < servers.Count;i++){
    var server = new SpeedTestServer(servers[i]);
    server.dist = haversine(server);

    if (closestKnownServer.dist - server.dist > distTreshold){
        closestKnownServer = server;
        this.servers.Add(server);
        this.servers.RemoveAt(0);
    }
    else if (Math.Abs(closestKnownServer.dist - server.dist) <= distTreshold){
        this.servers.Add(server);
        //BUG: we need to enable it but it causes hang
        pingTasks.Add(
            Task.Factory.StartNew(async () => {
                await server.ping();
                if (closestKnownServer.latency > server.latency){
                    closestKnownServer = server;
                    this.servers.RemoveAt(0);
                }
            })
        );
    }

}
await Task.WhenAll(pingTasks);
return closestKnownServer;

Look at the code above. We create a List of Tasks and populate it. Then we wait for them all. BUT IT DOESN'T WORK PROPERLY! The task which is generated with WhenAll if finished, but the Tasks it contains are not. This can be visible if you place breakpoints into the lambda and into the last line of the method.

The full code

P.S. I know I'll also need a syncronization, but havent found a built-in library for it in c#.

Upvotes: 1

Views: 666

Answers (1)

Lukazoid
Lukazoid

Reputation: 19426

As commented by Svick, Task.Factory.StartNew has different overloads and functionality to that of Task.Run these differences have been blogged about here: Task.Run vs Task.Factory.StartNew. The result of Task.Factory.StartNew(async () => ... is a Task<Task>, so when calling Task.WhenAll you are only waiting for the outer Task to complete.

At the end of the blog post linked above, the differences as far as async and await are concerned are highlighted. You are left with three solutions for your problem:

Use Task.Run which has implicit async support, so will return a Task for the execution of an async delegate.

pingTasks.Add(
    Task.Run(async () => {
        await server.ping();
        if (closestKnownServer.latency > server.latency){
            closestKnownServer = server;
            this.servers.RemoveAt(0);
        }
    })
);

Use the Unwrap(Task<Task>) extension method which will turn a Task<Task> into a Task, it is designed exactly for this purpose when using the more advanced overloads of Task.Factory.StartNew

pingTasks.Add(
    Task.Factory.StartNew(async () => {
        await server.ping();
        if (closestKnownServer.latency > server.latency){
            closestKnownServer = server;
            this.servers.RemoveAt(0);
        }
    }).Unwrap()
);

Await the result of Task.Factory.StartNew(async () => ...) which will wait for the outer task complete, and result in a Task which is for the execution of the asynchronous delegate.

pingTasks.Add(
    await Task.Factory.StartNew(async () => {
        await server.ping();
        if (closestKnownServer.latency > server.latency){
            closestKnownServer = server;
            this.servers.RemoveAt(0);
        }
    })
);

Out of all of these, for your situation, I would advise the use of Task.Run due to its implicit async support.

Upvotes: 3

Related Questions