Use Task.Run to start OperationQueue. #407
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
Use `Task.Run` to start `OperationQueue`'s processing task rather than using the constructor and then calling `task.Start`. This ensures two things: 1. The `TaskScheduler.Default` scheduler is used, which will be a threadpool scheduler 2. That `task` tracks the running of the entire task: previously the task will have completed when it `await`ed as the `Task` constructor accepts an `Action` and not a `Func<Task>` Fixes reactiveui#406
grokys
force-pushed
the
fixes/406-operationqueue-scheduler
branch
from
c4184e8 to
8727243
Compare
Member
Collaborator
|
On another note appveyor was broken a few months ago which this PR took care of with forcing the version of Cake |
grokys
added a commit
to github/VisualStudio
that referenced
this pull request
May 16, 2018
Beacuse editor-tools/Akavache#1 was updated to cherry pick reactiveui/Akavache#407.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix
What is the current behavior? (You can also link to an open issue here)
See #406
What is the new behavior (if this is a feature change)?
Use
Task.Runto startOperationQueue's processing task rather than using the constructor and then callingtask.Start. This ensures two things:TaskScheduler.Defaultscheduler is used, which will be a threadpool schedulertasktracks the running of the entire task: previously the task will have completed when itawaited as theTaskconstructor accepts anActionand not aFunc<Task>Note that
TaskCreationOptions.LongRunningis no longer being passed in, but according to https://blog.stephencleary.com/2013/08/startnew-is-dangerous.html that parameter is just an "optimization hint" and the the threadpool should adjust to the long-running task automatically.Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
I'm not quite sure what a test/docs would look like for this?