views:

112

answers:

5

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.

+1  A: 

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.

samkass
(from javaworld article) A commonly suggested nonfix is to declare the resource field of SomeClass as volatile. However, while the JMM prevents writes to volatile variables from being reordered with respect to one another and ensures that they are flushed to main memory immediately, it still permits reads and writes of volatile variables to be reordered with respect to nonvolatile reads and writes. That means -- unless all Resource fields are volatile as well -- thread B can still perceive the constructor's effect as happening after resource is set to reference the newly created Resource.
gawi
That was fixed in Java5, although it's worth pointing out that in older VMs even volatile won't fix the problem.
samkass
+10  A: 

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.

Péter Török
+1 Very interesting. I'm always outsmarted by stackoverflower. This is a never-ending humility exercise.
gawi
If Edsger Dijkstra were still with us, he would probably remark that if expert programmers are always outsmarted by stackoverflow, the profession is in a bad state.
Mark Lutton
the holder class idiom only works for singletons. not a general mechanism of lazy initialization.
irreputable
@irrep, true, however this question is about singletons, not lazy init in general.
Péter Török
+1  A: 

There's a safer and more readable idiom for lazy initialization of Java singletons:

class Singleton {

  private static class Holder {
    static final Singleton instance = create();
  }

  static Singleton getInstance() { 
    return Holder.instance; 
  }

  private Singleton create() {
    ⋮
  }

  private Singleton() { }

}

If you use the more verbose Double-Checked Locking pattern, you have to declare the field volatile, as other have already noted.

erickson
A: 

No. It doesn't have to be volatile. See http://stackoverflow.com/questions/3578604/how-to-solve-the-double-checked-locking-is-broken-declaration-in-java/3578674#3578674

previous attempts all failed because if you can cheat java and avoid volatile/sync, java can cheat you and give you an incorrect view of the object. however the new final semantics solves the problem. if you get an object reference (by ordinary read), you can safely read its final fields.

irreputable
Still, the lazy initialization holder idiom achieves the same, in a clearer way, with less code (thus less chance for errors) and without any need for explicit synchronization.
Péter Török
that only works for singletons. not for lazily creating dynamic number of objects at runtime.
irreputable
This question is expressly regarding singletons.
erickson