dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.06k stars 4.69k forks source link

[API Proposal]: Task.WhenAll(... CancellationToken) #67392

Open TonyValenti opened 2 years ago

TonyValenti commented 2 years ago

Background and motivation

Currently there is no way to cancel a Task.WhenAll(..). It would be great if the current overloads also accepted a cancellation token.

API Proposal

namespace System.Treading.Tasks
{
    public class Task 
    {
        public Task WhenAll(IEnumerable<Task> tasks, CancellationToken token);
        public Task WhenAll<TResult>(IEnumerable<Task<TResult>> tasks, CancellationToken token);
    }
}

API Usage

// Fancy the value
var T1 = Task.Run(...);
var T2 = Task.Run(...);

await Task.WhenAll(new[]{T1, T2}, token);

Alternative Designs

None that I can think of.

Risks

None

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/area-system-threading-tasks See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation Currently there is no way to cancel a Task.WhenAll(..). It would be great if the current overloads also accepted a cancellation token. ### API Proposal ```C# namespace System.Treading.Tasks { public class Task { public Task WhenAll(IEnumerable tasks, CancellationToken token); public Task WhenAll(IEnumerable> tasks, CancellationToken token); } } ``` ### API Usage ```C# // Fancy the value var T1 = Task.Run(...); var T2 = Task.Run(...); await Task.WhenAll(new[]{T1, T2}, token); ``` ### Alternative Designs None that I can think of. ### Risks None
Author: TonyValenti
Assignees: -
Labels: `api-suggestion`, `area-System.Threading.Tasks`, `untriaged`
Milestone: -
theodorzoulias commented 2 years ago

Isn't this scenario covered by the Task.WaitAsync method?

await Task.WhenAll(T1, T2).WaitAsync(token);
deeprobin commented 2 years ago

@theodorzoulias I think so.

But should we stay consistent with Task.WaitAll and Task.WaitAny? Because these accept a CancellationToken.

I'm currently just not sure for what reason this makes sense implementation-wise. Yes we could check for cancellation in TaskFactory.CommonCWAnyLogic but is that good? I don't know.

theodorzoulias commented 2 years ago

@deeprobin a disadvantage of using the Task.WhenAll+WaitAsync combination is that the continuations attached to the tasks are (most probably) not detached when the token is canceled. So if it's called repeatedly in a loop, and the tasks are long running, a serious memory leak might emerge.

Hard to imagine a scenario where someone would need to do something like this though.

MihaZupan commented 2 years ago

a disadvantage of using the Task.WhenAll+WaitAsync combination is that the continuations attached to the tasks are (most probably) not detached when the token is canceled

How would the behavior of Task.WhenAll(tasks, CT) overload differ here? CancellationToken uses a cooperative approach, so the work the Task represents would have to react to the cancellation token and stop itself. Task.WhenAll can't force an arbitrary task to abort.

theodorzoulias commented 2 years ago

How would the behavior of Task.WhenAll(tasks, CT) overload differ here?

@MihaZupan absolutely no behavioral difference. the Task.WhenAll(tasks, token) would behave exactly the same with the Task.WhenAll(tasks).WaitAsync(token). The only difference would be in the memory consumption after a canceled await. The GC.GetTotalMemory(true) would return a smaller number.

stephentoub commented 2 years ago

the continuations attached to the tasks are (most probably) not detached when the token is canceled

They're not. They can't be. If they were, it would break a use like:

Task t = Task.WhenAll(t1, t2);
while (true)
{
    try
    {
        await t.WaitAsync(pollInterval);
        break;
    }
    catch {}

    ... // do something 
}

as the continuations the WhenAll hooked up to t1 and t2 would then be removed when the first WaitAsync fails, after which point t would never complete.