8

As the title said, why is static nested class singleton thread-safe?

public class Singleton {
    private static class SingletonHolder {
        public static Singleton instance;

        public static Singleton getInstance() {
            if (null == instance) {
                instance = new Singleton();
            }
        }
    }

    public static Singleton getInstance() {
        return SingletonHolder.getInstance();
    }
}
Sergey
  • 2,557
  • 1
  • 27
  • 49
huashui
  • 1,648
  • 2
  • 15
  • 21
  • 3
    Firstly, your code can't be compiled. – OQJF Jul 23 '13 at 01:19
  • Firstly what you are trying to achieve is not going to be singleton as you can still make 'n' number of objects of this class. – Prashant Dec 01 '18 at 14:04
  • The reason for this mentioned class is not a Singleton and can able to create more than one instance is maybe checking wrong null checking. ```if (null == instance)``` need to be changed with ```if(instance == null)``` instead. Please correct me if I am wrong. – Saadat Sayem Jul 02 '20 at 10:00

2 Answers2

33

The code you show is not technically thread-safe. This sort of dodgy code often gets mangles.

The code should look like:

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

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

Here we are assigning within a static initialiser (of SingletonHolder), which will be seen by any thread accessing it with correct happens-before relationship. There's nothing really special about the nested class, it just allows the outer class to be used without immediately constructing the singleton object. Almost certainly this is entirely pointless, but it seems to please some people.

As ever [mutable] singletons are a really bad idea.

Hearen
  • 6,019
  • 2
  • 36
  • 50
Tom Hawtin - tackline
  • 139,906
  • 30
  • 206
  • 293
  • Your code won't compile. SingletonHolder has no `getInstance()` method – Gus Jul 23 '13 at 01:42
  • what's the point of immutable singleton? – ZhongYu Jul 23 '13 at 01:46
  • @zhong.j.yu Well it wouldn't be a Singleton by any reasonable definition. Single instance implementations are useful for functors (such as `Comparator`s) and distinguished value. – Tom Hawtin - tackline Jul 23 '13 at 08:31
  • 1
    there's no distinction between one functor or a dozen; a singleton functor is only a performance optimization, it has no significance to application logic. – ZhongYu Jul 23 '13 at 14:10
  • The code does compile, comments above appear to have been edited into the answer. – Jayson Minard Feb 25 '16 at 15:41
  • 2
    @TomHawtin-tackline you can add a private Singleton constructor `private Singleton() {}` to make this example complete. – Raman Sahasi Oct 19 '17 at 09:12
  • @RamanSahasi The code is supposed to have minimal changes from the original. Hiding the constructor and making the class final would be unnecessary distractions. – Tom Hawtin - tackline Oct 20 '17 at 21:59
  • I would disagree with your answer as i can still make "new Singleton()" – Prashant Dec 01 '18 at 14:05
  • @Prashant As my comment immediately above yours: Together with any useful methods, the private constructor was elided from the original question and, therefore to keep the differences relevant to the point in hand, also in my answer. – Tom Hawtin - tackline Dec 01 '18 at 14:59
6

It's thread-safe because the JVM handles lazily loading the nested class.

However, the code you posted doesn't seem to be using this pattern correctly (you shouldn't have a null check), and I think that actually breaks the thread safety. Here's a nice article where you can read more about why this pattern works and how to use it correctly:

Initialization-on-demand holder idiom

DaoWen
  • 31,184
  • 6
  • 65
  • 95