Clarify scope of cancellation in Task.Run #4253
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Task.Run accepts a cancellation token, but doesn't provide it to the invoked method. The cancellation token is only used to cancel the action if it has not started running. This edit corrects the following problems: * Summary sentence about cancelation didn't indicate that it only applies before the action starts running. * Summary sentence was inconsistently present across the various Run overloads. * The `cancelationToken` parameter offered inconsistent guidance. * The `cancelationToken` parameter for one overload said it "should" be provided. This is often poor guidance. In systems were the thread utilization is low, the action will start immediately, and there is no point in providing a cancellation token. It is often useful to provide a cancellation token to the action, but that's not what the `cancelationToken` parameter in `Run` does. Further updates to the docs to clarify this may be helpful.
breyed
changed the title
mairaw
reviewed
May 28, 2020
|
@breyed I've unresolved my comments since my suggestions weren't applied. Can you make sure that you click the commit suggestion button? |
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
Co-authored-by: Maira Wenzel <[email protected]>
tarekgh
approved these changes
Aug 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Task.Run accepts a cancellation token, but doesn't provide it to the invoked method. The cancellation token is only used to cancel the action if it has not started running.
This edit corrects the following problems:
cancelationTokenparameter offered inconsistent guidance.cancelationTokenparameter for one overload said it "should" be provided. This is often poor guidance. In systems were the thread utilization is low, the action will start immediately, and there is no point in providing a cancellation token. It is often useful to provide a cancellation token to the action, but that's not what thecancelationTokenparameter inRundoes. Further updates to the docs to clarify this may be helpful.