Double checked locking Article

Question

I was reading this article about "Double-Checked locking" and out of the main topic of the article I was wondering why at some point of the article the author uses the next Idiom:

Listing 7. Attempting to solve the out-of-order write problem

public static Singleton getInstance()  
{
    if (instance == null)
    {
        synchronized(Singleton.class) {      //1
            Singleton inst = instance;         //2
            if (inst == null)
            {
                synchronized(Singleton.class) {  //3
                    inst = new Singleton();        //4
                }
                instance = inst;                 //5
            }
        }
    }
    return instance;
}

And my question is: Is there any reason to synchronize twice some code with the same lock? Have this any purpose it?

Many thanks in advance.

Latashialatch

There is a related question "Best Singleton Implementation in Java" – Ought

Answer 1

The point of locking twice was to attempt to prevent out-of-order writes. The memory model specifies where reorderings can occur, partly in terms of locks. The lock ensures that no writes (including any within the singleton constructor) appear to happen after the "instance = inst;" line.

However, to go deeper into the subject I'd recommend Bill Pugh's article. And then never attempt it :)

Vibes

But you might use the actually working Java 5 variant at the end of the article. – Blackfellow

@hstoerr: I personally wouldn't, unless I found it to be the actual bottleneck in some code. It's too easy to get it wrong. Everything has its place, of course, but it would be pretty rare for me to use it. – Vibes

Answer 2

The article refers to the pre-5.0 Java memory model (JMM). Under that model leaving a synchronised block forced writes out to main memory. So it appears to be an attempt to make sure that the Singleton object is pushed out before the reference to it. However, it doesn't quite work because the write to instance can be moved up into the block - the roach motel.

However, the pre-5.0 model was never correctly implemented. 1.4 should follow the 5.0 model. Classes are initialised lazily, so you might as well just write

public static final Singleton instance = new Singleton();

Or better, don't use singletons for they are evil.

Shirk

Answer 3

Jon Skeet is right: read Bill Pugh's article. The idiom that Hans uses is the precise form that won't work, and should not be used.

This is unsafe:

private static Singleton instance;

public static Singleton getInstance() {
  if (instance == null) {
    synchronized(Singleton.class) {
      if (instance == null) {
        instance = new Singleton();
      }
    }
  }
  return instance;
}

This is also unsafe:

public static Singleton getInstance()  
{
    if (instance == null)
    {
        synchronized(Singleton.class) {      //1
            Singleton inst = instance;         //2
            if (inst == null)
            {
                synchronized(Singleton.class) {  //3
                    inst = new Singleton();        //4
                }
                instance = inst;                 //5
            }
        }
    }
    return instance;
}

Don't do either of them, ever.

Instead, synchronise the whole method:

    public static synchronized Singleton getInstance() {
      if (instance == null) {
        instance = new Singleton();
      }
      return instance;
    }

Unless you're retrieving this object a zillion times a second the performance hit, in real terms, is negligible.

Pancho

Answer 4

I cover a bunch of this here:

http://tech.puredanger.com/2007/06/15/double-checked-locking/

Manners

Answer 5

Following the John Skeet Recommendation:

However, to go deeper into the subject I'd recommend Bill Pugh's article. And then never attempt it :)

And here is the key for the second sync block:

This code puts construction of the Helper object inside an inner synchronized block. The intuitive idea here is that there should be a memory barrier at the point where synchronization is released, and that should prevent the reordering of the initialization of the Helper object and the assignment to the field helper.

So basically, with the Inner sync block, we are trying to "cheat" the JMM creating the Instance inside the sync block, to force the JMM to execute that allocation before the sync block finished. But the problem here is that the JMM is heading us up and is moving the assigment that is before the sync block inside the sync block, moving our problem back to the beginnig.

This is what i understood from those articles, really interesting and once more thanks for the replies.

Latashialatch

Answer 6

All right, but the article said that

The code in Listing 7 doesn't work because of the current definition of the memory model. The Java Language Specification (JLS) demands that code within a synchronized block not be moved out of a synchronized block. However, it does not say that code not in a synchronized block cannot be moved into a synchronized block.

And also seems like the JVM makes the next translation to "pseudo-code" in ASM:

public static Singleton getInstance()
{
  if (instance == null)
  {
    synchronized(Singleton.class) {      //1
      Singleton inst = instance;         //2
      if (inst == null)
      {
        synchronized(Singleton.class) {  //3
          //inst = new Singleton();      //4
          instance = new Singleton();               
        }
        //instance = inst;               //5
      }
    }
  }
  return instance;
}

So far, the point of no writes after the "instance=inst" is not accomplished?

I will read now the article, thanks for the link.

Latashialatch

Answer 7

Since Java 5, you can make double-checked locking work by declaring the field volatile.

See http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html for a full explanation.

Chagres

Answer 8

Regarding this idiom there is a very advisable and clarifying article:

http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html?page=1

On the other hand, I think what dhighwayman.myopenid means is why the writer has put one synchronized block referring to the same class (synchronized(Singleton.class)) within another synchronized block referring to the same class. It may happen as a new instance (Singleton inst = instance;) is created within that block and to guarantee it to be thread-safe it's necessary to write another synchronized.

Otherwise, I can't see any sense.

Seagraves

Answer 9

See the Google Tech Talk on the Java Memory Model for a really nice introduction to the finer points of the JMM. Since it is missing here, I would also like to point out Jeremy Mansons blog 'Java Concurrency' esp. the post on Double Checked locking (anyone who is anything in the Java world seems to have an article on this :).

Ali

Answer 10

For Java 5 and better there is actually a doublechecked variant that can be better than synchronizing the whole accessor. This is also mentioned in the Double-Checked Locking Declaration :

class Foo {
    private volatile Helper helper = null;
    public Helper getHelper() {
        if (helper == null) {
            synchronized(this) {
                if (helper == null)
                    helper = new Helper();
            }
        }
        return helper;
    }
}

The key difference here is the use of volatile in the variable declaration - otherwise it does not work, and it does not work in Java 1.4 or less, anyway.

Abiogenetic