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 CancellationTokenRegistration.Token property #23461

Closed
stephentoub opened this issue Sep 6, 2017 · 4 comments
Closed

Add CancellationTokenRegistration.Token property #23461

stephentoub opened this issue Sep 6, 2017 · 4 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Threading
Milestone

Comments

@stephentoub
Copy link
Member

When registering with a CancellationToken, a common pattern is to then store the resulting CancellationTokenRegistration onto some object for later disposal when the associated asynchronous operation completes. It's also common to store the original CancellationToken, so that it can, for example, be passed to TaskCompletionSource.TrySetCanceled. But having to store the CancellationToken in addition to the CancellationTokenRegistration should be unnecessary, as the registration already knows with which CancellationToken it's associated... it just doesn't expose that information. We should expose it.

public struct CancellationTokenRegistration
{
    public CancellationToken Token { get; }
    ...
}

This property now exists, it's just internal:
https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Threading/CancellationTokenRegistration.cs#L44
We should simply make it public.

This will allow us to shrink by a reference-sized field several objects that store the CT in addition to the CTR.

@tarekgh
Copy link
Member

tarekgh commented Sep 6, 2017

@stephentoub we have not same but related issue https://github.com/dotnet/corefx/issues/23716. do you think your proposal will affect that issue? or you think we can combine the 2 proposals if we are going to design review it?

@stephentoub
Copy link
Member Author

do you think your proposal will affect that issue?

I believe they are unrelated in both design and implementation. This is purely about exposing from CancellationTokenRegistration information we already have in order to not have to carry that information around separately. The other proposal you mention is about adding new functionality to CancellationTokenSource, such that the implementation can queue the invocation of any registered handlers if any exist.

@terrajobst
Copy link
Member

terrajobst commented Oct 3, 2017

Video

Approved as proposed.

@stephentoub stephentoub self-assigned this Oct 3, 2017
@karelz
Copy link
Member

karelz commented Oct 4, 2017

FYI: The API review discussion was recorded - see https://youtu.be/m4BUM3nJZRw?t=4706 (2 min duration)

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 20, 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
Projects
None yet
Development

No branches or pull requests

5 participants