84

From what I've read about Tasks, the following code should cancel the currently executing task without throwing an exception. I was under the impression that the whole point of task cancellation was to politely "ask" the task to stop without aborting threads.

The output from the following program is:

Dumping exception

[OperationCanceledException]

Cancelling and returning last calculated prime.

I am trying to avoid any exceptions when cancelling. How can I accomplish this?

void Main()
{
    var cancellationToken = new CancellationTokenSource();

    var task = new Task<int>(() => {
        return CalculatePrime(cancellationToken.Token, 10000);
    }, cancellationToken.Token);

    try
    {
        task.Start();
        Thread.Sleep(100);
        cancellationToken.Cancel();
        task.Wait(cancellationToken.Token);         
    }
    catch (Exception e)
    {
        Console.WriteLine("Dumping exception");
        e.Dump();
    }
}

int CalculatePrime(CancellationToken cancelToken, object digits)
{  
    int factor; 
    int lastPrime = 0;

    int c = (int)digits;

    for (int num = 2; num < c; num++)
    { 
        bool isprime = true;
        factor = 0; 

        if (cancelToken.IsCancellationRequested)
        {
            Console.WriteLine ("Cancelling and returning last calculated prime.");
            //cancelToken.ThrowIfCancellationRequested();
            return lastPrime;
        }

        // see if num is evenly divisible 
        for (int i = 2; i <= num/2; i++)
        { 
            if ((num % i) == 0)
            {             
                // num is evenly divisible -- not prime 
                isprime = false; 
                factor = i; 
            }
        } 

        if (isprime)
        {
            lastPrime = num;
        }
    }

    return lastPrime;
}
Gustavo Mori
  • 8,319
  • 3
  • 38
  • 52
Razor
  • 17,271
  • 25
  • 91
  • 138
  • If you are trying to avoid exceptions, you shouldn't be raising one with ThrowIfCancellationRequested. Just return gracefully from CalculatePrime. – K-ballo Sep 08 '11 at 05:01
  • I have removed cancelToken.ThrowIfCancellationRequested(); and I still get the same result. – Razor Sep 08 '11 at 05:03
  • Don't pass your cancellation token to Task.wait, since you have requested for the task to be cancelled. Call Task.wait() instead. – K-ballo Sep 08 '11 at 05:23
  • Hi. I still not get the point of having this class if we manually need to check `ThrowIfCancellationRequested` or `IsCancellationRequested`. I thought task should cancel automatically when I call `Cancel()` – Rashmin Javiya Dec 20 '18 at 06:58

5 Answers5

122

I am trying to avoid any exceptions when cancelling.

You shouldn't do that.

Throwing OperationCanceledException is the idiomatic way that "the method you called was cancelled" is expressed in TPL. Don't fight against that - just expect it.

It's a good thing, because it means that when you've got multiple operations using the same cancellation token, you don't need to pepper your code at every level with checks to see whether or not the method you've just called has actually completed normally or whether it's returned due to cancellation. You could use CancellationToken.IsCancellationRequested everywhere, but it'll make your code a lot less elegant in the long run.

Note that there are two pieces of code in your example which are throwing an exception - one within the task itself:

cancelToken.ThrowIfCancellationRequested()

and one where you wait for the task to complete:

task.Wait(cancellationToken.Token);

