Non volatile double checked locking, is it possible?

Question

Here is my singleton class.

Static instance field is not volatile thus reordering/visibility problem arises. To solve it instance val field is made final. Since instance is properly constructed its clients should always see val field initialized if they see instance at all.

    static class Singleton {
    private static Singleton instance;
    private final String val;
    public Singleton() { this.val = "foo"; }

    public static Singleton getInstance() {
        if (instance == null)
            synchronized (Singleton.class) {
                if(instance == null) {
                    instance = new Singleton();
            }
        }
        return instance;
    }
    public String toString() { return "Singleton: " + val; }
}

However there is another problem - I've got two unprotected reads of "instance" field which can be(?) reordered so that client may get null instead of real value:

public static Singleton getInstance() {
    Singleton temp = instance;
    if (instance != null) return temp;
    else { /* init singleton and return instance*/ }
}

To workaround this I feel like I can introduce local variable:

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

This seem to solve the problem since there is only one unprotected read of value so nothing really evil should happen. But... I've just modified the program flow without(almost?) changing its single threaded semantics. Does this mean that compiler can just undo my workaround since this transformation is safe and there is no way to make this code working without establishing proper happens-before relationship with volatile?

Since Java 1.5 the problem of singletons implementation has been solved with enums: http://stackoverflow.com/a/8027815/32090 — Boris Pavlović, Aug 22 '14 at 09:02
The question is not really about singletons. Rather its about double checked locking itself. — podcherkLIfe, Aug 22 '14 at 09:11
These problems are incredibly hard to solve. That's why we should use abstract constructs that Java is offering from the `java.util.concurrent` package. — Boris Pavlović, Aug 22 '14 at 11:46

score 1 · Accepted Answer · answered Sep 02 '14 at 18:05

I’m not sure whether a reordering of reads of the same variable really may occur, but it’s guaranteed that local variables are unaffected by other thread’s activities. Even if such read reorderings do not happen, this guaranty is relevant for every variable which might be updated concurrently while you read it: if you read a value and store it into a local variable you can be sure that the value of the local variable does not suddenly change afterwards. Of course, if the value is a reference, that guaranty does not apply to the fields of the referenced object.

The relevant sentence can be found in the JLS §17.4.1:

Local variables (§14.4), formal method parameters (§8.4.1), and exception handler parameters (§14.20) are never shared between threads and are unaffected by the memory model.

So the answer is no, a compiler is not allowed to undo your workaround of introducing a local variable.

score 0 · Answer 2 · answered Aug 22 '14 at 09:17

The safest way to do lazy-initialisation singletons is to use another class to hold the single instance field and rely on the guarantees the Java language provides for class initialisation

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

  public Singleton getInstance() {
    return Holder.instance;
  }
}

The Holder class will only be initialised (and thus the instance created) the first time getInstance() is called.

score -1 · Answer 3 · answered Aug 22 '14 at 09:44

-1

I don't think you have an issue from the beginning.

You use synchronized(Singleton.class). Upon synchronized Java guarantees any read/write before this keyword are readily reflected into memory for involved variables. Since your Singleton instance is also declared at class level, any modification to it is readily visible from other class and is populated into main memory.

answered Aug 22 '14 at 09:44

Alex Suo

2,977
1
14
22

The issue is reading value on the first and on the last lines of the method. These readings are outside of synchronized block so no happens-before relationships is established between reading and writing. This is why double checked locking does not work without volatile keyword. Thus I'm pretty sure that the problem exists. – podcherkLIfe Aug 22 '14 at 10:01
Your first line is just a NULL check and the instance got initialized only once; so if that got readily visible to this thread, great you are fine; if not, you got into synchronized and still no issue. I don't see which line you refer as last line though. – Alex Suo Aug 22 '14 at 10:04
As far as I understand reordering rules nonvolatile reads can be reordered. Thats why it is possible that first read returns nonnull value(check fails) and second one returns null. To illustrate this I've put the second snippet of code - it demonstrates valid reordering (at least I think that it is valid) which breaks multithreaded application(first thread reads null from instance field, then second thread sets instance to nonnull, then first thread reads nonnull value from instance field and returns null value stored in temp). May be my reasoning is wrong so please feel free to correct me. – podcherkLIfe Aug 22 '14 at 10:13
Remember 2 points: 1. reordering doesn't happen for statements separated by synchronized(). 2. upon synchronized() all the involved vars are flushed into main memory and the values are ensured to be the latest. So your first null second null case never happens. – Alex Suo Aug 25 '14 at 01:33

Non volatile double checked locking, is it possible?

3 Answers3