DCL with two synchronized block is broken?

Question

I could not understand below code snippet from A fix that doesn't work. (I did read the explanation that follows on same page).

If we have 2 synchronized blocks, how is this DCL version broken? Or is it not applicable post Java5?

// (Still) Broken multithreaded version
// "Double-Checked Locking" idiom
class Foo { 
  private Helper helper = null;
  public Helper getHelper() {
    if (helper == null) {
      Helper h;
      synchronized(this) {
        h = helper;
        if (h == null) 
            synchronized (this) {
              h = new Helper();
            } // release inner synchronization lock
        helper = h;
        } 
      }    
    return helper;
    }
  // other functions and members...
  }

Answer 1

There's no guarantee that a thread that sees helper as not null will be able to see all the writes made by new Helper();. So you could access a corrupt version of the singleton. You need something in a thread that sees helper as non-null to guarantee that it sees that after the h = new Helper(); completes. Observing a change to a non-volatile variable doesn't establish such a relationship, and that's all that thread does.

Oversimplifying a bit, the way Java's memory visibility model works is that two threads each do something that establishes a "happens before" / "happens after" relationship between the two operations done by the two threads. That can include operations inside a synchronized block or access to a volatile variable.

But with your code above, a thread can observe that helper is not null and then go on to access the object created by new Helper(). It doesn't have to access a volatile variable, nor does it have to enter a synchronized block. So there is nothing that could possibly establish the required "happens after" relationship to ensure it sees any changes made by new Helper().