What does it mean that singleton DCL broken?

2019-09-07 02:45发布

问题:

After reading dozens of articles about DCL. I feel that I should not use this concept without volatile.If I will not lead this technique my code will not thread save and very very bad according one hundreed different reasons.
Recently I reread basics Happens Before and I have a bit another view
Lets research singleton code listing:

public class Singleton{
    private static Something instance = null;
    public static Singleton getInstance() {
        if (instance == null) { // point 1
            synchronized (Singleton.class) {
                if (instance == null)
                    instance = new Something();
            }
        }
        return instance;  //point 2
    }

}

we change instance only inside synchronized (Something.class) {

thus we will see actual value inside synchronized section which uses same monitor. It is our case.

Thus now I have suspicion that this is not effective but it thread safe.

Am I right?

Only one concern:

Can if (instance == null) { to see non null value before actual assignment instance = new Something();

But I still absolutely sure that this code doesn't allow to create 2 singleton instances

P.S.

I read a bit more and looks like if in point 1 we read non-null value, return instance at point 2 could return null;

回答1:

The problem in your example is not in possible creating of two instances. That's true that only one instance will be created. Real problem is that on multiple thread access to this method, other thread can start using partially constructed version of instance (1)(2).

So, instance variable should be definitely defined as volatile (which is missed in your code block), otherwise you should concern about "freshness" of this variable's value.

Because there is no locking if the field is already initialized, it is critical that the field be declared volatile (Item 66) [J. Bloch, "Effective Java", Item 71]

So:

private static volatile Something instance;

(BTW, explicitly assigning null value is redundant).

Why it doesn't work without "volatile" is good explained here:

The most obvious reason it doesn't work it that the writes that initialize the Helper object and the write to the helper field can be done or perceived out of order