Saturday, April 7, 2012

Using a singleton (anti?)pattern

Some things that always apply to a Singleton Pattern (a pattern that falls into the creational pattern category):
  • Think twice before using a singleton. Singletons can be bad for: parallel unit tests, dependency coupling, scalability, performance, memory management, complex threading issues (see here and here and countless others).
  • Always use interfaces with your singletons.
  • Always return the interface from the default static property.
  • Prefer using a static default property that returns an interface over static methods that access a private static field. (This helps enforce the pattern when other developers add new members to the singleton).
  • Always use a private constructor, this prevents any ad-hoc use of the class bypassing the singleton. (No one can create the class so you can only use the static Default property).
  • Seal your singletons.  This will disallow someone sub-classing it then instantiating it as a transient class and bypassing the singleton.
  • By definition, a singleton should be constructing only when it is first used.

Ok, so you insist on using a Singleton. Very well, lets try to do it with as least smelly code as possible...

All code examples make use of this interface.
namespace SingletonDemo
{
    public interface ISomeSingleton
    {
        void DoSomething();
        string Stuff { get; }
    } 
}


Use a Simple Singleton as a first option. Only use this when you know there will be no thread contention at initialisation time of the singleton. Or maybe it doesn't matter if there is.  This works well for unit testing because you can easily inject a mock singleton using a private accessor (reflection) or you could add a internal setter to the Default property and allow your test project access to internals (see this blog post for more details).

namespace SingletonDemo
{
    public sealed class SimpleSingleton : ISomeSingleton
    {
        private static ISomeSingleton defaultInstance;

        private SimpleSingleton()
        {
            // private ctor to prevent transient usage.
        }

        public static ISomeSingleton Default
        {
            get
            {
                return defaultInstance ?? (defaultInstance = new SimpleSingleton());
            }
        }

        public void DoSomething()
        {
            // omitted...
        }

        public string Stuff
        {
            get
            {
                // omitted...
                return string.Empty;
            }
        }
    }
}

Use what I call a ContentionSingleton when you know there will be thread contention when the singleton is first accessed. This example guarantees only one thread can create the singleton, all other threads wait until it is initialised.

namespace SingletonDemo
{
    public sealed class ContentionSingleton : ISomeSingleton
{
        private static readonly object SyncRoot = new object();
        private static volatile ISomeSingleton defaultInstance;

        private ContentionSingleton()
        {
            // Intialisation code...
        }

        public ISomeSingleton Default
        {
            get
            {
                if (defaultInstance == null)
                {
                    lock (SyncRoot)
                    {
                        if (defaultInstance == null)
                        {
                            defaultInstance = new ContentionSingleton();
                        }
                    }
                }

                return defaultInstance;
            }
        }

        public void DoSomething()
        {
            // Omitted...
        }

        public string Stuff
        {
            get
            {
                // Omitted...
                return string.Empty;
            }
        }
    }
}
Things to note:
  • It is best practise to use a dedicated and separate lock object.  Never use the same lock object for different purposes. The lock object should always be private; no one outside this class should be using it.
  • Declare the singleton instance as "volatile".  This indicates to the runtime that this object might be changed by different threads at the same time. This exempts this field from compiler optimisation that assumes only one thread will access this object at a time. It also guarantees the most up to date value will be in the field at all times.
  • Private constructor.
  • Double check the field both outside the lock and in. This ensures that 2 (or more) threads trying to initialise the singleton at the same time one enters the lock and one waits. Then when the first exits the lock the second will enter the lock, when it does it must not re-initialise the singleton.
  • This still works well with unit testing for the same reasons as SimpleSingleton above.

There is another way to create singleton behaviour which I am not a fan of, Type Initialised Singleton:

namespace SingletonDemo
{
    public sealed class TypeInitialisedSingleton : ISomeSingleton
{
        private static readonly ISomeSingleton DefaultInstance;

        static TypeInitialisedSingleton()
        {
// .NET Runtime guarantees to run static constructors exactly once.
DefaultInstance = new TypeInitialisedSingleton(); // Impossible to intercept during unit testing and mock. } private TypeInitialisedSingleton() { // Connect to Database... // Get data using a bunch of SQL statements... } public ISomeSingleton Default { get { return DefaultInstance; } } public void DoSomething() { // Omitted... } public string Stuff { get { // Omitted... return string.Empty; } } } }


BAD BAD BAD.  There is no way to intercept the static constructor code during unit testing and inject mock behaviour.  As soon as the type is referenced by a piece of code the type initialiser will run.  I would avoid this at all costs.  Even if you don't plan to do unit testing, if you change you're mind later it may be hard to change. Generally speaking type initialisers (OO term for static constructors) should be avoided, and when used only used for simple static member initialisation.  If something goes wrong in the static constructor, even if handled, it can leave the application in a weird state (check this post).



A much improved version of a TypeInitialiserSingleton is the BishopSingleton (named after Judith Bishop and its from her book C# Design Patterns [O'Reilly])

namespace SingletonDemo
{
    public sealed class BishopSingleton : ISomeSingleton
    {
        // Local instance allows for tests to inject mock behaviour
        private static ISomeSingleton localInstance;

        private static class SingletonCreator
        {
            // Despite being internal, this is not visible outside BishopSingleton class.
            internal static readonly ISomeSingleton DefaultInstance;
            
            static SingletonCreator()
            {
                // if localInstance is not-null this will not be called.
                DefaultInstance = new BishopSingleton();
            }
        }

        private BishopSingleton()
        {
            // Initialisation...
        }

        public static ISomeSingleton Default
        {
            get
            {
                return localInstance ?? SingletonCreator.DefaultInstance;
            }
        }

        public void DoSomething()
        {
            // omitted...
        }

        public string Stuff
        {
            get
            {
                // Omitted...
                return string.Empty;
            }
        }
    }
}

This is quite elegant as it doesn't require understanding intricacies locking code and gives a means of tests injecting mock singletons (using private accessors). Although you will get Resharper and Code Analysis warnings that localInstance is never assigned, which feels a little dirty. Which is better? I personally prefer using SimpleSingleton when I know there is no thread contention and ContentionSingleton otherwise.  It is more obvious to the average developer what is going on inside the ContentionSingleton compared to the BishopSingleton in my humble opinion.

Finally, think once, twice, and thrice before using a Singleton, prefer using dependency injection, Inversion of Control, or a query or factory pattern, or even caching data in a shared way (MemCache or AppFabric).

1 comment:

  1. For completeness...

    public class IocSingleton : ISomeSingleton
    {
    public void DoSomething() {}

    public string Stuff { get; set; }
    }
    public class IoCSingletonConsumer
    {
    public void Main()
    {
    // Configuration code...
    var container = new UnityContainer();
    container.RegisterType<ISomeSingleton, IoCSingleton>(new ContainerControlledLifetimeManager());

    // Usage
    var singleton1 = container.Resolve();
    var singleton2 = container.Resolve();
    Debug.Assert(singleton1 == singleton2);
    }
    }

    ReplyDelete