Singleton with static inner class and double-checked locking

Question

I am reading about Singleton design pattern and evaluating different implementations. I have doubt with the below implementations:

A. Singleton Implementation with static inner class

public class SingletonWithStaticClass {

private SingletonWithStaticClass(){}

private static class SingletonInnerClass{
    public static SingletonWithStaticClass INSTANCE = new SingletonWithStaticClass();

}

public static SingletonWithStaticClass getInstance(){
    return SingletonInnerClass.INSTANCE;
}

}

B. Singleton double checked locking

public class SingletonWithDoubleCheck {
private static SingletonWithDoubleCheck INSTANCE = null;

private SingletonWithDoubleCheck(){
    if(INSTANCE != null){
        throw new RuntimeException("Accessing private constructor is prohibited. Use getInstance method instead");
    }
}

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

}

Which one is better?

I feel we can access the private constructor with Reflection in first implementation where as second implementation is safe (From Reflection attack).

However, I am not going to use any of these in my production code, I will use enum instead. But out of these two, isn't the first implementation is broken when considered Reflection attack?

Please correct me if my understanding is wrong.

The singleton design pattern is not intended, in general, to withstand some kind of deliberate "attack". If someone is using reflection to expose a private constructor, it is their responsibility to know if what they are doing contravenes the intended use of the code. — khelwood, Dec 29 '15 at 14:11
The second one is unsafe, as posted. This has been hashed and rehashed. I agree completely with khelwood. A singleton is a design (anti-)pattern. Not a security tool. If someone has access to your code, he can do anything using reflection. — JB Nizet, Dec 29 '15 at 14:17
@JBNizet AFAIK it was fixed in Java 5.0 if you used a `volatile` field, but it shows that it's error prone. — Peter Lawrey, Dec 29 '15 at 14:21
Consider avoiding singletons 99% of the time: http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons — TrueWill, Dec 29 '15 at 14:23
@PeterLawrey yes, with volatile. But as posted, it's unsafe. And indeed it's so easy to mess up that it should be avoided, especially when there are better alternatives. Singletons should be avoided anyway, in general. — JB Nizet, Dec 29 '15 at 14:27
@JBNizet I agree. IMHO Singletons should only be stateless where ever possible. e.g. implementations of a strategy. — Peter Lawrey, Dec 29 '15 at 14:33

score 6 · Accepted Answer · answered Dec 29 '15 at 14:17

Both are overly complicated. The first was the best option before Java 5.0, however the second was never a good option. It didn't work before Java 5.0, it requires a volatile field and after this version you could use enum

I prefer using an enum to define a class with exactly one instance. It is a final class, thread safe, lazy loaded, and has a private constructor.

enum Singleon {
    INSTANCE;
}

The double locking singleton is only useful if you must provide configuration information in it's construction. In this case, I prefer to use Dependency Injection to provide/configure singletons and only have stateless Singletons which don't require configuration.

score 1 · Answer 2 · answered Dec 29 '15 at 14:21

1

Neither are good - and attempting to compare them using different scales is counter-productive.

The first is unnecessarily complex and, as you say, is open to reflection hacking, but so is the other one, just slightly less so.

The second uses a synchronized which comes with a cost.

answered Dec 29 '15 at 14:21

OldCurmudgeon

64,482
16
119
213

2

Any lazy loaded solution needs a lock of some kind. Using a `final` field means there is no cost (not even a check) after the object has been initialised. – Peter Lawrey Dec 29 '15 at 14:24

Singleton with static inner class and double-checked locking

2 Answers2