Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Task.WithCancellation(CancellationToken) method #27722

Closed
AArnott opened this issue Oct 24, 2018 · 8 comments · Fixed by #48842
Closed

Add Task.WithCancellation(CancellationToken) method #27722

AArnott opened this issue Oct 24, 2018 · 8 comments · Fixed by #48842
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks
Milestone

Comments

@AArnott
Copy link
Contributor

AArnott commented Oct 24, 2018

The WithCancellation extension method is super-useful. Should we add it to the .NET Framework / .NET Standard itself?

The risk is that folks will misunderstand the subtle yet important difference between using this method and passing the CancellationToken into the original async method properly.

Preferably this is not an extension method so it doesn't conflict with extension methods that folks have already defined. It should just override them, assuming the semantics are equivalent (the type of exception thrown, etc.)

See also the source code for its definition in the vs-threading library.

As discussed in davidfowl/AspNetCoreDiagnosticScenarios#8 with @davidfowl.

@benaadams
Copy link
Member

Similar code is already in corefx; but only used for running tests rather than exposed as api

Task WithCancellation(this Task task, CancellationToken cancellationToken)

https://github.com/dotnet/corefx/blob/407c09d5840384fbd24bbd5b8869ef13be71de63/src/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs#L15

Also

Task WhenAllOrAnyFailed(this Task[] tasks)
Task WhenAllOrAnyFailed(this Task[] tasks, int millisecondsTimeout)

@Wraith2
Copy link
Contributor

Wraith2 commented Oct 25, 2018

The risk is that folks will misunderstand the subtle yet important difference between using this method and passing the CancellationToken into the original async method properly.

for those not good at async, what difference?

@benaadams
Copy link
Member

for those not good at async, what difference?

Cancelling the CancellationToken passed into the method cancels the Task

Cancelling the CancellationToken passed into the WithCancellation cancels waiting for the Task (but not the Task)

@omariom
Copy link
Contributor

omariom commented Oct 25, 2018

The risk is that folks will misunderstand the subtle yet important difference between using this method and passing the CancellationToken into the original async method properly.

What would be a good name for that? WithWaitingCancellation?

@jnm2
Copy link
Contributor

jnm2 commented Oct 25, 2018

ConfigureAwait(CancellationToken) :trollface:

@AArnott
Copy link
Contributor Author

AArnott commented Oct 25, 2018

ConfigureAwait(CancellationToken)

That's actually an interesting idea. The upside is I think people would understand it better. The downside is that Task is applicable in more scenarios than just awaiting, so if you're not awaiting and you want a Task that completes with early cancellation, you're not likely to look at a ConfigureAwait overload for that functionality.

@tarekgh
Copy link
Member

tarekgh commented Jan 17, 2019

CC @stephentoub

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Feb 25, 2020
@stephentoub stephentoub modified the milestones: 5.0, Future Feb 25, 2020
@StephenCleary
Copy link
Contributor

I really prefer the Task.WaitAsync(CancellationToken) naming over WithCancellation. I think WaitAsync makes it much more clear that it is the wait that is canceled and not the task.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 26, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 11, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants