Wednesday, May 23, 2007

Double-Checked Locking and the Problem with Wikipedia

I love Wikipedia, I really do. I use it for everything. Well, not everything. For example, if I want a really good piece of chocolate cake, I tend to use flour, sugar, butter, eggs, chocolate, and not Wikipedia at all. For many things, however, Wikipedia cannot be surpassed.

Imagine, then, my surprise when I found out that their page on Double-Checked Locking gave an example of a version of double-checked locking that does not work.

// Broken -- Do Not Use!
class Foo {
  private Helper helper = null;
  private boolean initialized = false;
  public Helper getHelper() {
    if (!initialized) {
      synchronized(this) {
        if (!initialized) {
          helper = new Helper();
          initialized = true;
        }
      }
    }
  return helper;
}


(For background on Double-Checked Locking, see Bill Pugh's "Double-Checked Locking is Broken" Declaration.)

The problem with this code is exactly the same as the problem with all of the other variants of the double-checked locking idiom. You can reorder the assignment of initialized with the assignment to helper. As a result, another thread might see the value true for initialized and think that the helper field has been correctly initialized, even though it hasn't been!

I have since fixed the Wikipedia entry; the old entry can be found here.

Please folks -- stop trying to come up with fancy ways of avoiding volatile and synchronized annotations. Friends don't let friends avoid necessary synchronization.

11 comments:

Anonymous said...

This struck me straight away as well... I have linked you on my blog!

Anonymous said...

What do you mean by "you can reorder the assignments" do you mean that compiler can do that depending on its semantics?
Why would a person reorder the assignment when its very clear that the correct assignment order should be there?

Jeremy Manson said...

anon: You interpreted it correctly; I mean that either a compiler or processor can reorder the assignments, or that the memory hierarchy can make it appear as if the assignments have been reordered because of weak cache consistency protocols.

Anonymous said...

I think i have got my answer by going through the link. Don't bother to reply :).

Anonymous said...

Too late for me to write that. You already replied :). Thanks.

liulijian said...

Thanks for clarify the DCL usage.

However, can you show me an example Java program to prove the non-volatile version is broken?

I have tried to run http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckTest.java on my Core 2 Duo dekstop with JDK 5, I couldn't reproduce the DCL problem with it.

Jeremy Manson said...

liulijian: You know how it is with multithreading -- just because something is wrong, that doesn't mean that the wrongness will always be triggered. In this case, it isn't all that likely that you will hit the combination of optimizations and processor behavior needed to get the broken behavior.

Having said that, that doesn't mean that that combination isn't legal. Writing DCL the correct way is just a way of preemptively ensuring that the runtime does what you mean it to do.

Anonymous said...

Hi all,
the memory model of Java was revised as of JDK 1.5, now the semantics of accessing volatile variables wrt. reordering is clarified/modified.
I.e. double checked locking *works* as of JDK 1.5, if the variable is declared as volatile.
Cheers,
Christian

Jeremy Manson said...

Christian -- yes, thank you. The example wikipedia had at the time did not have a volatile field as the holder. I fixed it at the time, and looking at it again, it looks as if it has remained fixed. I'm sorry if that was unclear from the original post.

Dao said...

It seems the mechanism of ACCT makes it impossible to start profiling a java program in the middle. Because for already loaded classes, the OnClassLoad callback of jvmti cannot be called...

Jeremy Manson said...

@dao I think you posted on the wrong thread. Having said that, you just have to make sure that you have your agent running from VM start.