Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Refactor how we run tasks synchronously so that TPL tasks aren't broken #814

Merged

Conversation

shana
Copy link
Member

@shana shana commented Jun 1, 2018

Our task system uses TPL tasks (System.Threading.Tasks.Task) under the hood (every ITask really is a TPL task whose body is the callback or Run/RunWithReturn/RunWithData method override), so when we run one of our ITask, what we're really doing is scheduling the underlying TPL task to be executed. If we want to run the ITask synchronously, it's straightforward to just execute the Run/RunWithReturn/RunWithData method that does the actual work.

Because of this, we also have an overload to pass in an existing TPL task into an ITask, so that we can also run async/await task types in our threading system (so that we can control the thread these tasks are going to be run in, given that async/await has no threading control model).

The problem with allowing a TPL task to be set directly into an ITask is that the TPL task wasn't set up in a way that exposed a Run/RunWithReturn/RunWithData method that could be called synchronously.This PR moves things around so that instead of calling directly the Run/RunWithReturn/RunWithData methods, there is one RunSynchronously method that does the right thing for all task types, regardless of how they're initialized. This has the added benefit that the exact same method (RunSynchronously) is executed independent of who's calling it - if the ITask is running on the scheduler, RunSynchronously is the task body that the scheduler will execute, and if the user wants to run it in thread, they can call it directly.

@shana shana force-pushed the fixes/fix-taskqueue-some-more branch 4 times, most recently from 3b8500e to cb8edf5 Compare June 1, 2018 10:10
Our task system uses TPL tasks (System.Threading.Tasks.Task) under the hood
(every ITask really is a TPL task whose body is the callback or
Run/RunWithReturn/RunWithData method override), so when we run one of our
ITask, what we're really doing is scheduling the underlying TPL task to be
executed. If we want to run the ITask synchronously, it's straightforward to
just execute the Run/RunWithReturn/RunWithData method that does the actual work.

Because of this, we also have an overload to pass in an existing TPL task into
an ITask, so that we can also run async/await task types in our threading system
(so that we can control the thread these tasks are going to be run in, given that
async/await has no threading control model).

The problem with allowing a TPL task to be set directly into an ITask is that the
TPL task wasn't set up in a way that exposed a Run/RunWithReturn/RunWithData
method that could be called synchronously.This PR moves things around so that
instead of calling directly the Run/RunWithReturn/RunWithData methods, there is one
RunSynchronously method that does the right thing for all task types, regardless of
how they're initialized.

This has the added benefit that the exact same method (`RunSynchronously`) is executed
independent of who's calling it
  - if the ITask is running on the scheduler, `RunSynchronously` is the task body that
    the scheduler will execute
  - if the user wants to run it in thread, they can call it directly.
@shana shana force-pushed the fixes/fix-taskqueue-some-more branch from cb8edf5 to c439795 Compare June 1, 2018 13:03
@StanleyGoldman StanleyGoldman merged commit 38bbe98 into fixes/lock-unlock-multiple-files Jun 1, 2018
@StanleyGoldman StanleyGoldman deleted the fixes/fix-taskqueue-some-more branch June 1, 2018 13:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants