Wrapping locking structures in tests

Over the last week I’ve been writing some locking code around a task queue, and wondering generally how to test whether locks are taken.

The straightforward thing to do is to take the locking calls as a seam and introduce a proxy, behind which I can add my own behaviour. I’ve not thought about the practicality of what followed once I started – it may be supremely impractical – but out of interest’s sake I thought I’d see where it went.

I’ve been working with ReaderWriterLockSlim a fair amount, so that was a natural place to start. I added a hook that meant I could introduce a new implementation for EnterWriteLock. There are a couple of wrinkles over the work I did with static classes recently – in this case I’m explicitly proxying a class instance, so I can’t rely on static hooks into external code, and default them with a static method group.

Neither can I specify a lambda, as it would rely on calling methods on an instance that wasn’t available at compile time, leading to errors like “cannot access non-static member in static context”. This just means I need to resolve any default implementation at runtime.

namespace Threading.Abstractions
{
    public class ReaderWriterLockSlim
    {
        private readonly System.Threading.ReaderWriterLockSlim _lock = 
            new System.Threading.ReaderWriterLockSlim();

        public ReaderWriterLockSlim()
        {
            EnterWriteLockImpl = (Action)(() => _lock.EnterWriteLock());
        }

        public void EnterWriteLock()
        {
            EnterWriteLockImpl();
        }

        public Action EnterWriteLockImpl;
    }
}

Then in a test I might replace the EnterWriteLock implementation:

[TestFixture]
public class Tests
{
    [Test]
    public void SimpleLockTest()
    {
        var locker = new Threading.Abstractions.ReaderWriterLockSlim();

        var count = 0;
        locker.EnterWriteLockImpl = (Action)(() => count++);

        locker.EnterWriteLock();

        Assert.AreEqual(1, count);
    }
}

So, that's good, I can tell if the lock has been taken or not. I can take it a little bit further though. One thing I might want to do is check log which locks had been taken when – something that might notionally be useful even in production code. If I add an OnEnterWriteLock hook then I can log an entry if needed:

namespace Threading.Abstractions
{
    public class ReaderWriterLockSlim
    {
        private readonly System.Threading.ReaderWriterLockSlim _lock = 
            new System.Threading.ReaderWriterLockSlim();

        public ReaderWriterLockSlim()
        {
            EnterWriteLockImpl = (Action)(() => _lock.EnterWriteLock());
        }

        public Action OnEnterWriteLock;

        public Action AfterEnterWriteLock;

        public void EnterWriteLock()
        {
            if(OnEnterWriteLock != null) OnEnterWriteLock();

            EnterWriteLockImpl();

            if (AfterEnterWriteLock != null) AfterEnterWriteLock();
        }

        public Action EnterWriteLockImpl;
    }
}

After that I can add any logging that I might want to – I could append part of the stack trace for example; the name of the object that’s being called; or any number of things that help us track how it’s used:

[TestFixture]
public class Tests
{
    [Test]
    public void LockTest()
    {
        var locker = new Threading.Abstractions.ReaderWriterLockSlim();

        locker.OnEnterWriteLock = 
            (Action) (() => Console.WriteLine("Entering write lock") );
        locker.AfterEnterWriteLock = 
            (Action)(() => Console.WriteLine("Entered write lock"));

        locker.EnterWriteLock();
    }
}

Another interesting thing to try might be to actually create a timeline of all locks being taken and released. If I create a simple lock log:

namespace Threading.Abstractions
{
    public static class LockHistory
    {
        private static object locker = new object();

        public readonly static 
            System.Collections.Generic.List Entries = 
                new System.Collections.Generic.List();

        public static void Add(string lockEvent)
        {
            lock (locker) Entries.Add(lockEvent);
        }
    }
}

I can then add a log entry every time a lock is taken or released, and play that log back at the end of the test.

[TestFixture]
public class Tests
{
    [Test]
    public void LockTest()
    {
        var locker = new Threading.Abstractions.ReaderWriterLockSlim();

        locker.OnEnterWriteLock = 
            (Action) (() => Threading.Abstractions.LockHistory.Add("Entering Write Lock") );
        locker.AfterEnterWriteLock = 
            (Action)(() => Console.WriteLine("Entered write lock"));
        
        locker.EnterWriteLock();

        foreach (var entry in Threading.Abstractions.LockHistory.Entries)
        {
            Console.WriteLine(entry);
        }
    }
}

The log history could then be parsed or analysed in some way. All good fun, although, as I say -- I'm not sure how practical it might be.

The distinctiveness of is, be, has; add

Tripped up today in a discussion of a code change in which I thought an interface implementation was being proposed, where in fact the proposal was to serve up an instance implementing the new interface.

We were discussing an existing class that acted somewhat like a registry, with which the population of instances to be served up was defined at construction time. We needed it to delay resolution of one of the types it was serving up until relatively late – at least, well after construction of the registry.

The phrase that tripped me up was along the lines of: “Add a new interface to the registry, named ISomeFactoryInterface”.

I interpreted this to mean that the registry should implement the factory interface, and had a hard time persuading my counterpart that this was a bad idea.

Of course, it was hard because he was using it to mean has, not is. He thought the registry should be serving up instances of this new interface. That was actually the position I came to too, so in the end we converged anyway.

Interestingly, if he’d used the word has explicitly I might still have misinterpreted to start with. In normal English saying that something “has a certain interface” might imply that it implements that interface.

It reminded me that in the absence of actual code being written down, we have to be really very careful about how we use certain words.

Comment on Mark Seemann’s post on Abstract Factory and Service Locator

Mark Seemann has a post that I’ve been puzzling over, because the patterns he presents didn’t make sense to me. It’s about a year old, but the issues seem to come up again and again. I thought I’d hash over it for my own benefit.

He’s summarizing the differences between Abstract Factory and Service Locator, saying they’re easily confused but are actually easy to differentiate.

He shows a condensed Abstract Factory interface:

public interface IFactory<T>
{
    T Create(object context);
}

and states that Abstract Factory is a generic type that can only return instances of one type -- the type given by the generic parameter.

To me that’s not an Abstract Factory. The original Design Patterns book states that the pattern’s intent is creating families of related or dependent objects; it can therefore a single factory type can create instances of multiple types. Mark’s factory can only create one type, dictated by the closed type of the factory.

A real abstract factory interface might looks more like:

public interface IEcosystem
{
    IHumanoid CreateHumanoid();

    ICreature CreateCreature();
}

This does look a little like a Service Locator, in that it each IEcosystem-derived type can return multiple types. A Service Locator also doesn’t need to be generic – it might have a distinct method per service type returned.

He also posits a simplified ServiceLocator interface:

public interface IServiceLocator
{
    T Create<T>(object context);
}

(again, slightly condensed by me) that has a method named Create. Again, this interface doesn’t look right. A Service Locator doesn't handle instantiation for you, it simply hands you references to single already-instantiated services; it's a structural pattern. I'd have expected a method like T Locate<T>(object context).

Specific instances of Service Locator might be created -- if you relax the requirement that it's a singleton -- that are then populated with distinct sets of services for different domains, but typically you wouldn't need multiple Service Locator types.

For me the key difference between Abstract Factory and Service Locator is that the first is a creational pattern, and the second a structural pattern.

The Factory exists in single instances of multiple distinct types, one for each family of related classes. The Locator is traditionally a singleton at global scope, with which single instances of each type supported are registered; it also supports types that are contextually related but not necessarily as closely related as those supported by the Factory.