singleton pattern in java. lazy initialization

Question

public static MySingleton getInstance() {
 if (_instance==null) {
   synchronized (MySingleton.class) {
      _instance = new MySingleton();
   }
 }
 return _instance;
} 

1.is there a flaw with the above implementation of the getInstance method? 2.What is the difference between the two implementations.?

public static synchronized MySingleton getInstance() { 
 if (_instance==null) {
  _instance = new MySingleton();
 }

 return _instance;
} 

I have seen a lot of answers on the singleton pattern in stackoverflow but the question I have posted is to know mainly difference of 'synchronize' at method and block level in this particular case.

Answer 1

The second one is thread safe, but it has the overhead of synchronized on every call, no matter if the instance is constructed or not. The first option has one major flaw that it doesn't have a check for if (_instance == null) in the synchronized block to prevent creating two instances.

Answer 2

1.is there a flaw with the above implementation of the getInstance method?

It does not work. You can end up with several instances of your Singleton.

2.What is the difference between the two implementations.?

The second one works, but requires synchronization, which could slow down the system when you have a lot of accesses to the method from different threads.

The most straightforward correct implementation:

public class MySingleton{
    private static final MySingleton _instance = new MySingleton();
    private MySingleton(){}
    public static MySingleton getInstance() { 
        return _instance;
    }
}

Shorter and better (safely serializable):

public enum MySingleton{
    INSTANCE;

    // methods go here
}

Lazy initialization of singletons is a topic that gets attention way out of proportion with its actual practical usefulness (IMO arguing about the intricacies of double-checked locking, to which your example is the first step, is nothing but a pissing contest).

In 99% of all cases, you don't need lazy initialization at all, or the "init when class is first referred" of Java is good enough. In the remaining 1% of cases, this is the best solution:

public enum MySingleton{
    private MySingleton(){}
    private static class Holder {
         static final MySingleton instance = new MySingleton();
    }
    static MySingleton getInstance() { return Holder.instance; }
}

Answer 3

The first is flawed in two ways. As others mentioned here, multiple threads could get through

if (_instance==null) {

They would wait for each other, until the object is completely constructed, but they would do the instantiation and replace the reference in the variable.

The second flaw is a little more complicated. One thread could get into the constructor new MySingleton() and then the JVM switches to another thread. Another thread may check the variable for null, but that may contain a reference to a partially constructed object. So the other thread works on the partially constructed Singleton, that's also not good. So the first variant should be avoided.

The second variant should work fine. Don't care too much about efficiency, until you identify this clearly as blocker. Modern JVMs can optimize away unneeded synchronizations, so in real production-code this construct may never hurt performance.

Answer 4

The various approaches to lazy-load singletons are discussed by Bob Lee in Lazy Loading Singletons and the "right" approach is the Initialization on Demand Holder (IODH) idiom which requires very little code and has zero synchronization overhead.

static class SingletonHolder {
  static Singleton instance = new Singleton();    
}

public static Singleton getInstance() {
  return SingletonHolder.instance;
}

Bob Lee also explain when he wants to lazy load a singleton (during tests and development). Honestly, I'm not convinced there is a huge benefit.

Answer 5

1.is there a flaw with the above implementation of the getInstance method?

Yes, the synchronized keyword should wrap the if statement as well. If it's not then two or more threads could potentially get through to the creation code.

2.What is the difference between the two implementations.?

The second implementation is correct and from my point of view easier to understand.

Using synchronized at the method level for a static method synchronizes on the class, i.e. what you've done in sample 1. Using synchronized at the method level for an instance method synchronizes on the object instance.

Answer 6

I would suggest the following implementation

public class MySingleTon
{

  private static MySingleton obj;

  //private Constructor
  private MySingleTon()
  {
  }


  public static MySingleTon getInstance()
  {
     if(obj==null)
     {
        synchronized(MySingleTon.class)
        {
         if(obj == null)
         {
             obj = new MySingleTon();
         }
        }
     }
     return obj;    
  }
}