Re: Another DCL-like approach, correct or broken?
Piotr Kobzda wrote:
Lew wrote:
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.
What's the problem with that? Why should one thread care if other
thread is sleeping or not in between?
AFAIK, the only problems with old JMM here I recall, was ability to read
unsynchronized variable too early, i.e. without all fields of this
variable fully initialized.
But now, thanks to the final field semantics, it seems that we don't
have to care about this problem:
"A thread that can only see a reference to an object after that object
has been completely initialized is guaranteed to see the correctly
initialized values for that object's final fields." [JLS3 17.5]
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.
I'm not as certain of that as you are.
"If Helper is an immutable object, such that all of the fields of Helper
are final, then double-checked locking will work without having to use
volatile fields." [the Declaration mentioned earlier]
"Final fields also allow programmers to implement thread-safe immutable
objects without synchronization. A thread-safe immutable object is seen
as immutable by all threads, even if a data race is used to pass
references to the immutable object between threads. This can provide
safety guarantees against misuse of an immutable class by incorrect or
malicious code. Final fields must be used correctly to provide a
guarantee of immutability." [JLS3 17.5]
I see what you mean, but even in the case of the primitive variable, to which
the article compares the use of an immutable object ("should behave in much
the same way as an int or float"), you will note that the check for whether
the variable is 0 ('null') is inside the critical section, not outside as you
coded it.
What's wrong with the normal suggested solution to this idiom?
What is the normal suggested solution you think of?
Google on Brian Goetz and "double-checked locking".
Your example 'get()' method is exactly parallel to the example of what does
not work in the article you cited. That article explains why the 'Helper', or
in your case, the 'ValueHolder', needs to be 'volatile', or else checked for
'null' inside the critical section the way primitives are checked for 0.
You did neither.
Again, from the article you quoted - note that the check for whether
'cachedHashCode' is zero is inside the critical section:
synchronized(this) {
if (cachedHashCode != 0) return cachedHashCode;
h = computeHashCode();
cachedHashCode = h;
}
To use an immutable object similarly, as the article suggests, you also have
to move the check for 'null' inside the critical section.
--
Lew