4

Let's say I have next class:

public class Singleton{

  private static Singleton _instance; 

  public static Singleton getInstance(){

   if(_instance == null){
         synchronized(Singleton.class){
           if(_instance == null)
           _instance = new Singleton();
         }
   }
   return _instance;
 }

For example we have two threads A and B which try to execute getInstance() method simultaneously. Am I understand the process correctly:

  1. Thread A enter into getInstance() method and acquire the lock;
  2. Thread B also enter into getInstance() method and blocked;
  3. Thread A create new Singleton() object and release the lock, now last value of _instance variable should be visible to thread B? Or thread B still could have own copy of _instance variable which is not synchronized with the main memory(_instance=null)?
Iurii
  • 1,755
  • 8
  • 24
  • 38

3 Answers3

5

Thread B is blocked on the synchronize, when it proceeds it will see the changed field _instance != null and therefore does not construct one but return the existing.

All other threads which come later see the instance being set and will not even lock.

Problem: your code is incomplete, you need volatile in order to make sure threads which do not go through the synchronized (most of them, hopefully) still only see a completely published singleton object.

The Java Memory Model does only guarantee that the final fields are initialized. For all others you need a safe publish, which is possible with:

  • Exchange the reference through a properly locked field (JLS 17.4.5)
  • Use static initializer to do the initializing stores (JLS 12.4)
  • Exchange the reference via a volatile field (JLS 17.4.5), or as the consequence of this rule, via the AtomicX classes
  • Initialize the value into a final field (JLS 17.5).

The easiest method to avoid the volatile (or an atomic reference which is also safe to publish objects to other threads) is to use normal Object initialisation, this is a valid and robust singleton (but not lazy) provided by the JVM:

class Singleton
{
    private static final Singleton HIGHLANDER = new Singleton();
    private Singleton() { } // not accessible
    public static getSingleton() { return HIGHLANDER; }
}

JDK internally uses this similar construct with "Holder" objects to implement the same simple and robust pattern but in a lazy fashion:

class Singleton
{
    private Singleton() { } // not accessible
    private static Class LazyHolder {
        private static final Singleton LAZY_HIGHLANDER = new Singleton();
    }
    public static Singleton getInstance() {
        return LazyHolder.LAZY_HIGHLANDER;
    }
}

Both methods do not require volatile variable access (which you need in DCL case) or synchronisation (it is implicitly done by the JVM which does the initialisation protected by a class lock).

eckes
  • 10,103
  • 1
  • 59
  • 71
  • If I put first if into synchronize block then in this case all threads should see completely published singleton object? – Iurii May 09 '15 at 21:40
  • 1
    Yes, that is the classical singleton pattern which is always safe. But it has bad scalability as each access is locking the factory. It is slower than volatile with multiple threads for sure. (thats why you normally want the DCL or one of the solutions I have shown). – eckes May 09 '15 at 21:58
3

What you show here is called double-checked locking.

The static variable belongs to the class, not the thread. Both threads will see the proper value, but it is possible that the compiler may optimize the reads such that the static variable is not checked both times. For this reason, you should declare the variable with the volatile keyword.

Please note that in versions of Java prior to version 5 this might not work correctly even with a volatile variable. It used to be possible for the assignment to assign a partially-constructed object to the variable. Now the constructor must return before the assignment can proceed. This will work correctly in any modern version Java.

Community
  • 1
  • 1
  • Even with Java 5 DCL require the field to be volatile or you need any other mechanism (from `java.util.concurrent`) which ensures safe publishing. – eckes May 09 '15 at 20:47
  • @eckes you are correct, turns out that `volatile` is required. Given how often I use singletons (and prefer to initialize them when the class loads in the rare cases I do need one) I missed that detail. –  May 09 '15 at 20:55
  • I think the explanation for the volatile is incomplete, the variable is actually checked correctly for threads which enter the synchronized (which would be all which do not see the new reference) however the fact that any later thread does see the reference does not mean that the referenced object is completely constructed at that time - volatile will ensure that. – eckes May 09 '15 at 21:03
  • @eckes It is possible that after one thread assigns the variable another thread might not see it right away _if it is already in the method_. However, Java 5 and later ensure the object is fully constructed before assigning the variable. –  May 09 '15 at 21:05
  • Yes it is possible, but if it is before the first `if` it will go into the synchronized (since it is `null`) and if it is after the first `if` it will also be already in the synchronized (since it was `null`). There is no way a thread can see a null field and not go the slow path (which will synchronize and therefore is safe). And Java 5 will not ensure the object to be fully constructed as seen by other threads as long as you not publish it safely. – eckes May 09 '15 at 21:06
1

Two problems that could exist as far as I know. Thread B might or might not see the latest value. Thread B might see a partially constructed object, incase there are a lot of things that the constructor does, and JVM decides to change the ordering of the code.

Making it volatile solves both problems, since it enforces the happens before relationship and stops JVM from re ordering the code execution, and updates the values in the other threads.

Sandeep Kaul
  • 2,957
  • 2
  • 20
  • 36