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

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.

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.

In order not to use 'double-checked locking mambo-jambo' or simply not to implement an own singleton reinventing the wheel, use a ready solution included into .NET 4.0 - Lazy<T>.

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

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

I am not convinced you can completely trust that. Yes, Interlocked.CompareExchanger is atomic, but new Singleton() is in not going to be atomic in any non-trivial case. Since it would have to evaluated before exchanging values, this would not be a thread-safe implementation in general.

what about this?

public sealed class Singleton
{
    Singleton()
    {
    }

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

    class Nested
    {
        // Explicit static constructor to tell C# compiler
        // not to mark type as beforefieldinit
        static Nested()
        {
        }

        internal static readonly Singleton instance = new Singleton();
    }
}

It's the fifth version on this page: http://www.yoda.arachsys.com/csharp/singleton.html

I'm not sure, but the author seems to think its both thread-safe and lazy loading.

Your singleton initializer is behaving exactly as it should. See Raymond Chen's Lock-free algorithms: The singleton constructor:

This is a double-check lock, but without the locking. Instead of taking lock when doing the initial construction, we just let it be a free-for-all over who gets to create the object. If five threads all reach this code at the same time, sure, let's create five objects. After everybody creates what they think is the winning object, they called InterlockedCompareExchangePointerRelease to attempt to update the global pointer.

This technique is suitable when it's okay to let multiple threads try to create the singleton (and have all the losers destroy their copy). If creating the singleton is expensive or has unwanted side-effects, then you don't want to use the free-for-all algorithm.

Each thread creates the object; as it thinks nobody has created it yet. But then during the InterlockedCompareExchange, only one thread will really be able to set the global singleton.

Bonus reading

One-Time Initialization helper functions save you from having to write all this code yourself. They deal with all the synchronization and memory barrier issues, and support both the one-person-gets-to-initialize and the free-for-all-initialization models.
A lazy initialization primitive for .NET provides a C# version of the same.

This is not thread-safe.

You would need a lock to hold the if() and the Interlocked.CompareExchange() together, and then you wouldn't need the CompareExchange anymore.

You still have the issue that you're quite possibly creating and throwing away instances of your singleton. When you execute Interlocked.CompareExchange(), the Singleton constructor will always be executed, regardless of whether the assignment will succeed. So you're no better off (or worse off, IMHO) than if you said:

if ( _instance == null )
{
  lock(latch)
  {
    _instance = new Singleton() ;
  }
}

Better performance vis-a-vis thread contention than if you swapped the position of the lock and the test for null, but at the risk of an extra instance being constructed.

An obvious singleton implementation for .NET?

Auto-Property initialization (C# 6.0) does not seem to cause the multiple instantiations of Singleton you are seeing.

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

I think the simplest way after .NET 4.0 is using System.Lazy<T>:

public class Singleton
{
    private static readonly Lazy<Singleton> lazy = new Lazy<Singleton>(() => new Singleton());

    public static Singleton Instance { get { return lazy.Value; } }

    private Singleton() { }
}

Jon Skeet has a nice article here that covers a lot of ways of implementing singleton and the problems of each one.

Don't use locking. Use your language environment

Mostly simple Thread-safe implementation is:

public class Singleton
{
    private static readonly Singleton _instance;

    private Singleton() { }

    static Singleton()
    {
        _instance = new Singleton();
    }

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