Double-checked locking, NetBeans confuses me?

Question

I have a queston regarding double-checked locking. Consider this example:

public class Singleton {

     private static volatile Singleton instance = null;

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

As I have understood, the above code is the correct way to make a Singleton class.

However, NetBeans wants me to remove the outer if statement, so it would look like this:

public class Singleton {

     private static volatile Singleton instance = null;

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

The only differece between these two snippets is that in the second example, the code will always get into the synchronized block and in the first it will not. Why would I listen to NetBeans and remove the outer if statement? It should be better avoid the locking.

Disorganize

BTW: I would just use enum Singleton { INSTANCE } Its much much simpler. ;) – Collate

Answer 1

NetBeans's automatic hint system obviously isn't aware that it's possible to do double-checked locking correctly with volatile, as you've done, so it suggests full locking instead. Better safe than sorry. But you're right in this case, not NetBeans.

Burchfield

Actually, NetBeans is RIGHT ... if the code is run on a JVM prior to 5.0. – Lamblike

@Stephen C But even 1.5 has completed its End Of Service Life some time ago. Even the Sun 1.4 implementation supports the 1.5 JMM. Unless you are targeting Java ME, it's not right. – Lashondra

Changing memory model semantics between JVM versions is a pain, and surely one should aim for the worst reasonable case. Luckily 1.4 is essentially out of existence now (I hope!). – Burchfield

Answer 2

Most of the time, the singleton will be used, and doesn't cost much to create, so just make it simple:

public class Singleton {
    private static final Singleton INSTANCE = new Singleton();

    public static Singleton getInstance() {
        return INSTANCE;
    }
    ...
}

If you really want lazy instantiation, use a static inner class:

public class Singleton {
    public static Singleton getInstance() {
        return Holder.INSTANCE;
    }
    ...

    private static class Holder {
        private static final Singleton INSTANCE = new Singleton();
    }
}

Gest

Answer 3

Don't listen to NetBeans in this situation. Your first code sample is correct.

Cilurzo

Actually, NetBeans is RIGHT ... if the code is run on a JVM prior to 5.0. Refer to the Wikipedia article. – Lamblike

NetBeans should refer to Source Level. If source level is 1.4 or below, of course NetBeans is right. But if source level is 1.5/1.6/1.7 - NetBeans is NOT right. – Cilurzo

actually, I think it will be the level of the platform on which the code is actually executed. – Lamblike