How to solve the "Double-Checked Locking is Broken" Declaration in Java?

Question

I want to implement lazy initialization for multithreading in Java.
I have some code of the sort:

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...
}

And I'm getting the the "Double-Checked Locking is Broken" declaration.
How can I solve this?

Solution 1

Here is the idiom recommended in the Item 71: Use lazy initialization judiciously of Effective Java:

If you need to use lazy initialization for performance on an instance field, use the double-check idiom. This idiom avoids the cost of locking when accessing the field after it has been initialized (Item 67). The idea behind the idiom is to check the value of the field twice (hence the name double-check): once without locking, and then, if the field appears to be uninitialized, a second time with locking. Only if the second check indicates that the field is uninitialized does the call initialize the field. Because there is no locking if the field is already initialized, it is critical that the field be declared volatile (Item 66). Here is the idiom:

// Double-check idiom for lazy initialization of instance fields
private volatile FieldType field;

private FieldType getField() {
    FieldType result = field;
    if (result != null) // First check (no locking)
        return result;
    synchronized(this) {
        if (field == null) // Second check (with locking)
            field = computeFieldValue();
        return field;
    }
}

This code may appear a bit convoluted. In particular, the need for the local variable result may be unclear. What this variable does is to ensure that field is read only once in the common case where it’s already initialized. While not strictly necessary, this may improve performance and is more elegant by the standards applied to low-level concurrent programming. On my machine, the method above is about 25 percent faster than the obvious version without a local variable.

Prior to release 1.5, the double-check idiom did not work reliably because the semantics of the volatile modifier were not strong enough to support it [Pugh01]. The memory model introduced in release 1.5 fixed this problem [JLS, 17, Goetz06 16]. Today, the double-check idiom is the technique of choice for lazily initializing an instance field. While you can apply the double-check idiom to static fields as well, there is no reason to do so: the lazy initialization holder class idiom is a better choice.

Reference

• Effective Java, Second Edition
  • Item 71: Use lazy initialization judiciously

Solution 2

Here is a pattern for correct double-checked locking.

class Foo {

  private volatile HeavyWeight lazy;

  HeavyWeight getLazy() {
    HeavyWeight tmp = lazy; /* Minimize slow accesses to `volatile` member. */
    if (tmp == null) {
      synchronized (this) {
        tmp = lazy;
        if (tmp == null) 
          lazy = tmp = createHeavyWeightObject();
      }
    }
    return tmp;
  }

}

For a singleton, there is a much more readable idiom for lazy initialization.

class Singleton {
  private static class Ref {
    static final Singleton instance = new Singleton();
  }
  public static Singleton get() {
    return Ref.instance;
  }
}

Solution 3

DCL using ThreadLocal By Brian Goetz @ JavaWorld

what's broken about DCL?

DCL relies on an unsynchronized use of the resource field. That appears to be harmless, but it is not. To see why, imagine that thread A is inside the synchronized block, executing the statement resource = new Resource(); while thread B is just entering getResource(). Consider the effect on memory of this initialization. Memory for the new Resource object will be allocated; the constructor for Resource will be called, initializing the member fields of the new object; and the field resource of SomeClass will be assigned a reference to the newly created object.

class SomeClass {
  private Resource resource = null;
  public Resource getResource() {
    if (resource == null) {
      synchronized {
        if (resource == null) 
          resource = new Resource();
      }
    }
    return resource;
  }
}

However, since thread B is not executing inside a synchronized block, it may see these memory operations in a different order than the one thread A executes. It could be the case that B sees these events in the following order (and the compiler is also free to reorder the instructions like this): allocate memory, assign reference to resource, call constructor. Suppose thread B comes along after the memory has been allocated and the resource field is set, but before the constructor is called. It sees that resource is not null, skips the synchronized block, and returns a reference to a partially constructed Resource! Needless to say, the result is neither expected nor desired.

Can ThreadLocal help fix DCL?

We can use ThreadLocal to achieve the DCL idiom's explicit goal -- lazy initialization without synchronization on the common code path. Consider this (thread-safe) version of DCL:

Listing 2. DCL using ThreadLocal

class ThreadLocalDCL {
  private static ThreadLocal initHolder = new ThreadLocal();
  private static Resource resource = null;
  public Resource getResource() {
    if (initHolder.get() == null) {
      synchronized {
        if (resource == null) 
          resource = new Resource();
        initHolder.set(Boolean.TRUE);
      }
    }
    return resource;
  }
}

I think; here each thread will once enters the SYNC block to update the threadLocal value; then it will not. So ThreadLocal DCL will ensure a thread will enter only once inside the SYNC block.

What does synchronized really mean?

Java treats each thread as if it runs on its own processor with its own local memory, each talking to and synchronizing with a shared main memory. Even on a single-processor system, that model makes sense because of the effects of memory caches and the use of processor registers to store variables. When a thread modifies a location in its local memory, that modification should eventually show up in the main memory as well, and the JMM defines the rules for when the JVM must transfer data between local and main memory. The Java architects realized that an overly restrictive memory model would seriously undermine program performance. They attempted to craft a memory model that would allow programs to perform well on modern computer hardware while still providing guarantees that would allow threads to interact in predictable ways.

