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 TaskCompletionSource.SetCanceled(CancellationToken) #30862

Closed
Frassle opened this issue Sep 15, 2019 · 4 comments
Closed

Add TaskCompletionSource.SetCanceled(CancellationToken) #30862

Frassle opened this issue Sep 15, 2019 · 4 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Tasks
Milestone

Comments

@Frassle
Copy link
Contributor

Frassle commented Sep 15, 2019

TaskCompletionSource currently has three methods to set it to canceled. void SetCanceled() which throws if the source is already completed, bool TrySetCanceled() which just returns false if the source is already cancelled, and bool TrySetCanceled(CancellationToken) which is like TrySetCanceled but also allows linking in a CancellationToken.

Can we also add void SetCanceled(CancellationToken) to use in cases where you want to make it clear that this should set the canceled state but you also want to link a CancellationToken in.
This is doable by writing your own that just throws if TrySetCanceled returns false but this can't easily get the TaskT_TransitionToFinal_AlreadyCompleted resource string to match the error message thrown by corefx. It also leads to people just writing TrySetCanceled and ignoring the result, which leads to less clear code and potentially hides bugs, or alternatively they write SetCanceled and throw away the token information (Me trying to fix one case of this in FSharp.Core is what lead to me raising this issue).

public class TaskCompletionSource<TResult>
{
    ...
    public void SetCanceled(CancellationToken cancellationToken);
    ...
}
@tarekgh
Copy link
Member

tarekgh commented Sep 15, 2019

CC @stephentoub

@stephentoub
Copy link
Member

This is reasonable.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review labels Feb 20, 2020
@terrajobst
Copy link
Member

terrajobst commented Feb 20, 2020

Video

  • Looks good as proposed and follows the guidelines for Try-APIs
public class TaskCompletionSource<TResult>
{
    ...
    public void SetCanceled(CancellationToken cancellationToken);
    ...
}

Frassle added a commit to Frassle/runtime that referenced this issue Feb 22, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
stephentoub pushed a commit that referenced this issue Feb 25, 2020
* Add TaskCompletionSource.SetCanceled(CancellationToken)

api-approved by #30862

* Add to ref

* SetCanceled(default)

* Change some tests to use SetCaneled not TrySetCanceled

These tests used SetResult/SetException and TrySetCancelled so it could
pass a token. Changed to use SetCancelled to match the Result/Exception
useage.

* Add SetCanceled(CT) test

* Check exception on re-cancel

* Equal not Equals

* Catch aggregate not TaskCancelled directly

*  Inner not exc

* Markup

* s/m/n
@stephentoub
Copy link
Member

Fixed by #32696

@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Feb 25, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading.Tasks
Projects
None yet
Development

No branches or pull requests

6 participants