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.
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.
Comments
Why would a person reorder the assignment when its very clear that the correct assignment order should be there?
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.
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.
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