I don't think you really want to be passing the cancellation token into the task.Wait call, to be honest... that allows other code to cancel your waiting. Given that you know you've just cancelled that token, it's pointless - it's bound to throw an exception, whether the task has actually noticed the cancellation yet or not. Options:

  • Use a different cancellation token (so that other code can cancel your wait independently)
  • Use a time-out
  • Just wait for as long as it takes
Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • Jon cost of entering try is higher than if(IsCancellationRequested). in normal project you would ignore that for the benefits of what you've said. however if you want avoid extra cost at any cost then you effectively cannot use cancellation and need to build it by hand........ I am sure they could have implemented property something ThrowIfCancelled for those who wants exceptions. But no. – Boppity Bop Jun 04 '12 at 15:03
  • 13
    @Bobb: Note that both the property (`IsCancellationRequested`) and the method (`ThrowIfCancellationRequested`) *are* both implemented in the TPL. So in the *very extreme cases* where you really don't want the cost of an exception, you can avoid it - but I wouldn't do so without *evidence* that it was actually a bottleneck in the system. – Jon Skeet Jun 04 '12 at 19:14
  • 13
    I'm currently having trouble with this, mostly because cancellation seems like a normal operation, and using an exception for normal flow control *stll* feels wrong. What am I missing? – Tim Barrass Jun 24 '14 at 14:33
  • 10
    @TimBarrass: It feels a *bit* wrong to me too, but I can live with it. If you think of the exception not as an *error*, but as a way of saying "I didn't complete the operation you asked me to do - and here's why" then it may make more sense. The mantra of not using exceptions for flow control is possibly a little overblown - they're *always* used for flow control to some extent. It's more a matter of not using them in the normal success path. – Jon Skeet Jun 24 '14 at 14:56
  • 2
    @JonSkeet No answer has a nice comment to another reply here -- noting that a cancellation is a rare/on-off event, so criticism based on performance (for example) may be basically irrelevant. But more closely related to your reply -- it occurs to me that cancellations **are** _exceptional_ in some sense ... /squints slightly – Tim Barrass Jun 24 '14 at 16:21
  • 1
    When the exception is raised how would one know how far along the method that the task is running on had executed? Since the exception would rudely abort the Task, couldn't this cause data inconsistency issues? – Abhijeet Patel Jul 08 '14 at 06:21
  • 3
    @AbhijeetPatel: That's very context-dependent, but usually you would try to code it so that it didn't matter at which point the task was cancelled. – Jon Skeet Jul 08 '14 at 06:25
80

You are explicitly throwing an Exception on this line:

cancelToken.ThrowIfCancellationRequested();

If you want to gracefully exit the task, then you simply need to get rid of that line.

Typically people use this as a control mechanism to ensure the current processing gets aborted without potentially running any extra code. Also, there is no need to check for cancellation when calling ThrowIfCancellationRequested() since it is functionally equivalent to:

if (token.IsCancellationRequested) 
    throw new OperationCanceledException(token);

When using ThrowIfCancellationRequested() your Task might look more like this:

int CalculatePrime(CancellationToken cancelToken, object digits) {
    try{
        while(true){
            cancelToken.ThrowIfCancellationRequested();

            //Long operation here...
        }
    }
    finally{
        //Do some cleanup
    }
}

Also, Task.Wait(CancellationToken) will throw an exception if the token was cancelled. To use this method, you will need to wrap your Wait call in a Try...Catch block.

MSDN: How to Cancel a Task

Josh
  • 44,706
  • 7
  • 102
  • 124
  • 2
    Thank's Josh. It makes sense to call ThrowIfCancellationRequested() on each iteration of the loop rather than doubling up and checking the cancellation flag. – Razor Sep 08 '11 at 07:18
  • 58
    As a warning: replacing `ThrowIfCancellationRequested` with a bunch of checks for `IsCancellationRequested` exits gracefully, as this answer says. But that's not just an implementation detail; that affects observable behavior: the task will no longer end in the cancelled state, but in `RanToCompletion`. And *that* can affect not just explicit state checks, but also, more subtly, task chaining with e.g. `ContinueWith`, depending on the `TaskContinuationOptions` used. I'd say that avoiding `ThrowIfCancellationRequested` is dangerous advice. – Eamon Nerbonne Sep 23 '15 at 08:37
  • "Task.Wait(CancellationToken) will throw an exception if the token was cancelled." That was my issue. Thx. – granadaCoder Jan 05 '17 at 13:42
  • Couldn't an alternative be to just wait on the token's handle? i.e. `token.WaitHandle.WaitOne()` as opposed to having your wait sit in a `try`/`catch` block? – Snoop Aug 24 '17 at 17:01
12

Some of the above answers read as if ThrowIfCancellationRequested() would be an option. It is not in this case, because you won't get your resulting last prime. The idiomatic way that "the method you called was cancelled" is defined for cases when canceling means throwing away any (intermediate) results. If your definition of cancelling is "stop computation and return the last intermediate result" you already left that way.

Discussing the benefits especially in terms of runtime is also quite misleading: The implemented algorithm sucks at runtime. Even a highly optimized cancellation will not do any good.

The easiest optimization would be to unroll this loop and skip some unneccessary cycles:

