How to make a method cancelable without it becoming ugly?

Question 1

If the steps are somehow independend regarding the dataflow within the method, but can't be executed in a parallel matter, the following approach may be better 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 don't need the cancellation token because you can split them up in tiny units, you can even write a smaller list definition:

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

Question 2

what about continuation?

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);

Question 3

I admit it isn't pretty, but the guidance is to either do what you have done:

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

...or the slightly shorter:

cancellationToken.ThrowIfCancellationRequested()

Generally, if you can pass the cancellation token to the individual steps, you can spread the checks out such that they don't saturate the code. You may also choose not to check for cancellation constantly; if the operations you are performing are idempotent and aren't resource intensive, you don't necessarily have to check for cancellation at every stage. The most important time to check is before returning a result.

If you're passing the token to all of your steps, you could do something like this:

public static CancellationToken VerifyNotCancelled(this CancellationToken t) {
    t.ThrowIfCancellationRequested();
    return t;
}

...

Step1(token.VerifyNotCancelled());
Step2(token.VerifyNotCancelled());
Step3(token.VerifyNotCancelled());

Question 4

When I had to do something like that, I created a delegate to do it:

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 never any code between the steps, you can write:

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

Question 5

Short version:

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

Let me explain version:

Generally, when aborting thread's you'd have to consider two types of code:

Long running code you don't really care if it finishes
Code that absolutely has to run it's course

First type is the type we don't care about if the user requested abort. Maybe we were counting to a hundred billion and he doesn't care anymore?

If we'd use sentinels such as the CancellationToken, we would hardly test them in each iteration of the unimportant code would we?

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

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

Unfortunately, as some say, you can't use Thread.Abort() because of the atomic code that absolutely has to run! The code has already subtracted money form your account, now it has to complete the transaction and transfer the money to target account. Nobody likes money disappearing.

Luckily, we have mutual exclusion to help us with this sort of thing. And C# makes it pretty:

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

And elsewhere

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

The number of locks will always be <= number of sentinels, because you'd be wanting sentinels on the unimportant code too (to make it abort faster). This makes the code slightly prettier than the sentinel version, but it also makes aborting better, because it doesn't have to wait for unimportant things.

Also to note is that ThreadAbortException could be raised anywhere, even in finally blocks. This unpredictability makes Thread.Abort() so controversial.

With locks you'd avoid this by just locking the entire try-catch-finally block. It the cleanup in finally is essential, then the entire block can be locked. try blocks are generally made to be as short as possible, one line preferably, so we aren't locking any unnecessary code.

This makes Thread.Abort(), as you say, a bit less evil. You might not want to call it on the UI thread, though, as you're now locking it.

Question 6

I am kind of amazed that no one has proposed the standard, built-in, way of handling 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 normally you don't want to rely on pushing checks to deeper levels, in this case the steps already accept a CancellationToken and should have the check anyway (as should any non-trivial method accepting a CancellationToken).

This also allows you to be as granular or non-granular with the checks as needed, i.e. before long running / intensive operations.

Question 7

If a refactor is permissible, you could refactor the Step methods such that there is one method Step(int number).

You can then loop through from 1 to N and check if the cancellation token is requested only once.

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

    return !cancellationToken.IsCancellationRequested;
}

Or, equivalently: (Whichever you prefer)

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

Question 8

You might want to take a look at Apple's NSOperation pattern as an example. It's more complicated than simply making a single method cancelable, but it's pretty robust.