The famous double-checked locking technique in C#

Question

I saw in a mprss book this recommendation of singleton (partial code attached):

public static Singleton GetSingleton() 
{
    if (s_value != null) 
        return s_value;
    Monitor.Enter(s_lock); 
    if (s_value == null) 
    {
        Singleton temp = new Singleton();
        Interlocked.Exchange(ref s_value, temp);
    }
    Monitor.Exit(s_lock);
    return s_value;
}

---

We add two lines of code in the 2nd if statement block instead of just writing:

s_value = new Singleton();

this should handle a situation that a second thread enters the method and finds s_value != null, but not initialized.

---

My question is, can we just write at the 2nd if block instead:

{    
    Singleton temp = new Singleton();
    s_value = temp;  // instead of Interlocked.Exchange(ref s_value, temp);    
}

So now the function is:

public static Singleton GetSingleton() 
{      
    if (s_value != null)
        return s_value;

    Monitor.Enter(s_lock);   
    if (s_value == null)   
    { 
        Singleton temp = new Singleton();    
        s_value = temp; 
    }   
    Monitor.Exit(s_lock);   
    return s_value; 
} 

I guess not, because they don't use it.

Does anyone have any suggestions?

---

It is possible that svalue may contains uninitialized? svalue can be constructed just after temp was fully initialized (may i wrong). if i wrong can u point of an example it is wrong? may the compiler produce differenent code?

---

Answer 1

I'll post this not as a real answer but as an aside: if you're using .NET 4 you really should consider the Lazy<T> singleton pattern:

http://geekswithblogs.net/BlackRabbitCoder/archive/2010/05/19/c-system.lazylttgt-and-the-singleton-design-pattern.aspx

public class LazySingleton3
{
   // static holder for instance, need to use lambda to construct since constructor private
   private static readonly Lazy<LazySingleton3> _instance
       = new Lazy<LazySingleton3>(() => new LazySingleton3());

   // private to prevent direct instantiation.
   private LazySingleton3()
   {
   }

   // accessor for instance
   public static LazySingleton3 Instance
   {
       get
       {
           return _instance.Value;
       }
   }

}

Thread-safe, easy to read, and obvious: what's not to like?

Answer 2

While the assignment is atomic, it is also required that the memory is "synchronized" across the different cores/CPUs (full fence), or another core concurrently reading the value might get an outdated value (outside of the synchronization block). The Interlocked class does this for all its operations.

http://www.albahari.com/threading/part4.aspx

Edit: Wikipedia has some useful information about the issues with the double-locking pattern and where/when to use memory barriers for it to work.