Issue with Double Check Locking in Java

Viewed 608

One of the article mentions an issue with "Double Check Locking". Please see the below example

public class MyBrokenFactory {
  private static MyBrokenFactory instance;
  private int field1, field2 ...

  public static MyBrokenFactory getFactory() {
    // This is incorrect: don't do it!
    if (instance == null) {
      synchronized (MyBrokenFactory.class) {
        if (instance == null)
          instance = new MyBrokenFactory();
      }
    }
    return instance;
  }

  private MyBrokenFactory() {
    field1 = ...
    field2 = ...
  }
}

Reason:- (Please note the order of execution by the numbering)

Thread 1: 'gets in first' and starts creating instance.

1. Is instance null? Yes.
2. Synchronize on class.
3. Memory is allocated for instance.
4. Pointer to memory saved into instance.

[[Thread 2]]

7. Values for field1 and field2 are written
to memory allocated for object.

.....................
Thread 2: gets in just as Thread 1 has written the object reference
to memory, but before it has written all the fields.

5. Is instance null? No.
6. instance is non-null, but field1 and field2 haven't yet been set!
   This thread sees invalid values for field1 and field2!

Question :
As the creation of the new instance(new MyBrokenFactory()) is done from the synchronized block, will the lock be released before the entire initialization is completed (private MyBrokenFactory() is completely executed) ?

Reference - https://www.javamex.com/tutorials/double_checked_locking.shtml

Please explain.

2 Answers

The problem is here:

Thread 2: gets in just as Thread 1 has written the object reference to memory, but before it has written all the fields.

Is instance null? No.

With out synchronization, thread 2 might see instance as null, even though thread 1 has written it. Notice that the first check of instance is outside of the synchronized block:

if (instance == null) {
  synchronized (MyBrokenFactory.class) {

Since that first check is done outside of the block there's no guarantee that thread 2 will see the correct value of instance.

I have no idea what you're trying to do with field1 and field2, you never even write them.

Re. Your edit:

As the creation of the new instance(new MyBrokenFactory()) is done from the synchronized block

I think what you're asking is if the two instance fields, field1 and field2 are guaranteed to be visible. The answer is no, and the problem is the same as with instance. Because you don't read instance from within a synchronized block, there's no guarantee that those instance fields will be read correctly. If instance is non-null, you never enter the synchronized block, so no synchronization occurs.

Please find an answer to my question. I got the answer by looking into another similar question here.

Synchronize guarantees, that only one thread can enter a block of code. But it doesn't guarantee, that variables modifications done within synchronized section will be visible to other threads. Only the threads that enters the synchronized block is guaranteed to see the changes. This is the reason why double checked locking is broken - it is not synchronized on the reader's side. The reading thread may see, that the singleton is not null, but singleton data may not be fully initialized (visible).

Ordering is provided by volatile. volatile guarantees ordering, for instance write to volatile singleton static field guarantees that writes to the singleton object will be finished before the write to volatile static field. It doesn't prevent creating singleton of two objects, this is provided by synchronize.

Class final static fields doesn't need to be volatile. In Java, the JVM takes care of this problem.

Related