Does Java synchronized keyword flush the cache?

Question

Java 5 and above only. Assume a multiprocessor shared-memory computer (you're probably using one right now).

Here is a code for lazy initialization of a singleton:

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

Does instance have to be declared volatile in order to prevent the optimizer from rewriting getInstance() as follows (which would be correct in a sequential program):

public static MySingleton getInstance() {
  if (instance == null) {
    synchronized (MySingleton.class) {
      // instance must be null or we wouldn't be here  (WRONG!)
      instance = new MySingleton();
    }
  }
}

Assuming the optimizer does not rewrite the code, if instance is not declared volatile is it still guaranteed to be flushed to memory when the synchronized block is exited, and read from memory when the synchronized block is entered?

EDIT: I forgot to make getInstance() static. I don't think that changes the validity of the answers; you all knew what I meant.

Answer 1

Yes, instance should be declared volatile. Even then, it is advised not to use double-checked locking. It (or to be precise, the Java Memory Model) used to have a serious flaw which permitted publication of partially implemented objects. This has been fixed in Java5, still DCL is an obsolete idiom and there is no need to use it anymore - use the lazy initialization holder idiom instead.

From Java Concurrency in Practice, section 16.2:

The real problem with DCL is the assumption that the worst thing that can happen when reading a shared object reference without synchronization is to erroneously see a stale value (in this case, null); in that case the DCL idiom compensates for this risk by trying again with the lock held. But the worst case is actually considerably worse - it is possible to see a current value of the reference but stale values for the object's state, meaning that the object could be seen to be in an invalid or incorrect state.

Subsequent changes in the JMM (Java 5.0 and later) have enabled DCL to work if resource is made volatile, and the performance impact of this is small since volatile reads are usually only slightly more expensive than nonvolatile reads. However, this is an idiom whose utility has largely passed - the forces that motivated it (slow uncontended synchronization, slow JVM startup) are no longer in play, making it less effective as an optimization. The lazy initialization holder idiom offers the same benefits and is easier to understand.

Answer 2

Yes, instance needs to be volatile using double-checked locking in Java, because otherwise the initialization of MySingleton could expose a partially-constructed object to the rest of the system. It's also true that the threads will sync up when they reach the "synchronized" statement, but that's too late in this case.

Wikipedia and a couple other Stack Overflow questions have good discussions of "double-checked locking", so I'd advise reading up about it. I'd also advise not using it unless profiling shows a real need for performance in this particular code.