How to make a method canceled without its ugliness?

Question

I have currently refitted our long lasting methods for cancellation. I plan to use System.Threading.Tasks.CancellationToken to implement this.

Our methods usually take several long steps (sending commands and then waiting for the hardware mostly), for example

void Run() { Step1(); Step2(); Step3(); } 

My first (maybe stupid) thought of canceling will turn it into

 bool Run(CancellationToken cancellationToken) { Step1(cancellationToken); if (cancellationToken.IsCancellationRequested) return false; Step2(cancellationToken); if (cancellationToken.IsCancellationRequested) return false; Step3(cancellationToken); if (cancellationToken.IsCancellationRequested) return false; return true; } 

which frankly looks awful. This “pattern” will continue inside one step (and they are already quite durable). This will make Thread.Abort () look pretty sexy, although I know this is not recommended.

Is there a cleaner template to achieve this that doesn't hide application logic under a lot of template code?

Edit

As an example for the nature of the steps, the Run method can read

 void Run() { GiantRobotor.MoveToBase(); Oven.ThrowBaguetteTowardsBase(); GiantRobotor.CatchBaguette(); // ... } 

We control various hardware units that need to be synchronized for collaboration.

Answer 1

If the steps are somehow independent of the data flow inside the method, but cannot be performed in a parallel question, the following approach may be more readable:

 void Run() { // list of actions, defines the order of execution var actions = new List<Action<CancellationToken>>() { ct => Step1(ct), ct => Step2(ct), ct => Step3(ct) }; // execute actions and check for cancellation token foreach(var action in actions) { action(cancellationToken); if (cancellationToken.IsCancellationRequested) return false; } return true; } 

If the steps do not require a cancellation token, because you can divide them into small units, you can even write a smaller list definition:

 var actions = new List<Action>() { Step1, Step2, Step3 }; 

Answer 2

how about going on?

 var t = Task.Factory.StartNew(() => Step1(cancellationToken), cancellationToken) .ContinueWith(task => Step2(cancellationToken), cancellationToken, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Current) .ContinueWith(task => Step3(cancellationToken), cancellationToken, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.Current); 

Answer 3

I admit that this is ugly, but management should either do what you did:

 if (cancellationToken.IsCancellationRequested) { /* Stop */ } 

... or a little shorter:

 cancellationToken.ThrowIfCancellationRequested() 

As a rule, if you can transfer the cancellation token to separate stages, you can distribute checks so that they do not saturate the code. You may also not constantly check the cancellation; if the operations you perform are idempotent and not resource intensive, you do not need to check the cancellation at each stage. The most important check time is to return the result.

If you pass the token to all of your steps, you can do something like this:

 public static CancellationToken VerifyNotCancelled(this CancellationToken t) { t.ThrowIfCancellationRequested(); return t; } ... Step1(token.VerifyNotCancelled()); Step2(token.VerifyNotCancelled()); Step3(token.VerifyNotCancelled()); 

Answer 4

When I needed to do something like this, I created a delegate to do this:

 bool Run(CancellationToken cancellationToken) { var DoIt = new Func<Action<CancellationToken>,bool>((f) => { f(cancellationToken); return cancellationToken.IsCancellationRequested; }); if (!DoIt(Step1)) return false; if (!DoIt(Step2)) return false; if (!DoIt(Step3)) return false; return true; } 

Or, if there is no code between the steps, you can write:

 return DoIt(Step1) && DoIt(Step2) && DoIt(Step3); 

Answer 5

Short version:

Use lock() to synchronize a Thread.Abort() call with critical sections.

---

Let me clarify the version:

Generally, when interrupting a stream, you will need to consider two types of code:

• Long code that doesn't suit you if it ends
• Code that must run its course

The first type is a type that we do not need if the user requested an interrupt. Maybe we were counting on a hundred billion, and he doesn’t care?

If we use sentinels such as the CancellationToken , are we unlikely to test them at each iteration of the non-essential code?

 for(long i = 0; i < bajillion; i++){ if(cancellationToken.IsCancellationRequested) return false; counter++; } 

So ugly. Therefore, for these cases, Thread.Abort() is a godsend.

Unfortunately, as some say, you cannot use Thread.Abort() because of the atomic code that absolutely needs to be run! The code has already deducted money from your account, now it must complete the transaction and transfer the money to the target account. Nobody likes money to disappear.

Fortunately, we have a mutual exception to help us with this. And C # does it pretty:

 //unimportant long task code lock(_lock) { //atomic task code } 

And in another place

 lock(_lock) //same lock { _thatThread.Abort(); } 

The number of locks will always be <= number of sentries, because you will also need sentries for non-essential code (to speed up its execution). This makes the code a little prettier than the sentinel version, but it also makes the interrupt better because it does not need to wait for irrelevant things.

---

It should also be noted that a ThreadAbortException can be thrown anywhere, even in finally blocks. This unpredictability makes Thread.Abort() so inconsistent.

With locks, you avoid this by simply locking the entire try-catch-finally . This cleanup in finally is important, then the whole block can be locked. try blocks are usually executed as short as possible, one line is preferable, therefore we do not block unnecessary code.

This makes Thread.Abort() , as you say, a little less evil. You may not want to name it in the user interface thread, however, since you are now blocking it.

Answer 6

If the refactor is allowed, you can reorganize the Step methods so that there is one Step(int number) method Step(int number) .

Then you can execute a cycle from 1 to N and check if the cancel token is requested once.

 bool Run(CancellationToken cancellationToken) { for (int i = 1; i < 3 && !cancellationToken.IsCancellationRequested; i++) Step(i, cancellationToken); return !cancellationToken.IsCancellationRequested; } 

Or, equivalently: (depending on what you prefer)

 bool Run(CancellationToken cancellationToken) { for (int i = 1; i < 3; i++) { Step(i, cancellationToken); if (cancellationToken.IsCancellationRequested) return false; } return true; } 

Answer 7

As an example, you can take a look at the Apple NSOperation template . This is more complicated than just making one method canceled, but it is pretty reliable.

Answer 8

I'm a little surprised that no one suggested a standard, built-in way to handle this:

 bool Run(CancellationToken cancellationToken) { //cancellationToke.ThrowIfCancellationRequested(); try { Step1(cancellationToken); Step2(cancellationToken); Step3(cancellationToken); } catch(OperationCanceledException ex) { return false; } return true; } void Step1(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ... } 

While usually you do not want to rely on imposing checks on deeper levels, in this case the steps already accept a CancellationToken and should have a check anyway (like any non-trivial method that accepts a CancellationToken).

It also allows you to be both granular and non-granular with checks as needed, that is, before lengthy / intensive operations.