Re: Another DCL-like approach, correct or broken?

From:

Lew <noone@invalid.com>

Newsgroups:

comp.lang.java.programmer

Date:

Wed, 06 Aug 2008 19:58:00 -0400

Message-ID:

<7oKdnbGJFvMVpwfVnZ2dnUVZ_hqdnZ2d@comcast.com>

Piotr Kobzda wrote:

[...]

OK, so let's make it a bit simpler:

public abstract class SingletonReference<T> {

    private static final class ValueHolder<T> {
        final T value;

        ValueHolder(final T value) {
            this.value = value;
        }
    }

    private ValueHolder<T> valueHolder;

    protected abstract T initialValue();

    public final T get() {
        if (valueHolder == null) {

This needs to be synchronized or another thread might slip in between the null
check and the critical section.

            synchronized (this) {
                if (valueHolder == null) {
                    valueHolder = new ValueHolder<T>(initialValue());
                }
            }
        }
        return valueHolder.value;
    }
}

Now, it's classic DCL idiom supported by final field initialization
semantics. No volatile field is used.

Do you think it's correct?

Clearly not.

What's wrong with the normal suggested solution to this idiom?

--
Lew