Task Execution with Cancel Logic
up vote
1
down vote
favorite
In my assignment I am executing a long running NodeJs script from C# using EdgeJs. Since the script may take a long time to execute, I also want to provide user an option to Cancel the script execution in the middle. The cancellation status is maintained in the database and function IsScripteExecutionCancelled returns true if the user decides to cancel the execution.
To achieve this I am executing script in a background thread using Task.Run(). The main thread contains a while loop that runs until the ScriptExecution is over or is cancelled. I can solve this problem using native System.Threading.Thread, but here I am trying to do it using TPL. Here is the code I have put in place:
bool executionStarted = false;
bool taskCompleted = false;
Result returnValue;
CancellationTokenSource tokenSource = new CancellationTokenSource();
CancellationToken token = tokenSource.Token;
Task executionTask;
while (!(taskCompleted || token.IsCancellationRequested))
{
if (!executionStarted)
{
executionStarted = true;
executionTask = Task.Run(() =>
{
returnValue = ExecuteScript();
taskCompleted = true;
}, token);
}
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
}
if(token.IsCancellationRequested)
returnValue= GetDefaultValueForCancellation();
return returnValue;
So, if IsScripteExecutionCancelled returns true the program will come out of the loop and will check if the script was cancelled, in which case I will return a default response. If the ExecuteScript completes my program will return the value returned by function ExecuteScript. I am able to get the desired results using this implementation however I would like to get it validated.
Question: Is it the right way to address this problem?
c# task-parallel-library
bumped to the homepage by Community♦ 10 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
add a comment |
up vote
1
down vote
favorite
In my assignment I am executing a long running NodeJs script from C# using EdgeJs. Since the script may take a long time to execute, I also want to provide user an option to Cancel the script execution in the middle. The cancellation status is maintained in the database and function IsScripteExecutionCancelled returns true if the user decides to cancel the execution.
To achieve this I am executing script in a background thread using Task.Run(). The main thread contains a while loop that runs until the ScriptExecution is over or is cancelled. I can solve this problem using native System.Threading.Thread, but here I am trying to do it using TPL. Here is the code I have put in place:
bool executionStarted = false;
bool taskCompleted = false;
Result returnValue;
CancellationTokenSource tokenSource = new CancellationTokenSource();
CancellationToken token = tokenSource.Token;
Task executionTask;
while (!(taskCompleted || token.IsCancellationRequested))
{
if (!executionStarted)
{
executionStarted = true;
executionTask = Task.Run(() =>
{
returnValue = ExecuteScript();
taskCompleted = true;
}, token);
}
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
}
if(token.IsCancellationRequested)
returnValue= GetDefaultValueForCancellation();
return returnValue;
So, if IsScripteExecutionCancelled returns true the program will come out of the loop and will check if the script was cancelled, in which case I will return a default response. If the ExecuteScript completes my program will return the value returned by function ExecuteScript. I am able to get the desired results using this implementation however I would like to get it validated.
Question: Is it the right way to address this problem?
c# task-parallel-library
bumped to the homepage by Community♦ 10 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
1
You are going to have to provide more context. It is unclear what this code is suppose to be doing.
– Nkosi
Mar 13 at 11:33
This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does
– Dannnno
Mar 13 at 13:57
add a comment |
up vote
1
down vote
favorite
up vote
1
down vote
favorite
In my assignment I am executing a long running NodeJs script from C# using EdgeJs. Since the script may take a long time to execute, I also want to provide user an option to Cancel the script execution in the middle. The cancellation status is maintained in the database and function IsScripteExecutionCancelled returns true if the user decides to cancel the execution.
To achieve this I am executing script in a background thread using Task.Run(). The main thread contains a while loop that runs until the ScriptExecution is over or is cancelled. I can solve this problem using native System.Threading.Thread, but here I am trying to do it using TPL. Here is the code I have put in place:
bool executionStarted = false;
bool taskCompleted = false;
Result returnValue;
CancellationTokenSource tokenSource = new CancellationTokenSource();
CancellationToken token = tokenSource.Token;
Task executionTask;
while (!(taskCompleted || token.IsCancellationRequested))
{
if (!executionStarted)
{
executionStarted = true;
executionTask = Task.Run(() =>
{
returnValue = ExecuteScript();
taskCompleted = true;
}, token);
}
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
}
if(token.IsCancellationRequested)
returnValue= GetDefaultValueForCancellation();
return returnValue;
So, if IsScripteExecutionCancelled returns true the program will come out of the loop and will check if the script was cancelled, in which case I will return a default response. If the ExecuteScript completes my program will return the value returned by function ExecuteScript. I am able to get the desired results using this implementation however I would like to get it validated.
Question: Is it the right way to address this problem?
c# task-parallel-library
In my assignment I am executing a long running NodeJs script from C# using EdgeJs. Since the script may take a long time to execute, I also want to provide user an option to Cancel the script execution in the middle. The cancellation status is maintained in the database and function IsScripteExecutionCancelled returns true if the user decides to cancel the execution.
To achieve this I am executing script in a background thread using Task.Run(). The main thread contains a while loop that runs until the ScriptExecution is over or is cancelled. I can solve this problem using native System.Threading.Thread, but here I am trying to do it using TPL. Here is the code I have put in place:
bool executionStarted = false;
bool taskCompleted = false;
Result returnValue;
CancellationTokenSource tokenSource = new CancellationTokenSource();
CancellationToken token = tokenSource.Token;
Task executionTask;
while (!(taskCompleted || token.IsCancellationRequested))
{
if (!executionStarted)
{
executionStarted = true;
executionTask = Task.Run(() =>
{
returnValue = ExecuteScript();
taskCompleted = true;
}, token);
}
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
}
if(token.IsCancellationRequested)
returnValue= GetDefaultValueForCancellation();
return returnValue;
So, if IsScripteExecutionCancelled returns true the program will come out of the loop and will check if the script was cancelled, in which case I will return a default response. If the ExecuteScript completes my program will return the value returned by function ExecuteScript. I am able to get the desired results using this implementation however I would like to get it validated.
Question: Is it the right way to address this problem?
c# task-parallel-library
c# task-parallel-library
edited Mar 14 at 16:48
Sᴀᴍ Onᴇᴌᴀ
8,07461751
8,07461751
asked Mar 13 at 10:33
Aashish
1394
1394
bumped to the homepage by Community♦ 10 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
bumped to the homepage by Community♦ 10 mins ago
This question has answers that may be good or bad; the system has marked it active so that they can be reviewed.
1
You are going to have to provide more context. It is unclear what this code is suppose to be doing.
– Nkosi
Mar 13 at 11:33
This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does
– Dannnno
Mar 13 at 13:57
add a comment |
1
You are going to have to provide more context. It is unclear what this code is suppose to be doing.
– Nkosi
Mar 13 at 11:33
This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does
– Dannnno
Mar 13 at 13:57
1
1
You are going to have to provide more context. It is unclear what this code is suppose to be doing.
– Nkosi
Mar 13 at 11:33
You are going to have to provide more context. It is unclear what this code is suppose to be doing.
– Nkosi
Mar 13 at 11:33
This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does
– Dannnno
Mar 13 at 13:57
This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does
– Dannnno
Mar 13 at 13:57
add a comment |
2 Answers
2
active
oldest
votes
up vote
0
down vote
Is there a better way of doing Task cancellation than the one I implemented
There's no task cancellation implemented here. By passing a token to Task.Run you just make the returned task IsCanceled equal to true in case it cancels itself (without, it gets IsFaulted by the OperationCanceledException). But you need (at least) to pass the token to DoSomeWork, because that has to check the token and cancel itself.
Task cancellation works through cooperation of the to-be-canceled task and has nothing to do with Thread.Abort.
add a comment |
up vote
0
down vote
you create a variable here and use it only once
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
I would just use the method in the if statement, like
if (IsScripteExecutionCancelled())
{
tokenSource.Cancel();
}
if you were going to use this value somewhere else and didn't want it evaluated again, then you could assign it to a variable, but then you need to keep in mind that calling the variable will not run the method.
add a comment |
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
0
down vote
Is there a better way of doing Task cancellation than the one I implemented
There's no task cancellation implemented here. By passing a token to Task.Run you just make the returned task IsCanceled equal to true in case it cancels itself (without, it gets IsFaulted by the OperationCanceledException). But you need (at least) to pass the token to DoSomeWork, because that has to check the token and cancel itself.
Task cancellation works through cooperation of the to-be-canceled task and has nothing to do with Thread.Abort.
add a comment |
up vote
0
down vote
Is there a better way of doing Task cancellation than the one I implemented
There's no task cancellation implemented here. By passing a token to Task.Run you just make the returned task IsCanceled equal to true in case it cancels itself (without, it gets IsFaulted by the OperationCanceledException). But you need (at least) to pass the token to DoSomeWork, because that has to check the token and cancel itself.
Task cancellation works through cooperation of the to-be-canceled task and has nothing to do with Thread.Abort.
add a comment |
up vote
0
down vote
up vote
0
down vote
Is there a better way of doing Task cancellation than the one I implemented
There's no task cancellation implemented here. By passing a token to Task.Run you just make the returned task IsCanceled equal to true in case it cancels itself (without, it gets IsFaulted by the OperationCanceledException). But you need (at least) to pass the token to DoSomeWork, because that has to check the token and cancel itself.
Task cancellation works through cooperation of the to-be-canceled task and has nothing to do with Thread.Abort.
Is there a better way of doing Task cancellation than the one I implemented
There's no task cancellation implemented here. By passing a token to Task.Run you just make the returned task IsCanceled equal to true in case it cancels itself (without, it gets IsFaulted by the OperationCanceledException). But you need (at least) to pass the token to DoSomeWork, because that has to check the token and cancel itself.
Task cancellation works through cooperation of the to-be-canceled task and has nothing to do with Thread.Abort.
answered Mar 13 at 15:36
Haukinger
1214
1214
add a comment |
add a comment |
up vote
0
down vote
you create a variable here and use it only once
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
I would just use the method in the if statement, like
if (IsScripteExecutionCancelled())
{
tokenSource.Cancel();
}
if you were going to use this value somewhere else and didn't want it evaluated again, then you could assign it to a variable, but then you need to keep in mind that calling the variable will not run the method.
add a comment |
up vote
0
down vote
you create a variable here and use it only once
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
I would just use the method in the if statement, like
if (IsScripteExecutionCancelled())
{
tokenSource.Cancel();
}
if you were going to use this value somewhere else and didn't want it evaluated again, then you could assign it to a variable, but then you need to keep in mind that calling the variable will not run the method.
add a comment |
up vote
0
down vote
up vote
0
down vote
you create a variable here and use it only once
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
I would just use the method in the if statement, like
if (IsScripteExecutionCancelled())
{
tokenSource.Cancel();
}
if you were going to use this value somewhere else and didn't want it evaluated again, then you could assign it to a variable, but then you need to keep in mind that calling the variable will not run the method.
you create a variable here and use it only once
bool taskExecutionCancelled = IsScripteExecutionCancelled();
if (taskExecutionCancelled)
{
tokenSource.Cancel();
}
I would just use the method in the if statement, like
if (IsScripteExecutionCancelled())
{
tokenSource.Cancel();
}
if you were going to use this value somewhere else and didn't want it evaluated again, then you could assign it to a variable, but then you need to keep in mind that calling the variable will not run the method.
edited Mar 14 at 18:59
answered Mar 14 at 17:45
Malachi♦
25.5k773175
25.5k773175
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f189478%2ftask-execution-with-cancel-logic%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
You are going to have to provide more context. It is unclear what this code is suppose to be doing.
– Nkosi
Mar 13 at 11:33
This question is incomplete. To help reviewers give you better answers, please add sufficient context to your question. The more you tell us about what your code does and what the purpose of doing that is, the easier it will be for reviewers to help you. Questions should include a description of what the code does
– Dannnno
Mar 13 at 13:57