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.

Something about Nothing

I must admit when I first saw the list of new language features for C# 6  like many I wasn’t that impressed, especially off the back of async/await.  This last month I have at last started my first new real C# 6 project and I must say while the features have not been earth shattering they are actually a joy to use, and certainly do help to clean up a fair amount of boiler plate code. After string interpolation I have found the null conditional operator (?.) aka the Elvis operator to be the most helpful, as it removes those often ugly if not null checks.  Historically I have removed allot of null checks using the Null Pattern

public abstract class Logger
{
   public static Logger NullLogger = new NullLogger();
   public abstract void Log(string message);
}

public class NullLogger : Logger
{ 
  public override void Log(string message)  {
     return;
  }
}

public class ConsoleLogger : Logger
{
  public override void Log(string message){
   Console.WriteLine(message);
  }
}
...
static void Main(string[] args)
{
  Logger logger = Logger.NullLogger;

  DoIt(logger);
}
private static void DoIt(Logger logger)
{
  logger.Log("Starting");
              ....
}

Making the logger assignment inside Main to be Logger.NullLogger as opposed to just null allows the code to work with out a null check.

C#6 ?. operator removes the need for the null pattern as we can simply make the call, removing the need for the NullLogger class instance.

 private static void DoIt(Logger logger)
 {
     logger?.Log("Starting");
 }

What makes the ?. operator even more compelling is that you can chain them one after another, this has been especially useful in LINQ to  XML processing. The following code won’t throw a null reference exception even though the Address and Street element is missing from the personWithNoAddress xml

 private static void Main(string[] args)
 {
  var person = new XElement("Person",
      new XElement("Name", "Andy"),
      new XElement("Address",
          new XElement("Street", "Redwing")));
  
  var personWithNoAddress = new XElement("Person");
  
  Console.WriteLine(person?.Element("Address")
                          ?.Element("Street"));
  
  Console.WriteLine(personWithNoAddress?.Element("Address")
                            ?.Element("Street"));
 }

Most of what has been written above is well know, what is perhaps slightly less well known is that you can also use the technique to remove the need for null checking when invoking an event.  Historically we have seen event invocation written as follows

public class Clock
 {
     public event EventHandler Tick;

     protected virtual void OnTick()
     {
         // non thread safe version
         if (Tick != null)
         {
             Tick(this, EventArgs.Empty);
         }

         // thread safe version
         var localTick = Tick;
         if (localTick != null)
         {
             localTick(this, EventArgs.Empty);
         }

        
     }
 }

We can now indeed use the ?. operator here too, the compiler emits the same code as the thread safe version in the above example.

protected virtual void OnTick()
{
  Tick?.Invoke(this, EventArgs.Empty);
}

I can’t say I’m a fan of this as we lose the simplification of the event invocation we have had since .NET 1.0.  So for me its back to the good old Null pattern for thread safe event invocation.

public class Clock
 {
     public event EventHandler Tick = delegate { }; 

     protected virtual void OnTick()
     {
        Tick(this,EventArgs.Empty);
     }
 }

This technique for me is by far the cleanest, and easiest to read.  So the Elvis operator will certainly not be universally adopted in my code.

While writing this article it also got me thinking where else does the compiler protect you from NullReferenceException.  One place I found was the using statement.

public class LazyLogger : Logger, IDisposable
 {
     private readonly string filename;
     private StreamWriter output;
     public LazyLogger(string filename)
     {
         this.filename = filename;
     }

     public override void Log(string msg)
     {
         output = output ?? new StreamWriter(filename);
         output.Write(msg);
     }

     public void Dispose()
     {
         using(output);
     }
 }

Clearly now you would use ?., which provides a far more clear intent but interesting to see this has been around for a while.

So to sum up I would strongly recommend keeping in mind the new features of C#6 as you plough through building another class, in the style of C#<6 and have a quick think could I make this more readable and cleaner with C#6.

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