Java double checked locking

Question

Double check locking is broken. Since initialized is a primitive, it may not require it to be volatile to work, however nothing prevents initialized being seen as true to the non-syncronized code before instance is initialized.

EDIT: To clarify the above answer, the original question asked about using a boolean to control the double check locking. Without the solutions in the link above, it will not work. You could double check lock actually setting a boolean, but you still have issues about instruction reordering when it comes to creating the class instance. The suggested solution does not work because instance may not be initialized after you see the initialized boolean as true in the non-syncronized block.

The proper solution to double-check locking is to either use volatile (on the instance field) and forget about the initialized boolean, and be sure to be using JDK 1.5 or greater, or initialize it in a final field, as elaborated in the linked article and Tom's answer, or just don't use it.

Certainly the whole concept seems like a huge premature optimization unless you know you are going to get a ton of thread contention on getting this Singleton, or you have profiled the application and have seen this to be a hot spot.

That would work if initialized was volatile. Just as with synchronized the interesting effects of volatile are not really so much to do with the reference as what we can say about other data. Setting up of the instance field and the Test object is forced to happen-before the write to initialized. When using the cached value through the short circuit, the initialize read happens-before reading of instance and objects reached through the reference. There is no significant difference in having a separate initialized flag (other than it causes even more complexity in the code).

(The rules for final fields in constructors for unsafe publication are a little different.)

However, you should rarely see the bug in this case. The chances of getting into trouble when using for the first time is minimal, and it is a non-repeated race.

The code is over-complicated. You could just write it as:

private static final Test instance = new Test();

public static Test getInstance() {
    return instance;
}

Double checked locking is indeed broken, and the solution to the problem is actually simpler to implement code-wise than this idiom - just use a static initializer.

public class Test {
    private static final Test instance = createInstance();

    private static Test createInstance() {
        // construction logic goes here...
        return new Test();
    }

    public static Test getInstance() {
        return instance;
    }
}

A static initializer is guaranteed to be executed the first time that the JVM loads the class, and before the class reference can be returned to any thread - making it inherently threadsafe.

This is the reason why double checked locking is broken.

Synchronize guarantees, that only one thread can enter a block of code. But it doesn't guarantee, that variables modifications done within synchronized section will be visible to other threads. Only the threads that enters the synchronized block is guaranteed to see the changes. This is the reason why double checked locking is broken - it is not synchronized on the reader's side. The reading thread may see, that the singleton is not null, but singleton data may not be fully initialized (visible).

Ordering is provided by volatile. volatile guarantees ordering, for instance write to volatile singleton static field guarantees that writes to the singleton object will be finished before the write to volatile static field. It doesn't prevent creating singleton of two objects, this is provided by synchronize.

Class final static fields doesn't need to be volatile. In Java, the JVM takes care of this problem.

See my post, an answer to Singleton pattern and broken double checked locking in a real-world Java application, illustrating an example of a singleton with respect to double-checked locking that looks clever but is broken.

Double-checked Locking is the anti-pattern.

Lazy Initialization Holder Class is the pattern you should be looking at.

Despite so many other answers, I figured I should answer because there still isn't one simple answer that says why DCL is broken in many contexts, why it is unnecessary and what you should do instead. So I'll use a quote from Goetz: Java Concurrency In Practice which for me provides the most succint explanation in its final chapter on the Java Memory Model.

It's about Safe Publication of variables:

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.

Listing 16.6. Lazy Initialization Holder Class Idiom.

public class ResourceFactory
    private static class ResourceHolder {
        public static Resource resource = new Resource();
    }

    public static Resource getResource() {
        return ResourceHolder.resource;
    }
}

That's the way to do it.