3

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.

Pratham
  • 1,522
  • 1
  • 18
  • 33
Sunny
  • 858
  • 3
  • 17
  • 39
  • Also to shortly answer give an insight on your specification "will the lock be released before the entire initialisation is completed" the answer is: no. But as the second thread never even attempts to acquire the lock this does not help you here. – Ben Oct 31 '18 at 06:16
  • I suppose the solution is to declare `instance` `volatile` – Tootsie Oct 31 '18 at 06:22
  • I've updated my answer (and I think Tootise is correct, `volatile` is almost always the fix for a broken double-checked lock.) – markspace Oct 31 '18 at 06:25
  • 1
    I know that volatile will fix it. Guys i am not asking what is the fix my question is a fundamental question on Thread Locking. 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) ? – Sunny Oct 31 '18 at 06:30
  • 1
    @Sunny The problem is that you cannot define "the entire initialization is complete". Yes, for the thread that creates it, the answer is yes. But other threads may observe a partially constructed object because synchronization is missing – Tootsie Oct 31 '18 at 06:35
  • @Tootsie : This was the something that i was looking for. Thanks i also got this info from another questions in SO. – Sunny Oct 31 '18 at 06:37
  • 1
    To be more precise, if a second thread observes a non-null value for `instance`, you cannot reliably access it since that thread does not enter the synchronization block and hence the object is unsynchronized – Tootsie Oct 31 '18 at 06:37

2 Answers2

0

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.

Community
  • 1
  • 1
markspace
  • 10,621
  • 3
  • 25
  • 39
  • I know that volatile will fix it. Guys i am not asking what is the fix my question is a fundamental question on Thread Locking. 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) ? – Sunny Oct 31 '18 at 06:30
  • What does that have to do with anything? Why do you care? The code is broken, it doesn't actually matter when the lock is released. – markspace Oct 31 '18 at 07:01
0

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.

Pang
  • 9,564
  • 146
  • 81
  • 122
Sunny
  • 858
  • 3
  • 17
  • 39
  • One correction here: the *initialization* of a static field doesn't need further synchronization (when done as an initializer or a static block in that class). However every write after the initialization does need to be synchronized somehow. – markspace Oct 31 '18 at 07:04