Java's primary tool for rendering interactions between threads predictably is the synchronized keyword. Many programmers think of synchronized strictly in terms of enforcing a mutual exclusion semaphore (mutex) to prevent execution of critical sections by more than one thread at a time. Unfortunately, that intuition does not fully describe what synchronized means.

The semantics of synchronized do indeed include mutual exclusion of execution based on the status of a semaphore, but they also include rules about the synchronizing thread's interaction with main memory. In particular, the acquisition or release of a lock triggers a memory barrier -- a forced synchronization between the thread's local memory and main memory. (Some processors -- like the Alpha -- have explicit machine instructions for performing memory barriers.) When a thread exits a synchronized block, it performs a write barrier -- it must flush out any variables modified in that block to main memory before releasing the lock. Similarly, when entering a synchronized block, it performs a read barrier -- it is as if the local memory has been invalidated, and it must fetch any variables that will be referenced in the block from main memory.

Solution 4

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. Since "synchronized" flushes them at the end of the block, it may not actually be any more efficient (or even less efficient). I'd recommend just not using double-checked locking unless you've profiled your code and found there to be a performance problem in this area.

Solution 5

Define the variable that should be double-checked with volatile midifier

You don't need the h variable. Here is an example from here

class Foo {
    private volatile Helper helper = null;
    public Helper getHelper() {
        if (helper == null) {
            synchronized(this) {
                if (helper == null)
                    helper = new Helper();
            }
        }
        return helper;
    }
}

View more solutions

Comments

[Mike Baranczak]

A very interesting idea. Of course, it won't work in cases where you don't want to use the singleton pattern.

[irreputable]

the question is about a non static field.

[Brett]

Probably better off using volatile (although I haven't benchmarked it).

[samkass]

I see your answer doesn't use volatile and is correct, but it's also likely no faster since it requires two objects and an extra memory fetch. The bottom line is still that double-checked locking is likely a marginal performance gain at best and you shouldn't bother with it unless your profiling shows it to be extremely performance-critical code... it's too easy to get wrong and/or introduce difficult-to-find problems for little gain.

[irreputable]

on average volatile read causes more memory fetches, because cache line is flushed. the presence of volatile kills optimization. if we change all our variables to volatile, out programs will crap out.

[Ajoy Bhatia]

The one and only correct answer to the questions about double-checked locking in all of Stack Overflow. Just wanted to add a statement by Joshua Bloch, the author of the book that is referred to in this answer. In an interview (oracle.com/technetwork/articles/javase/…), said: "The idiom is very fast but also complicated and delicate, so don't be tempted to modify it in any way. Just copy and paste -- normally not a good idea, but appropriate here". Just a warning for anyone who might be tempted to try to "improve" on the above code. :-)

[Kanagavelu Sugumar]

@Pascal Thivent I have two questions on the code sample. 1) Why result = field; is needed inside the SYNC block? SYNC itself enough to update the thread right? e.g.) if (field== null) // Second check (with locking) 2) Assigning the value to result is needed at the last? result = computeFieldValue();

[Kanagavelu Sugumar]

@erickson I have two questions on the code sample. 1) Why tmp= lazy; is needed inside the SYNC block? SYNC itself enough to update the thread right? so we can check only on if (lazy== null) inside SYNC instead if (temp== null) 2) Assigning the value to result is needed at the last? tmp = createHeavyWeightObject();

[brady]

@KanagaveluSugumar The (1) first assignment to tmp is there to save an extra read of the volatile variable, lazy; the assumption is that reading a volatile variable is much slower than storing a local variable. The (2) second assignment supports a single return, which is a stylistic choice for readability.

[Kanagavelu Sugumar]

@erickson Fine, Thankyou!

[Stefan Reich]

@KanagaveluSugumar 1) Actually I wonder that too, your logic seems correct to me. 2) Assigning the value to result first ensures (apparently) that the constructor is executed fully first, and only then field is updated (fixing the core problem of double-checked locking). Only thing I wonder here is why the compiler doesn't optimize away the assignment to a local variable that is not used afterwards. Apparently it can be assumed that it doesn't do that.

[Anton]

Joshua Bloch, corrected this example. Here is his twitter post twitter.com/joshbloch/status/964327677816532992. And here is the official errata for the book ptgmedia.pearsoncmg.com/imprint_downloads/informit/bookreg/…

[Steven]

Does this example still hold if we need to make field "field" static? private static volatile FieldType field; private static FieldType getField() {)

[user11981729]

@Anton, thanks for sharing the errata, but I think the prevision version was correct also. It was an intermediate version that had definition like - FieldType result = field; if (result == null) {synchronized(this) {if (field == null) field = result = computeFieldValue();}} return result; that was incorrect because the synchronized block doesn't update the result in case field!=null causing result which is still null to be returned from the method