Singleton Pattern in Multi threaded environment

Question

During my interview, interviewer started his question with singleton pattern. I wrote below. Then, he asked Shouldn't we check for Nullity inside getInstance method?

I replied with, It is NOT necessary, since member is static type and is being initialized at the same time. But, seems like he was not satisfied with my answer.Am I correct or not ?

class Single {

        private final static Single sing = new Single();       
        private Single() {
        }        
        public static Single getInstance() {
            return sing;
        }
    }

Now, next question he ask to write singleton class for multi-threaded environment. Then, I wrote double check singleton class.

  class MultithreadedSingle {        
        private static MultithreadedSingle single;       
        private MultithreadedSingle() {
        }        
        public static MultithreadedSingle getInstance() {
            if(single==null){
                    synchronized(MultithreadedSingle.class){
                      if(single==null){
                            single= new MultithreadedSingle(); 
                              }      
                      }
                   }
             return single;
        }
    }

Then, he had an objection with using synchronized and double check and said It is useless. Why are you checking twice and why are you using synchronized ? I tried to convince him with multiple scenario. But, he didn't.

Later, at home I tried below code where I'm using simple singleton class with multiple thread.

public class Test {

    public static void main(String ar[]) {
        Test1 t = new Test1();
        Test1 t2 = new Test1();
        Test1 t3 = new Test1();
        Thread tt = new Thread(t);
        Thread tt2 = new Thread(t2);
        Thread tt3 = new Thread(t3);
        Thread tt4 = new Thread(t);
        Thread tt5 = new Thread(t);
        tt.start();
        tt2.start();
        tt3.start();
        tt4.start();
        tt5.start();

    }
}

final class Test1 implements Runnable {

    @Override
    public void run() {
        for (int i = 0; i < 5; i++) {
            System.out.println(Thread.currentThread().getName() + " : " + Single.getInstance().hashCode());
        }
    }

}
     class Single {

        private final static Single sing = new Single();       
        private Single() {
        }        
        public static Single getInstance() {
            return sing;
        }
    }

Below is the output :

Thread-0 : 1153093538
Thread-0 : 1153093538
Thread-0 : 1153093538
Thread-0 : 1153093538
Thread-0 : 1153093538
Thread-4 : 1153093538
Thread-1 : 1153093538
Thread-2 : 1153093538
Thread-3 : 1153093538
Thread-3 : 1153093538
Thread-3 : 1153093538
Thread-3 : 1153093538
Thread-3 : 1153093538
Thread-2 : 1153093538
Thread-2 : 1153093538
Thread-2 : 1153093538
Thread-2 : 1153093538
Thread-1 : 1153093538
Thread-1 : 1153093538
Thread-1 : 1153093538
Thread-1 : 1153093538
Thread-4 : 1153093538
Thread-4 : 1153093538
Thread-4 : 1153093538
Thread-4 : 1153093538

So, question is, Is it necessary to use synchronize or/and double check method in multi-threaded environment ? It seems like my first code itself (without adding any extra line of code) was the answer for both question. Any correction and knowledge share will be appreciated.

Answer 1

In case #2 add 'volatile' keyword to the static field 'single'.

Consider this scenario when using Double-Checked Locking

1. Thread A comes in first and gets the lock and proceeds to initialize the object.
2. According to Java Memory Model (JMM), memory is allocated for a variable and published before initializing the Java Object.
3. Thread B comes in and because of the double checked locking, and the variable is initialized, it does not acquire the lock.
4. This does not guarantee that the object is initialized and even if so, the per-cpu cache may not be updated. Refer Cache Coherence

Now coming to the volatile keyword.

Volatile variables are always written into the main memory. Hence no cache incoherence.

Answer 2

according to Double-checked_locking, its probably the best way

class Foo {
    private volatile Helper helper;
    public Helper getHelper() {
        Helper result = helper;
        if (result == null) {
            synchronized(this) {
                result = helper;
                if (result == null) {
                    helper = result = new Helper();
                }
            }
        }
        return result;
    }
}

or using the Initialization-on-demand holder idiom

public class Something {
    private Something() {}

    private static class LazyHolder {
        private static final Something INSTANCE = new Something();
    }

    public static Something getInstance() {
        return LazyHolder.INSTANCE;
    }
}

Answer 3

1) Class #1 is good for multithreaded environment

2) Class #2 is a singleton with lazy initialization and double checked locking, it's a known pattern and it needs to use synchronization. But your implementation is broken, it needs volatile on the field. You can find out why in this article http://www.javaworld.com/article/2074979/java-concurrency/double-checked-locking--clever--but-broken.html

3) a Singleton with one method does not need to use lazy pattern, because its class will be loaded and initialized only at first usage.

Answer 4

Your first answer seems to be good for me, as there are no chance of a race condition whatsoever.

As for knowledge share, the best approach to implement a singleton in Java is using Enum. Create an enum with exactly one instance, and that's it. As for code sample -

public enum MyEnum {
    INSTANCE;

    // your other methods
}

From the good book Effective Java -

[....] This approach is functionally equivalent to the public field approach, except that it is much more concise, provides the serialization machinery for free, and provides an ironclad guarantee against multiple instantiation, even in the face of sophisticated serialization or reflection attacks.[...] a single-element enum type is the best way to implement a singleton.

Answer 5

Your first example is absolutely correct, and is usually the preferred "idiom" for singletons. The other one is to make a single-element enum:

public enum Single {
    INSTANCE;

    ...
}

The two approaches are pretty similar unless the class is Serializable, in which case the enum approach is much easier to get right -- but if the class isn't Serializable, I actually prefer your approach the enum one, as a stylistic matter. Watch out for "accidentally" becoming Serializable due to implementing an interface or extending a class which is itself Serializable.

You are also right about the second check for nullity in the double-checked lock example. However, the sing field must be volatile for this to work in Java; otherwise, there is no formal "happens-before" edge between one thread writing to sing and another thread reading to it. This can result in that second thread seeing null even though the first thread assigned to the variable, or, if the sing instance has state, it could even result in that second thread seeing only some of that state (seeing a partially-constructed object).