Factory method to create singleton instance

Question

I have written below code in static factory method to return single instance of DefaultCache.

public static ICache getInstance() {
   if (cacheInstance == null) {
      synchronized (ICache.class) {
         if (cacheInstance == null) {
            cacheInstance = new DefaultCache();
         }
      }
   }
   return cacheInstance;
}

Do we really need 2nd null check for cacheInstance inside the synchronized block?

Answer 1

You need the second check as the value could have been set by another thread while you are trying to get the lock. In fact you don't have a safe view of this value until you are inside the synchronized block. It could have been set by another thread any amount of time ago.

The simplest lazy singleton is to use an enum

public enum DefaultCache implements ICache {
     INSTANCE
}

I assume you are not doing this so you can change the implementation.

BTW: I suggest you only use stateless singletons where possible and use dependanecy injection for all stateful objects as much as possible.

Answer 2

I would avoid the need for the contention check using a lazily initialized singleton:

public class Singleton {
    public static ICache getInstance() {
       return LazyCacheInitializer.INSTANCE;
    }

    private class LazyCacheInitializer {
       private static final ICache INSTANCE = new DefaultCache();
    }
}