A generic singleton

Question

What do you guys think about this for a generic singleton?

using System;
using System.Reflection;

// Use like this
/*
public class Highlander : Singleton<Highlander>
{
    private Highlander()
    {
        Console.WriteLine("There can be only one...");
    }
}
*/

public class Singleton<T> where T : class
{
    private static T instance;
    private static object initLock = new object();

    public static T GetInstance()
    {
        if (instance == null)
        {
            CreateInstance();
        }

        return instance;
    }

    private static void CreateInstance()
    {
        lock (initLock)
        {
            if (instance == null)
            {
                Type t = typeof(T);

                // Ensure there are no public constructors...
                ConstructorInfo[] ctors = t.GetConstructors();
                if (ctors.Length > 0)
                {
                   throw new InvalidOperationException(String.Format("{0} has at least one accesible ctor making it impossible to enforce singleton behaviour", t.Name));
                }

                // Create an instance via the private constructor
                instance = (T)Activator.CreateInstance(t, true);
            }
        }
    }
}

Answer 1

Well, it isn't really singleton - since you can't control T, there can be as many T instances as you like.

(removed thread-race; noted the double-checked usage)

Answer 2

Creating a singleton class is just a few lines of code, and with the difficulty of making a generic singleton i always write those lines of code.

public class Singleton
{
    private Singleton() {}
    private static Singleton _instance = new Singleton();
    public static Singleton Instance { get { return _instance; }}
}

The

private static Singleton _instance = new Singleton();

line removes the need for locking, as a static constructor is thread safe.

Answer 3

I've deleted my previous answer as I hadn't noticed the code which checks for non-public constructors. However, this is a check which is only performed at execution time - there's no compile-time check, which is a strike against it. It also relies on having enough access to call the non-public constructor, which adds some limitations.

In addition, it doesn't prohibit internal constructors - so you can end up with non-singletons.

I'd personally create the instance in a static constructor for simple thread safety, too.

Basically I'm not much of a fan - it's pretty easy to create singleton classes, and you shouldn't be doing it that often anyway. Singletons are a pain for testing, decoupling etc.