Singleton Implementation example

Question

I am trying to find my own way of Java Singleton implementation. The code is as follows:

public class Singleton{
   private volatile static Singleton _instance = null;
   private Singleton(){}
   public static Singleton getInstance(){
      if (_instance == null)
         Object obj = new Object();
         synchronized(obj){
            if (_instance == null)
               _instance = new Singleton();
         }
      return _instance;
}

Does this code work? If not work, how to fix it?

Answer 1

No - your code doesn't work, because the lock object is a local variable and therefore different for every thread.

You are attempting to implement the lazy initialization pattern - where the instance is created when first used.

But there is a simple trick that allows you to code a thread-safe implementation that doesn't require synchronization! It is known as the Initialization-on-demand holder idiom, and it looks like this:

public class Singleton {
    private static class Holder {
        static final Singleton INSTANCE = new Singleton();
    }

    public static Singleton getInstance() {
        return Holder.INSTANCE;
    }

    private Singleton() {
    }

    // rest of class omitted
}

This code initializes the instance on the first calling of getInstance(), and importantly dosen't need synchronization because of the contract of the class loader:

• the class loader loads classes when they are first accessed (in this case Holder's only access is within the getInstance()` method)
• when a class is loaded, and before anyone can use it, all static initializers are guaranteed to be executed (that's when Holder's static block fires)
• the class loader has its own synchronization built right in that make the above two points guaranteed to be threadsafe

It's a neat little trick that I use whenever I need lazy initialization. You also get the bonus of a final instance, even thought it's created lazily. Also note how clean and simple the code is.

Answer 2

Ticky and really simple implementation of not-lazy singleton:

public enum TickySingleton {

    INSTANCE;

    public void doSomething() { ... }
    public Object returnSomething() { ... }
}

}

Not everybody will like this. ;)

Answer 3

it would be better if your synchronization object is final static. Otherwise each possible concurrent thread will create its own sync object and lock different objects. And they will not wait for each other.

public class Singleton{

   private final static Object obj = new Object();
   private volatile static Singleton _instance = null;

   private Singleton(){}

   public static Singleton getInstance(){
     if (_instance == null)
        synchronized(obj){
           if (_instance == null)
              _instance = new Singleton();
         }
      }
      return _instance;
   }

Answer 4

When you write:

Object obj = new Object();
synchronized(obj){}

The JVM can prove that no two threads can acquire that lock (because it is a local variable) and can therefore completely remove the synchronization.

A few comments about singletons:

• in most cases, you don't really need them
• they will be a pain later on, particularly for testing (see section called: "Flaw: Brittle Global State & Singletons")
• if you really do need a singleton, use an enum which is the easiest, thread-safe, lazy way to create a singleton

Answer 5

No, it's not correct, you should synchronize on Singleton.class

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

it's a known double-checked locking pattern, see http://www.ibm.com/developerworks/java/library/j-dcl/index.html for details

Note that since there's only one method in this class the following serves the same purpose without any double-checked tricks and it's lazy too

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