for(i=2; i <= num/2; i++) { 
  if((num % i) == 0) { 
    // num is evenly divisible -- not prime 
    isprime = false; 
    factor = i; 
  }
} 

You can

  • save (num/2)-1 cycles for every even number, which is slightly less than 50% overall (unrolling),
  • save (num/2)-square_root_of(num) cycles for every prime (choose bound according to math of smallest prime factor),
  • save at least that much for every non-prime, expect much more savings, e.g. num = 999 finishes with 1 cycle instead of 499 (break, if answer is found) and
  • save another 50% of cycles, which is of course 25% overall (choose step according to math of primes, unrolling handles the special case 2).

That accounts to saving a guaranteed minimum of 75% (rough estimation: 90%) of cycles in the inner loop, just by replacing it with:

if ((num % 2) == 0) {
  isprime = false; 
  factor = 2;
} else {
  for(i=3; i <= (int)Math.sqrt(num); i+=2) { 
    if((num % i) == 0) { 
      // num is evenly divisible -- not prime 
      isprime = false; 
      factor = i;
      break;
    }
  }
} 

There are much faster algorithms (which I won't discuss because I'm far enough off-topic) but this optimization is quite easy and still proves my point: Don't worry about micro-optimizing runtime when your algorithm is this far from optimal.

No answer
  • 907
  • 1
  • 9
  • 10
  • +1, however I'm not sure your answer does justice to how slow the algorithm in use is compared to a proper one. He should start at the maximum number and work down using a [Baillie–PSW primality test](http://en.wikipedia.org/wiki/Baillie%E2%80%93PSW_primality_test). – Sam Harwell Jan 15 '14 at 01:37
  • I suppose the task at hand is **not** solve the problem of "computing as large a prime as possible in defined computation time" but learn coding by examples like "calculate all primes until a request for cancellation upon which return the largest prime computed". I didn't want to start a discussion of randomized prime tests, because requirements might call for 100% assurance of detected primes. For large primes this would lead to the deterministic AKS primality test, which in turn uses or is beaten by the Sieve of Eratosthenes for all numbers smaller than 2^23, if I remember correctly. – No answer Jan 21 '14 at 10:08
  • 3
    Sorry, I went as far off-topic as I originally didn't want to. Yet, the point missing in my answer to "280Z28 Jan 15 at 1:37" is that a guaranteed saving of 75% of inner cycles renders any " **cancelation** runtime discussion" obsolete. Considering that cancelation is per definition executed only a single time per call, there shouldn't be many applications you need to worry about its runtime. – No answer Jan 21 '14 at 10:19
11

Another note about the benefit of using ThrowIfCancellationRequested rather than IsCancellationRequested: I've found that when needing to use ContinueWith with a continuation option of TaskContinuationOptions.OnlyOnCanceled, IsCancellationRequested will not cause the conditioned ContinueWith to fire. ThrowIfCancellationRequested, however, will set the Canceled condition of the task, causing the ContinueWith to fire.

Note: This is only true when the task is already running and not when the task is starting. This is why I added a Thread.Sleep() between the start and cancellation.

CancellationTokenSource cts = new CancellationTokenSource();

Task task1 = new Task(() => {
    while(true){
        if(cts.Token.IsCancellationRequested)
            break;
    }
}, cts.Token);
task1.ContinueWith((ant) => {
    // Perform task1 post-cancellation logic.
    // This will NOT fire when calling cst.Cancel().
}

Task task2 = new Task(() => {
    while(true){
        cts.Token.ThrowIfCancellationRequested();
    }
}, cts.Token);
task2.ContinueWith((ant) => {
    // Perform task2 post-cancellation logic.
    // This will fire when calling cst.Cancel().
}

task1.Start();
task2.Start();
Thread.Sleep(3000);
cts.Cancel();
Gerard Torres
  • 113
  • 1
  • 5
  • 1
    That is because the Task status is only set to Canceled when it exits with a given Exception type, otherwise, if you just "return" out of it, its status is set to RanToCompletion. And hence, the OnlyOnCanceled piece of code is not called. – almulo Oct 14 '14 at 13:10
1

You have two things listening to the token, the calculate prime method and also the Task instance named task. The calculate prime method should return gracefully, but task gets cancelled while it is still running so it throws. When you construct task don't bother giving it the token.

andrew pate
  • 3,833
  • 36
  • 28