0

Consider the following non-traditional implementation of double-check locking that does not use volatile:

public class ValueProvider {
  private static State state = new Initial();

  public static Value getValue() {
      return state.getValue();
  }

  private static class Initial implements State {
      @Override
      public synchronized Value getValue() {
          if (state instanceof Initial) {
              Value value = new Value();
              value.x = 1;
              value.y = 2; 
              state = new Initialized(value);
              return value;
          } else {
              return state.getValue();
          }
      }
  }

  private static class Initialized implements State {
      private final Value value;

      private Initialized(Value value) {
          this.value = value;
      }

      @Override
      public Value getValue() {
          return value;
      }
  }

  private interface State {
      Value getValue();
  }

  public static final class Value {
      private int x;
      private int y;

      public int getX() {
          return x;
      }

      public int getY() {
          return y;
      }
  }

}

Is this code thread-safe ?

Specifically I am asking about the final field and the guarantees it gives, so the question may be reformulated as is that possible for some thread to get a non-initialized instance of Value ?

UPDATE: removed mention about setters, so that only reads are available after publication

AngryJuice
  • 61
  • 1
  • 6
  • 4
    Consider this [far easier, rock solid, approach](http://stackoverflow.com/a/16106598/256196) to lazy initialization. – Bohemian Jul 04 '14 at 08:08
  • @Bohemian good approach; just be careful of uncaught exceptions during the initialisation phase and you are golden. – Chris K Jul 04 '14 at 08:13
  • @Bohemian Thank you, but the key point here is not to find the best approach for lazy initialization, but rather to illustrate the principle of safe publication using final keyword. While it's definitely true for simple cases, like primitive attributes in constructor, the question is will that work in more complicated cases like described. – AngryJuice Jul 04 '14 at 09:39

2 Answers2

0

No, this is not thread safe. There is no memory barrier on the read of ValueProvider.state and none at all on Value.

The rule of thumb with Java concurrency is that there needs to be a memory barrier on read and a memory barrier on write.

The only ways to add a memory barrier in Java are:

  • synchronized
  • volatile
  • class initialisation (implicit by the jvm)
  • atomic
  • unsafe

For most things Hotspot ignores the final keyword, and prefers to infer it itself. However where final does affect the JMM is to do with class construction and inlining. The reordering rules for final fields are covered in the cookbook that you have already mentioned. It does not mention final classes. The cookbook states:

Loads and Stores of final fields act as "normal" accesses 
with respect to locks and volatiles, but impose two additional reordering

1) A store of a final field (inside a constructor and, if the field is a reference, any store that this final can reference, cannot be reordered with a subsequent store

2) The initial load (i.e., the very first encounter by a thread) of a final field cannot be reordered with the initial load of the reference to the object containing the final field.

Chris K
  • 11,622
  • 1
  • 36
  • 49
  • Right, it means you cannot reorder assignment to final field and reference, but what about the assigned value itself ? Can the actions with it (x=1, y=2) be reordered with reference ? – AngryJuice Jul 04 '14 at 09:46
  • @AngryJuice A small Java example to make this question about x and y being reordered with reference clearer would be helpful. – Chris K Jul 04 '14 at 09:59
0

Well, besides the fact that your approach is way too complicated as Bohemian ♦ has pointed out, it could work regarding publication. If two threads access getValue() concurrently, only one thread can enter the synchronized block. The other will either be blocked on the synchronized block or see an instance of Initialized with a correctly initialized value field due to the final field initialization guaranty.

However, it still doesn’t work because the instance of class Value is mutable and your comment // getters and setters indicates that the instance will be mutated after construction. In this case the entire final field initialization guaranty is pointless as the Value class is not thread safe. You might be guarded against seeing the default values for x and y but you will never know what values regarding later-on modifications you will see and the values for (x, y) are not necessarily consistent.

Community
  • 1
  • 1
Holger
  • 285,553
  • 42
  • 434
  • 765
  • Good point. But let's consider we remove public setters and only keep getters (so only reads are available after publication). Will the final keyword guarantee the safe publication or it will only guarantee that other threads won't see instance of Initialized with value=null, but the value itself may not be fully constructed (e.g. with x=1, y=0) ? – AngryJuice Jul 04 '14 at 09:34
  • 1
    The `final` safety guaranty expands to all objects reachable through that `final` reference, as long as there are no mutations *after* the construction and, of course, there is no access via other references to these objects. In your case, you will see a fully constructed `Value` object including the right values for `x` and `y`. A prominent example which relies on this guarantee is `java.lang.String` which cannot declare the elements of its internal `char[]` array as immutable. But all elements are always read through a `final` array reference… – Holger Jul 04 '14 at 09:52