Is double check with final variable working

Question

I have class designed for lazy initialization and storing objects which creation is not necessary threadsafe. Here is the code:

class SyncTest {
    private static final Object NOT_INITIALIZED = new Object();
    private Object object;

    /**
     * It's guaranteed by outer code that creation of this object is thread safe
     * */
    public SyncTest() {
       object = NOT_INITIALIZED;
    }

    public Object getObject() {
        if (object == NOT_INITIALIZED) {
            synchronized (NOT_INITIALIZED) {
                if (object == NOT_INITIALIZED) {
                    final Object tmpRef = createObject();
                    object = tmpRef;
                }
            }
        }
        return object;
    }

    /**
     * Creates some object which initialization is not thread safe
     * @return required object or NOT_INITIALIZED
     * */
    private Object createObject() {
        //do some work here
    }
}

Here final variable tmpRef is used for storing created object before assigning it to checked variable object . This works in tests but I can't say it's correct for sure and won't be optimsed by compiler. Can this appraoch be used or object field must be declared as volatile?

Also variant with wrapper class was considered where the line

final Object tmpRef = createObject();

must be replaced with this one:

Object tmpRef = new FinalWrapper(createObject()).getVal();

Wrapper class looks like this:

private class FinalWrapper {
    private final Object val;

    public FinalWrapper(Object val) {
        this.val = val;
    }

    public Object getVal() {
        return val;
    }
}

Can some of this examples be safely used in multithreaded environment (especially variant with final local field)?

I still haven't drunk my first coffee, but `private volatile Object object` should do. The drawback of this solution is that several instances synchronize on the same static mutex. — Joop Eggen, Aug 17 '15 at 08:09
If you are trying to create a lazy initialise singleton, I suggest you just use a `enum` It's a lot less code and it works. — Peter Lawrey, Aug 17 '15 at 08:16
Examples with volatile and wrapper class are taken from Joshua Bloch `Effective Java` book. Main part is final local field. It's guranteed by JMM that calss initialization with only final fields is always threadsafe. Is the same applicable for local final fields? — user3007501, Aug 17 '15 at 08:17
Enum is not best way because the same class must be used for storing many different objects wich types are determined in realtime — user3007501, Aug 17 '15 at 08:26

Marko Topolnik · Accepted Answer · 2015-08-17T08:43:28.950

2

object = NOT_INITIALIZED;

If you envision this as a trick which will avoid the problems of the usual lazy singleton where you would simply have

object = null;

then it is incorrect; your trick didn't win you any thread safety. You cannot beat the standard double-checked idiom with the volatile variable pointing to the lazily initialized object. So my suggestion would be to get rid of the extra complexity, use null, and use volatile.

Answering your comments:

It's guranteed by JMM that calss initialization with only final fields is always threadsafe.

Class initialization is always thread-safe, regardless of the kind of fields. Every usage of the class is guaranteed to see the objects referred by static fields at least as up-to-date as they were at the time that all class init code completed.

Is the same applicable for local final fields?

The object reached by dereferencing a final field will be at least as up-to-date as it was at the time the constructor of the object containing the final field completed. However, in your solution you never even dereference the field, you just check its value. It is strictly equivalent to check for equality to null as to the value of the NOT_INITIALIZED constant.

edited Aug 17 '15 at 08:43

answered Aug 17 '15 at 08:29

Marko Topolnik

195,646
29
319
436

About class initalization it was mistake, class instance initialization was meant there. As I know using wrapper class like the FinalWrapper in the example can be used to read objects wich are safely initialized even if the object itself is not designed for safe initialization. I've tried to replace final instance variable in FieldWrapper class with final local variable in the block of code, but not sure if it has same functions as instance final field. Using NOT_INITIALIZED object is just an example. Null check was originally there. Key point is final local field, can it replace volatile? – user3007501 Aug 17 '15 at 09:01
Ok, no sure I understand the situation you described above. If you can post here some links about this or give an example how exactly the issue can be reproduced that'd be great – user3007501 Aug 17 '15 at 18:38
There is no reliable reproduction of issues due to data races. You may even never experience it on any given JVM implementation and hardware platform. This is why it is so essential to write code which is correct with respect to the JMM and resist the urge ti outsmart it. – Marko Topolnik Aug 17 '15 at 18:56
Knowing situation which'll cause the issue, maybe you could show a sequence of states which'll break shown implementation to clarify this? – user3007501 Aug 17 '15 at 22:32
If you haven't done so yet, I recommend you carefully study [Section 17.4](https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.4) of the Java Language Specification, especially everything up to and including 17.4.5. – Marko Topolnik Aug 18 '15 at 08:01
Thanks for the info. Also I've searched some additional info. I'm tending to accept this [answer](http://stackoverflow.com/a/30048680/3007501) which shows that omitting volatile is possible if final wrapper is used. Do you agree with the author of this answer? If so, this discussion exhausted itself and should be closed. – user3007501 Aug 18 '15 at 08:17
In that answer Alex repeats all the points I have tried to drive home in this discussion: 1. use `volatile`; 2. `FinalWrapper` is correct, but doesn't bring any benefit, just complication; 3. if all your singleton's fields are final, you don't need `FinalWrapper`; 4. use `volatile`. I also agree that this discussion has been exhausted. – Marko Topolnik Aug 18 '15 at 08:22

score 1 · Answer 2 · answered Aug 17 '15 at 08:09

You should mark the object variable as volatile to guarantee thread safety, also note that this pattern is only safe in Java 1.5 and later.

This is a tricky piece of code, to quote Joshua Bloch:

The idiom is very fast but also complicated and delicate, so don't be tempted to modify it in any way. Just copy and paste -- normally not a good idea, but appropriate here

Is double check with final variable working

2 Answers2