0

Below is some code from a C# book to show how Singleton pattern is constructed in multithreading:

internal sealed class Singleton {
   // s_lock is required for thread safety and having this object assumes that creating
   // the singleton object is more expensive than creating a System.Object object
   private static readonly Object s_lock = new Object();

   // This field will refer to the one Singleton object
   private static Singleton s_value = null; 

   // Private constructor prevents any code outside this class from creating an instance
   private Singleton() {
      // Code to initialize the one Singleton object goes here...
   }

   // Public, static method that returns the Singleton object (creating it if necessary)
   public static Singleton GetSingleton() {
      // If the Singleton was already created, just return it (this is fast)
      if (s_value != null) return s_value;

      Monitor.Enter(s_lock); // Not created, let 1 thread create it

      if (s_value == null) {
         // Still not created, create it
         Singleton temp = new Singleton();

         // Save the reference in s_value (see discussion for details)
         Volatile.Write(ref s_value, temp); 
      }
      Monitor.Exit(s_lock);

      // Return a reference to the one Singleton object
      return s_value;
   }
}

I get the idea why the code does:

Singleton temp = new Singleton();
Volatile.Write(ref s_value, temp);

instead of

s_value = new Singleton();

because the compiler can allocate memory for the Singleton, assign the reference into s_value, and then call the constructor. From a single thread's perspective, changing the order like this has no impact. But if after publishing the reference into s_value and before calling the constructor, another thread calls the GetSingleton method, then thread will see that s_value is not null and start to use the Singleton object, but its constructor has not finished executing yet.

But I don't understand why we have to use Volatile.Write, can't we do:

Singleton temp = new Singleton();
s_value = temp;

The compiler cannot reorder e.g execute s_value = temp first then execute Singleton temp = new Singleton(), because temp have to exist before s_value = temp?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • The point is not to prevent reordering of the line `new Singleton` (as you say, that cannot happen), the point is to prevent reordering of `if (s_value != null)`. It doesn't really help anyway, because you still have a race condition without the lock, and if you have a lock then you anyway have a memory barrier, so `Volatile` is not necessary – Charlieface Aug 25 '21 at 11:54
  • 1
    In .net you can avoid it because static constructor is guaranteed to be executed in thread-safe manner – JL0PD Aug 25 '21 at 11:55
  • 2
    Another thing wrong here is that `Monitor.Enter` and `Monitor.Exit` should be in `try/finally`, or better, just use `lock(` like you're uspposed to – Charlieface Aug 25 '21 at 11:56
  • 1
    Whatever you do, don't use this book for guidance on how to implement singletons, because 1) singletons are evil to start with, and should only be considered if there are no better creation patterns to solve things, 2) if you *must* have singletons, a simple `static readonly Singleton = new Singleton()` will usually suffice, with locking guaranteed by the framework, 3) if you *must* have a thread-safe, lazily-initialized singleton, .NET 4 introduced `Lazy`, so there's no motivation to roll your own with all the ways to get it wrong. – Jeroen Mostert Aug 25 '21 at 12:54
  • Beware of [double-checked locking](https://en.wikipedia.org/wiki/Double-checked_locking) *"The pattern, when implemented in some language/hardware combinations, can be unsafe. At times, it can be considered an anti-pattern."* Most sane people would avoid messing around with techniques that require detailed knowledge of [memory models](https://learn.microsoft.com/en-us/archive/msdn-magazine/2012/december/csharp-the-csharp-memory-model-in-theory-and-practice), [cache coherency protocols](https://stackoverflow.com/a/66490395/11178549), and similar lovely stuff. – Theodor Zoulias Aug 25 '21 at 21:33

1 Answers1

2

This code is from CLR via C# by Jeffrey Richter.

The explanation from the author in the book (as referred to in the 'see discussion for details' comment) is that Volatile.Write:

ensures that the reference in temp can be published into s_value only after the constructor has finished executing.

Chris Brumme wrote in 2003 about an identical C# double-check pattern (variable names changed):

It works just fine on X86. But it would be broken by a legal but weak implementation of the ECMA CLI spec.

Assume that a series of stores have taken place during construction of [Singleton]. Those stores can be arbitrarily reordered, including the possibility of delaying them until after the publishing store which assigns the new object to [s_value]. At that point, there is a small window before the store.release implied by leaving the lock. Inside that window, other CPUs can navigate through the reference [s_value] and see a partially constructed instance.

So the Volatile.Write is only required when coding for a (theoretical) weakest possible implementation of the ECMA CLI standard memory model.

N.B. that the CLI standard calls out the need for barriers in this case (12.6.8):

It is explicitly not a requirement that a conforming implementation of the CLI guarantee that all state updates performed within a constructor be uniformly visible before the constructor completes. CIL generators may ensure this requirement themselves by inserting appropriate calls to the memory barrier or volatile write instructions.

I've not been able practically demonstrate this problem (visibility of a partially constructed instance due to reordering) with current versions of .net on x64 / ARM64 but I've not tried very hard.

TLDR: writing this sort of code is complicated, use Lazy<T>

Peter Wishart
  • 11,600
  • 1
  • 26
  • 45