Double checking for singleton seems to work just fine without getting broken , why and how?

Question

I have read that double checking mechanism for singleton is a failure, because of some memory model followed by JVM which makes the reference read not null even if the constructor has not got executed completely.

I tried testing the same by making an time taking operation inside the constructor in the below code but even then it seems to be working fine.

public class Singleton {
private static Singleton singleton;
private Integer i =  0;
private Singleton() {

    for(long j = 0; j<99999999; j++){
        double k = Math.random();
        k= k+1;
    }
    i = 10;
}

private static Singleton getSinglton() {
    if(singleton == null){
        synchronized (Singleton.class) {
            if(singleton == null){
                singleton = new Singleton();
            }
        }
    }
    return singleton;
}

public static void main(String[] args) {
    Runnable runnable1 = new Runnable() {

        @Override
        public void run() {
            Singleton singleton = Singleton.getSinglton();
            System.out.println(singleton.getI());
        }
    };
    Thread t1 = new Thread(runnable1);
    Thread t2 = new Thread(runnable1);
    Thread t3 = new Thread(runnable1);
    Thread t4 = new Thread(runnable1);
    Thread t5 = new Thread(runnable1);
    Thread t6 = new Thread(runnable1);
    t1.start();
    t2.start();
    t3.start();
    t4.start();
    t5.start();
    t6.start();
}

public void setI(Integer i) {
    this.i = i;
}

public Integer getI() {
    return i;
}

}

The result I get is 10 10 10 10 10 10

I was expecting few threads to read value 0 , instead of 10, but each time the value is correctly read as 10 , so it the issue solved in Java SE-1.6 as I am using the same ?

As usual, thread races are difficult to reproduce. For a broken DCL it's even harder than usual, as it's "almost correct" in some sense. You'd need to use volatile for a correct DCL implementation. (Or better, [IODH](http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom), if you just want a singleton). — kiheru, Oct 06 '13 at 12:29
You're going to have to run this many **many** more times of you want to force an error. Maybe tens of millions. I would just take brokenness of DCL as a fact and use `volatile`. — Boris the Spider, Oct 06 '13 at 12:32
@kiheru current best practice is to use an `enum` for a singleton. — Boris the Spider, Oct 06 '13 at 12:33
Quote from [Wikipedia](http://en.wikipedia.org/wiki/Double-checked_locking): `One of the dangers of using double-checked locking in J2SE 1.4 (and earlier versions) is that it will often appear to work: it is not easy to distinguish between a correct implementation of the technique and one that has subtle problems. Depending on the compiler, the interleaving of threads by the scheduler and the nature of other concurrent system activity, failures resulting from an incorrect implementation of double-checked locking may only occur intermittently. Reproducing the failures can be difficult.` — Srikanth Reddy Lingala, Oct 06 '13 at 12:36
Thanks friends for the replies. The IODH concept seams quite good. The problem with using enums for Singleton is that We cannot extend the singleton class from any other class if we want to, else its good. — nits.kk, Oct 06 '13 at 12:51

score 1 · Answer 1 · answered Oct 06 '13 at 13:02

Firstly, don't use DSL, not because it is broken, but because it is needlessly complicated.

enum Singleton {
    INSTANCE;
}

Not only is this much simpler, it is also faster as it doesn't need the checks your getInstance() does.

Secondly, DSL was fixed in the memory model in Java 5.0 nine years ago.

Lastly, even though the model was broken, there was no guarantee it would ever show for a particular version of Java. Only that it wasn't guaranteed to work on all versions of Java. The Sun version of Java tended to fix things not covered well by the JLS.

“…DSL was fixed in the memory model in Java 5.0 nine years ago”- saying it this way is wrong. — Holger, Oct 08 '13 at 08:49

score 0 · Answer 2 · answered Oct 08 '13 at 08:56

First, as Peter Lawrey has pointed out, there is no reason to use double-checked locking at all.

The other point is, that the Java Memory Model has been changed with Java 5 so that it is possible to fix double-checked locking code by either making the instance immutable with final instance fields or by declaring the singleton holder volatile.

Since you’re doing neither, your code is subject to possible data races, but that doesn’t imply that you will ever see that when testing. The reason for seeing uninitialized instances when accessing shared variables without proper synchronization is the reordering of reads and writes to heap variables. But the JVM does not reorder these operations just for fun. It does so when it thinks that it can improve performance. In your example code, that’s rather unlikely.

But don’t count on it…

Double checking for singleton seems to work just fine without getting broken , why and how?

2 Answers2