Still Double Checks !!!

Singleton, though easy on paper, is one of the most difficult patters to implement in Java. I recenty came across this code for an "efficient thread safe singleton" in a tech forum .

public class ClassicSingleton {
private static ClassicSingleton instance = null;

private ClassicSingleton() {
// make ur constructor private to prevent the instantiation
}

public static ClassicSingleton getInstance() {
if (instance == null) {

//Allow only one thread to access
//Incase of two threads accessing exactly at same time.
//Thread 1 will get the lock and Thread 2 will wait.

synchronized (ClassicSingleton.class) {
//check one more time
//For thread 1 instance is null and it will create the object.
//When Thread 2 will enter the condition will be false as thread 1 has already created the object

if (instance == null) {
instance = new ClassicSingleton();
}
}
}

return instance;
}
}


The above code is not correct .Its exhibits an anti-pattern/ idiom commonly called "Double Checked Locking" DCL for short.The issue is that DCL is broken in java. This has to with the java memory model. Though it got revised in JDK 5.0 DCL is still not supported, even with the java.util.concurrent.* abstractions.

The first and simplest reason it does not work is that the writes that initialize the 'ClassicSingleton' object and the write to the 'instance' field can be done or perceived out of order. Thus, a thread which invokes getInstance() could see a non-null reference to a 'ClassicSingleton' object, but see the default values for fields of the 'ClassicSingleton' object, rather than the values set in the constructor. If the compiler inlines the call to the constructor, then the writes that initialize the object and the write to the 'instance' field can be freely reordered if the compiler can prove that the constructor cannot throw an exception or perform synchronization.
So the after the first thread exits and the second thread runs the second will see that the instance is not null as expected, but the intance it gets may be having its fields initialized improperly, since there is no guarantee that the constructor run has compled the writes that initializes the fields (if any).
The most saddeing fact is that this sort of re-ordering is completely legal as per the spec for the Java memory model.
Doug Lea is an authority on compiler re-orderings and has written an excellent article here http://gee.cs.oswego.edu/dl/cpj/jmm.html

As for the solution, the best way would be to synchonize the entire method(ie synchronized getInstance()), while this has the penalty that sysnchronization will be used every time the method is invoked.(While its absolutely necessary only for the first call.after that the null check would always return false)