Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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.

like image 567
Kicsi Avatar asked Apr 26 '15 20:04

Kicsi


People also ask

Why volatile with double-checked locking?

Since it requires the volatile keyword to work properly, it's not compatible with Java 1.4 and lower versions. The problem is that an out-of-order write may allow the instance reference to be returned before the singleton constructor is executed. Performance issue because of decline cache for volatile variable.

Why double-checked locking is broken?

Although the double-checked locking idiom cannot be used for references to objects, it can work for 32-bit primitive values (e.g., int's or float's). Note that it does not work for long's or double's, since unsynchronized reads/writes of 64-bit primitives are not guaranteed to be atomic.

Is double-checked locking safe?

Since only the first access requires locking, double-checked locking is used to avoid locking overhead of subsequent accesses. However, on many languages and hardware, the design can be unsafe.

How do I stop double check locking?

The only way to do double-checked locking correctly in Java is to use "volatile" declarations on the variable in question. While that solution is correct, note that "volatile" means cache lines get flushed at every access.


2 Answers

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.

like image 189
Aleksey Shipilev Avatar answered Sep 21 '22 07:09

Aleksey Shipilev


In short

The version of the code without the volatile or the wrapper class is dependent on the memory model of the underlying operating system that the JVM is running on.

The version with the wrapper class is a known alternative known as the Initialization on Demand Holder design pattern and relies upon the ClassLoader contract that any given class is loaded at most once, upon first access, and in a thread-safe way.

The need for volatile

The way developers think of code execution most of the time is that the program is loaded into main memory and directly executed from there. The reality, however, is that there are a number of hardware caches between main memory and the processor cores. The problem arises because each thread might run on separate processors, each with their own independent copy of the variables in scope; while we like to logically think of field as a single location, the reality is more complicated.

To run through a simple (though perhaps verbose) example, consider a scenario with two threads and a single level of hardware caching, where each thread has their own copy of field in that cache. So already there are three versions of field: one in main memory, one in the first copy, and one in the second copy. I'll refer to these as fieldM, fieldA, and fieldB respectively.

  1. Initial state
    fieldM = null
    fieldA = null
    fieldB = null
  2. Thread A performs the first null-check, finds fieldA is null.
  3. Thread A acquires the lock on this.
  4. Thread B performs the first null-check, finds fieldB is null.
  5. Thread B tries to acquire the lock on this but finds that it's held by thread A. Thread B sleeps.
  6. Thread A performs the second null-check, finds fieldA is null.
  7. Thread A assigns fieldA the value fieldType1 and releases the lock. Since field is not volatile this assignment is not propagated out.
    fieldM = null
    fieldA = fieldType1
    fieldB = null
  8. Thread B awakes and acquires the lock on this.
  9. Thread B performs the second null-check, finds fieldB is null.
  10. Thread B assigns fieldB the value fieldType2 and releases the lock.
    fieldM = null
    fieldA = fieldType1
    fieldB = fieldType2
  11. At some point, the writes to cache copy A are synched back to main memory.
    fieldM = fieldType1
    fieldA = fieldType1
    fieldB = fieldType2
  12. At some later point, the writes to cache copy B are synched back to main memory overwriting the assignment made by copy A.
    fieldM = fieldType2
    fieldA = fieldType1
    fieldB = fieldType2

As one of the commenters on the question mentioned, using volatile ensures writes are visible. I don't know the mechanism used to ensure this -- it could be that changes are propagated out to each copy, it could be that the copies are never made in the first place and all accesses of field are against main memory.

One last note on this: I mentioned earlier that the results are system dependent. This is because different underlying systems may take less optimistic approaches to their memory model and treat all memory shared across threads as volatile or may perhaps apply a heuristic to determine whether a particular reference should be treated as volatile or not, though at the cost of performance of synching to main memory. This can make testing for these problems a nightmare; not only do you have to run against a enough large sample to try to trigger the race condition, you might just happen to be testing on a system which is conservative enough to never trigger the condition.

Initialization on Demand holder

The main thing I wanted to point out here is that this works because we're essentially sneaking a singleton into the mix. The ClassLoader contract means that while there can many instances of Class, there can be only a single instance of Class<A> available for any type A, which also happens to be loaded on first when first reference / lazily-initialized. In fact, you can think of any static field in a class's definition as really being fields in a singleton associated with that class where there happens to be increased member access privileges between that singleton and instances of the class.

like image 40
hayden.sikh Avatar answered Sep 22 '22 07:09

hayden.sikh