1
1.   class Foo {
2.       private Helper helper = null;
3.       public Helper getHelper() {
4.           if (helper == null) {
5.              synchronized(this) {
6.                 if (helper == null) {
7.                    helper = new Helper();
8.                 }
9.              }
10.          }
11.          return helper;
12.      }
13.  }

The reason why this structure is considered broken is generally described the reordering of assignments done by the compiler such that the Helper constructor is called after the write to the helper variable. My question is that how is this code thread-safe and are the following steps possible?

  • Thread 1, enters the synchronized block and finds out that helper is null.
  • Thread 1, gives up the monitor at this point
  • Thread 2, enters the object monitor and instantiates the helper
  • Thread 1, comes back and re-initializes the helper instance as

I don't see how this solution is any better than single checked locking.

1
  • 1
    It isn't. This article gives a detailed treatment of Singletons in Java; the double-checked locking scenario is discussed in page two of the article. Jul 17, 2014 at 18:23

3 Answers 3

4

This works for the reference to the helper, but is still subtly broken.

Its broken because the VM is allowed to reorder program actions within the synchronized block as much as it likes, so the reference helper can be set to non-null before construction of the helper instance (by Thread 1) has completed.

Thread 2 can now see not-null outside the synchronized block, never attempts to enter the synchronized block (Thread 1 would be still holding the lock and be busy constructing Helper) and works with the half-constructed Helper instance.

This may or may not happen on a specific VM version. But the specification explicitly allows a VM to do this. Thats why the example is broken. It could be fixed by declaring helper volatile (only with Java 5+).

2

How could thread 1 give up the monitor after checking that helper is null? It won't release the lock until it has initialized helper.

This didn't work years ago on the JVM, but they changed the memory model and it fixed this.

The current "best" way is not DCL, but to implement singletons as enums.

5
  • This answer is kind of ramshackle. Think you could put a bit more effort into it? I have no idea what "implement singletons as enums" means. Jul 17, 2014 at 18:25
  • Then you've obviously spent your time more usefully than trying to optimize singleton initialization.
    – Kayaman
    Jul 17, 2014 at 18:27
  • @Kayaman What if a higer priority thread is scheduled to run? Even then thread 1 won't give up the monitor? Jul 17, 2014 at 18:34
  • @user1071840 Of course not. He got the monitor when he entered the synchronized block, and it won't give it up before it exits the block or wait()s. If the other thread gets to run, it will block trying to acquire the monitor at the beginning of the synch block (and when it gets to the block, the other thread has already initialized the instance).
    – Kayaman
    Jul 17, 2014 at 18:37
  • @RobertHarvey, Google for enum singleton java. Jul 17, 2014 at 19:11
0

It's still broken because the writes of any fields within Helper may not be published even though the Helper instance is not-null. For instance:

class Helper {
   int someField;
   public Helper(){
     someField = 10;
   }
}

In this case, it is possible according to the JLS to have something like:

Helper someHelper = getHelper();
if(someHelper.someField == 0){
   // error
}

The line marked //error in theory could be hit since the read & write of someField aren't synchronized with the write of helper within getHelper().

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.