Double-checked locking without volatile
I read this question about how to do Double-checked locking:
// Double-check idiom for lazy initialization of instance fields
private volatile FieldType field;
FieldType getField() {
FieldType result = field;
if (result == null) { // First check (no locking)
synchronized(this) {
result = field;
if (result == null) // Second check (with locking)
field = result = computeFieldValue();
}
}
return result;
}
My aim is to get lazy-loading a field (NOT a singleton) work without the volatile attribute. The field object is never changed after initialization.
After some testing my final approach:
private FieldType field;
FieldType getField() {
if (field == null) {
synchronized(this) {
if (field == null)
field = Publisher.publish(computeFieldValue());
}
}
return fieldHolder.field;
}
public class Publisher {
public static <T> T publish(T val){
return new Publish<T>(val).get();
}
private static class Publish<T>{
private final T val;
public Publish(T val) {
this.val = val;
}
public T get(){
return val;
}
}
}
The benefit is possibly faster access time due to not needing volatile, while still keeping the simplicity with the reusable Publisher class.
I tested this using jcstress. SafeDCLFinal worked as expected while UnsafeDCLFinal was inconsistent (as expected). At this point im 99% sure it works, but please, prove me wrong. Compiled with mvn clean install -pl tests-custom -am and run with java -XX:-UseCompressedOops -jar tests-custom/target/jcstress.jar -t DCLFinal. Testing code below (mostly modified singleton testing classes):
/*
* SafeDCLFinal.java:
*/
package org.openjdk.jcstress.tests.singletons;
public class SafeDCLFinal {
@JCStressTest
@JCStressMeta(GradingSafe.class)
public static class Unsafe {
@Actor
public final void actor1(SafeDCLFinalFactory s) {
s.getInstance(SingletonUnsafe::new);
}
@Actor
public final void actor2(SafeDCLFinalFactory s, IntResult1 r) {
r.r1 = Singleton.map(s.getInstance(SingletonUnsafe::new));
}
}
@JCStressTest
@JCStressMeta(GradingSafe.class)
public static class Safe {
@Actor
public final void actor1(SafeDCLFinalFactory s) {
s.getInstance(SingletonSafe::new);
}
@Actor
public final void actor2(SafeDCLFinalFactory s, IntResult1 r) {
r.r1 = Singleton.map(s.getInstance(SingletonSafe::new));
}
}
@State
public static class SafeDCLFinalFactory {
private Singleton instance; // specifically non-volatile
public Singleton getInstance(Supplier<Singleton> s) {
if (instance == null) {
synchronized (this) {
if (instance == null) {
// instance = s.get();
instance = Publisher.publish(s.get(), true);
}
}
}
return instance;
}
}
}
/*
* UnsafeDCLFinal.java:
*/
package org.openjdk.jcstress.tests.singletons;
public class UnsafeDCLFinal {
@JCStressTest
@JCStressMeta(GradingUnsafe.class)
public static class Unsafe {
@Actor
public final void actor1(UnsafeDCLFinalFactory s) {
s.getInstance(SingletonUnsafe::new);
}
@Actor
public final void actor2(UnsafeDCLFinalFactory s, IntResult1 r) {
r.r1 = Singleton.map(s.getInstance(SingletonUnsafe::new));
}
}
@JCStressTest
@JCStressMeta(GradingUnsafe.class)
public static class Safe {
@Actor
public final void actor1(UnsafeDCLFinalFactory s) {
s.getInstance(SingletonSafe::new);
}
@Actor
public final void actor2(UnsafeDCLFinalFactory s, IntResult1 r) {
r.r1 = Singleton.map(s.getInstance(SingletonSafe::new));
}
}
@State
public static class UnsafeDCLFinalFactory {
private Singleton instance; // specifically non-volatile
public Singleton getInstance(Supplier<Singleton> s) {
if (instance == null) {
synchronized (this) {
if (instance == null) {
// instance = s.get();
instance = Publisher.publish(s.get(), false);
}
}
}
return instance;
}
}
}
/*
* Publisher.java:
*/
package org.openjdk.jcstress.tests.singletons;
public class Publisher {
public static <T> T publish(T val, boolean safe){
if(safe){
return new SafePublish<T>(val).get();
}
return new UnsafePublish<T>(val).get();
}
private static class UnsafePublish<T>{
T val;
public UnsafePublish(T val) {
this.val = val;
}
public T get(){
return val;
}
}
private static class SafePublish<T>{
final T val;
public SafePublish(T val) {
this.val = val;
}
public T get(){
return val;
}
}
}
Tested with java 8, but should work at least with java 6+. See docs
But I wonder if this would work:
// Double-check idiom for lazy initialization of instance fields without volatile
private FieldHolder fieldHolder = null;
private static class FieldHolder{
public final FieldType field;
FieldHolder(){
field = computeFieldValue();
}
}
FieldType getField() {
if (fieldHolder == null) { // First check (no locking)
synchronized(this) {
if (fieldHolder == null) // Second check (with locking)
fieldHolder = new FieldHolder();
}
}
return fieldHolder.field;
}
Or maybe even:
// Double-check idiom for lazy initialization of instance fields without volatile
private FieldType field = null;
private static class FieldHolder{
public final FieldType field;
FieldHolder(){
field = computeFieldValue();
}
}
FieldType getField() {
if (field == null) { // First check (no locking)
synchronized(this) {
if (field == null) // Second check (with locking)
field = new FieldHolder().field;
}
}
return field;
}
Or:
// Double-check idiom for lazy initialization of instance fields without volatile
private FieldType field = null;
FieldType getField() {
if (field == null) { // First check (no locking)
synchronized(this) {
if (field == null) // Second check (with locking)
field = new Object(){
public final FieldType field = computeFieldValue();
}.field;
}
}
return field;
}
I belive this would work based on this oracle doc:
The usage model for final fields is a simple one: Set the final fields for an object in that object's constructor; and do not write a reference to the object being constructed in a place where another thread can see it before the object's constructor is finished. If this is followed, then when the object is seen by another thread, that thread will always see the correctly constructed version of that object's final fields. It will also see versions of any object or array referenced by those final fields that are at least as up-to-date as the final fields are.
Answer
First things first: what you are trying to do is dangerous at best. I am getting a bit nervous when people try to cheat with finals. Java language provides you with volatile as the go-to tool to deal with inter-thread consistency. Use it.
Anyhow, the relevant approach is described in "Safe Publication and Initialization in Java" as:
public class FinalWrapperFactory {
private FinalWrapper wrapper;
public Singleton get() {
FinalWrapper w = wrapper;
if (w == null) { // check 1
synchronized(this) {
w = wrapper;
if (w == null) { // check2
w = new FinalWrapper(new Singleton());
wrapper = w;
}
}
}
return w.instance;
}
private static class FinalWrapper {
public final Singleton instance;
public FinalWrapper(Singleton instance) {
this.instance = instance;
}
}
}
It layman's terms, it works like this. synchronized yields the proper synchronization when we observe wrapper as null -- in other words, the code would be obviously correct if we drop the first check altogether and extend synchronized to the entire method body. final in FinalWrapper guarantees iff we saw the non-null wrapper, it is fully constructed, and all Singleton fields are visible -- this recovers from the racy read of wrapper.
Note that it carries over the FinalWrapper in the field, not the value itself. If instance were to be published without the FinalWrapper, all bets would be off (in layman terms, that's premature publication). This is why your Publisher.publish is disfunctional: just putting the value through final field, reading it back, and publishing it unsafely is not safe -- it's very similar to just putting the naked instance write out.
Also, you have to be careful to make a "fallback" read under the lock, when you discover the null wrapper, and use its value. Doing the second (third) read of wrapper in return statement would also ruin the correctness, setting you up for a legitimate race.
EDIT: That entire thing, by the way, says that if the object you are publishing is covered with final-s internally, you may cut the middleman of FinalWrapper, and publish the instance itself.
EDIT 2: See also, LCK10-J. Use a correct form of the double-checked locking idiom, and some discussion in comments there.