A simple implementation of a Singleton

Question

Isn't this a simpler as well as safe (and hence better) way to implement a singleton instead of doing double-checked locking mambo-jambo? Any drawbacks of this approach?

public class Singleton
{
    private static Singleton _instance;
    private Singleton() { Console.WriteLine("Instance created"); }

    public static Singleton Instance
    {
        get
        {
            if (_instance == null)
            {
                Interlocked.CompareExchange(ref _instance, new Singleton(), null);
            }
            return _instance;
        }
    }
    public void DoStuff() { }
}

EDIT: the test for thread-safety failed, can anyone explain why? How come Interlocked.CompareExchange isn't truly atomic?

public class Program
{
   static void Main(string[] args)
   {
      Parallel.For(0, 1000000, delegate(int i) { Singleton.Instance.DoStuff(); });
   }
} 

Result (4 cores, 4 logical processors)
Instance created
Instance created
Instance created
Instance created
Instance created

Answer 1

If your singleton is ever in danger of initializing itself multiple times, you have a lot worse problems. Why not just use:

public class Singleton
{
  private static Singleton instance=new Singleton();
  private Singleton() {}

  public static Singleton Instance{get{return instance;}}
}

Absolutely thread-safe in regards to initialization.

Edit: in case I wasn't clear, your code is horribly wrong. Both the if check and the new are not thread-safe! You need to use a proper singleton class.

Answer 2

You may well be creating multiple instances, but these will get garbage collected because they are not used anywhere. In no case does the static _instance field variable change its value more than once, the single time that it goes from null to a valid value. Hence consumers of this code will only ever see the same instance, despite the fact that multiple instances have been created.

Lock free programming

Joe Duffy, in his book entitled Concurrent Programming on Windows actually analyses this very pattern that you are trying to use on chapter 10, Memory models and Lock Freedom, page 526.

He refers to this pattern as a Lazy initialization of a relaxed reference:

public class LazyInitRelaxedRef<T> where T : class
{
    private volatile T m_value;
    private Func<T> m_factory;

    public LazyInitRelaxedRef(Func<T> factory) { m_factory = factory; }


    public T Value
    {
        get
        {
            if (m_value == null) 
              Interlocked.CompareExchange(ref m_value, m_factory(), null);
            return m_value;
        }
    }

    /// <summary>
    /// An alternative version of the above Value accessor that disposes
    /// of garbage if it loses the race to publish a new value.  (Page 527.)
    /// </summary>
    public T ValueWithDisposalOfGarbage
    {
        get
        {
            if (m_value == null)
            {
                T obj = m_factory();
                if (Interlocked.CompareExchange(ref m_value, obj, null) != null && obj is IDisposable)
                    ((IDisposable)obj).Dispose();
            }
            return m_value;
        }
    }
}

As we can see, in the above sample methods are lock free at the price of creating throw-away objects. In any case the Value property will not change for consumers of such an API.

Balancing Trade-offs

Lock Freedom comes at a price and is a matter of choosing your trade-offs carefully. In this case the price of lock freedom is that you have to create instances of objects that you are not going to use. This may be an acceptable price to pay since you know that by being lock free, there is a lower risk of deadlocks and also thread contention.

In this particular instance however, the semantics of a singleton are in essence to Create a single instance of an object, so I would much rather opt for Lazy<T> as @Centro has quoted in his answer.

Nevertheless, it still begs the question, when should we use Interlocked.CompareExchange? I liked your example, it is quite thought provoking and many people are very quick to diss it as wrong when it is not horribly wrong as @Blindy quotes.

It all boils down to whether you have calculated the tradeoffs and decided:

• How important is it that you produce one and only one instance?
• How important is it to be lock free?

As long as you are aware of the trade-offs and make it a conscious decision to create new objects for the benefit of being lock free, then your example could also be an acceptable answer.