Working with CancellationToken and Dispose

CancellationToken, and its owner CancellationTokenSource (CTS), were introduced in .NET 4.0 as a general purpose cancellation framework. It is often associated with Task as that was the first API to it. However, it is, in fact, independent of Task and should be used wherever you are supporting the concept of cancellation as async APIs you may call with be designed with it in mind.

One thing I only noticed recently is that CTS implements IDisposable. It previously hadn’t occurred to me that I would need to Dispose a CTS. Looking at the implementation in DotPeek I saw there were a few things that would require cleanup. So, dutifully, I put the calls in my code – only to see it sometimes blow up spectacularly with an ObjectDisposedException. To understand what was happening consider the following code

public class CancellableExecutor
{
    CancellationTokenSource cts = new CancellationTokenSource();
    public Task Execute(Action<CancellationToken> action)
    {
        return Task.Run(() => action(cts.Token), cts.Token);
    }

    public void Cancel()
    {
       cts.Cancel();
       cts.Dispose();
       cts = new CancellationTokenSource();
    }
}

Here we have a class whose job it is to be able to run a series of actions and cancel any outstanding on request. As you can imagine this type of object is intended to be used in asynchronous execution and therefore new pieces of work could be getting requested for execution while someone else is going to cancel.

The problem is that cts.Token throws an ObjectDisposedException if accessed after Dispose has been called. Now, I fully understand why I shouldn’t be able to, say, call Cancel on the CTS once it is Disposed but accessing a value that allows no more interaction than if I had accessed it just before Dispose was called seems very odd. Because of this, let’s call it a, “feature” we have to write the above code in the following non-obvious way

public class CancellableExecutor
{
    CancellationTokenSource cts = new CancellationTokenSource();
    CancellationToken token = CancellationToken.None;

    public CancellableExecutor()
    {
        token = cts.Token;
    }

    public Task Execute(Action<CancellationToken> action)
    {
        return Task.Run(() => action(token), token);
    }

    public void Cancel()
    {
        cts.Cancel();
        cts.Dispose();
        cts = new CancellationTokenSource();
        token = cts.Token;
    }
}

As you can see, we have to cache the CancellationToken and pass that around. Although this works, I really wish we could access the Token property after Dispose is called.

Let’s Stop the Async Suffix Bullshit

There is a common pattern that is used in .NET APIs where anything that returns a Task has the suffix Async.

Task<Result> GetResultAsync();

However, in reality this is just another form of Hungarian Notation. “But Microsoft do it in the BCL” you may say. And of course you are right. Buts lets be clear – they had to as they already had synchronous versions of the APIs from previous versions of .NET.

WebResponse GetResponse();
Task<WebResponse> GetResponseAsync();

So the Async suffix really came from expediency rather than a proposed standard pattern. In fact, where the API already support the Event Async Pattern (EAP) there already existed a version of the API with an Async suffix so they had to use TaskAsync to disambiguate.

Task<byte[]> DownloadDataTaskAsync();

If you have to support both a sync and async version of an API then maybe still use it – or rather, to encourage async interaction suffix the synchronous version with a Sync suffix as in NodeJS APIs.

But most APIs should be either inherently sync or async and in that case leave off the suffix and just return the appropriate type.

Unit Testing Decorator Base Classes

When using the decorator pattern it is a common practice to create a base class implementation of the abstraction that simply forwards all calls to the decorated class. This means that in the actual functional decorators, once you derive from this base class, you only need to implement the behavior for the specific methods of interest rather than every method. The caveat though is that, because C# methods are not virtual by default you must remember to mark all methods as virtual.

image

I used to think it gave little benefit to write unit tests for these base decorators but a couple of experiences have shown that without too much effort you can write tests that live as guards as the decorated abstraction evolves. The two main issues I have seen are

  1. Failing to forward on the call to the downstream object
  2. Failing to mark the base implementation as virtual

To solve issue 1 we just make sure we have tests that cover each of the methods on the abstraction. As an example, take an abstraction for a prioritized queue

image

Now we can write tests for a decorator base class quite easily to verify forwarding

image

However, how do we write a test to ensure all members are virtual? Well, reflection to the rescue. We can look at each method in the SUT and verify it is virtual. However, beware that the MethodBase class has an IsVirtual method but this does not mean the same as the keyword in C#. Interface implementations also return IsVirtual as true as they are also subject to virtual dispatch. What we really are looking for is a way to ask “Is this method overridable?”. To ensure that we also need to check that the IsFinal flag on MethodBase is false.

image

Now we are much more likely to catch issues with the base class as the abstraction evolves