Question

I came across this article discussing why the double-check locking paradigm is broken in Java. Is the paradigm valid for .NET (in particular, C#), if variables are declared volatile?

Was it helpful?

Solution

Implementing the Singleton Pattern in C# talks about this problem in the third version.

It says:

Making the instance variable volatile can make it work, as would explicit memory barrier calls, although in the latter case even experts can't agree exactly which barriers are required. I tend to try to avoid situations where experts don't agree what's right and what's wrong!

The author seems to imply that double locking is less likely to work than other strategies and thus should not be used.

OTHER TIPS

Double-checking locking now works in Java as well as C# (the Java memory model changed and this is one of the effects). However, you have to get it exactly right. If you mess things up even slightly, you may well end up losing the thread safety.

As other answers have stated, if you're implementing the singleton pattern there are much better ways to do it. Personally, if I'm in a situation where I have to choose between double-checked locking and "lock every time" code I'd go for locking every time until I'd got real evidence that it was causing a bottleneck. When it comes to threading, a simple and obviously-correct pattern is worth a lot.

.NET 4.0 has a new type: Lazy<T> that takes away any concern about getting the pattern wrong. It's part of the new Task Parallel Library.

See the MSDN Parallel Computing Dev Center: http://msdn.microsoft.com/en-us/concurrency/default.aspx

BTW, there's a backport (I believe it is unsupported) for .NET 3.5 SP1 available here.

Note than in Java (and most likely in .Net as well), double-checked locking for singleton initialization is completely unnecessary as well as broken. Since classes are not initialized until they're first used, the desired lazy initialization is already achieved by this;

private static Singleton instance = new Singleton();

Unless your Singleton class contains stuff like constants that may be accessed before a Singleton instance is first used, this is all you need to do.

I don't get why all people says that the double-check locking is bad pattern, but don't adapt the code to make it work correctly. In my opinion, this below code should work just fine.

If someone could tell me if this code suffer from the problem mentionned in Cameron's article, please do.

public sealed class Singleton {
    static Singleton instance = null;
    static readonly object padlock = new object();

    Singleton() {
    }

    public static Singleton Instance {
        get {
            if (instance != null) {
                return instance;
            }

            lock (padlock) {
                if (instance != null) {
                    return instance;
                }

                tempInstance = new Singleton();

                // initialize the object with data

                instance = tempInstance;
            }
            return instance;
        }
    }
}

I've gotten double-checked locking to work by using a boolean (i.e. using a primitive to avoid the lazy initialisation):

The singleton using boolean does not work. The order of operations as seen between different threads is not guaranteed unless you go through a memory barrier. In other words, as seen from a second thread, created = true may be executed before instance= new Singleton();

I don't quite understand why there are a bunch of implementation patterns on double-checked locking (apparently to work around compiler idiosyncrasies in various languages). The Wikipedia article on this topic shows the naive method and the possible ways to solve the problem, but none are as simple as this (in C#):

public class Foo
{
  static Foo _singleton = null;
  static object _singletonLock = new object();

  public static Foo Singleton
  {
    get
    {
      if ( _singleton == null )
        lock ( _singletonLock )
          if ( _singleton == null )
          {
            Foo foo = new Foo();

            // Do possibly lengthy initialization,
            // but make sure the initialization
            // chain doesn't invoke Foo.Singleton.
            foo.Initialize();

            // _singleton remains null until
            // object construction is done.
            _singleton = foo;
          }
      return _singleton;
    }
  }

In Java, you'd use synchronized() instead of lock(), but it's basically the same idea. If there's the possible inconsistency of when the singleton field gets assigned, then why not just use a locally scoped variable first and then assign the singleton field at the last possible moment before exiting the critical section? Am I missing something?

There's the argument by @michael-borgwardt that in C# and Java the static field only gets initialized once on first use, but that behavior is language specific. And I've used this pattern frequently for lazy initialization of a collection property (e.g. user.Sessions).

I've gotten double-checked locking to work by using a boolean (i.e. using a primitive to avoid the lazy initialisation):

private static Singleton instance;
private static boolean created;
public static Singleton getInstance() {
    if (!created) {
        synchronized (Singleton.class) {
            if (!created) {
                instance = new Singleton();
                created = true;
            }
        }
    }
    return instance;
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top