Simplified Singleton pattern in Java

Question

The default way to implement singleton pattern is:

class MyClass {
  private static MyClass instance;
  public static MyClass getInstance() {
    if (instance == null) {
      instance = new MyClass();
    }
    return instance;
  }
}

In an old project, I've tried to simplify the things writing:

class MyClass {
  private static final MyClass instance = new MyClass();
  public static MyClass getInstance() {
    return instance;
  }
}

But it sometimes fail. I just never knew why, and I did the default way. Making a SSCCE to post here today, I've realized the code works.

So, I would like to know opinions.. Is this a aleatory fail code? Is there any chance of the second approach return null? Am I going crazy?

-- Although I don't know if is the right answer for every case, it's a really interesting answer by @Alfred: I also would like to point out that singletons are testing nightmare and that according to the big guys you should use google's dependency injection framework.

Answer 1

The recommended (by Effective Java 2nd ed) way is to do the "enum singleton pattern":

enum MyClass {
    INSTANCE;

    // rest of singleton goes here
}

The key insight here is that enum values are single-instance, just like singleton. So, by making a one-value enum, you have just made yourself a singleton. The beauty of this approach is that it's completely thread-safe, and it's also safe against any kinds of loopholes that would allow people to create other instances.

Answer 2

The first solution is (I believe) not thread-safe.

The second solution is (I believe) thread-safe, but might not work if you have complicated initialization dependencies in which MyClass.getInstance() is called before the MyClass static initializations are completed. That's probably the problem you were seeing.

Both solutions allow someone to create another instance of your (notionally) singleton class.

A more robust solution is:

class MyClass {

  private static MyClass instance;

  private MyClass() { }

  public synchronized static MyClass getInstance() {
    if (instance == null) {
      instance = new MyClass();
    }
    return instance;
  }
}

In a modern JVM, the cost of acquiring a lock is miniscule, provided that there is no contention over the lock.

EDIT @Nate questions my statement about static initialization order possibly causing problems. Consider the following (pathological) example:

public ClassA {
    public static ClassB myB = ClassB.getInstance();
    public static ClassA me = new ClassA();
    public static ClassA getInstance() {
        return me;
    }
}

public ClassB {
    public static ClassA myA = ClassA.getInstance();
    public static ClassB me = new ClassB();
    public static ClassB getInstance() {
        return me;
    }
}

There are two possible initialization orders for these two classes. Both result in a static method being called before the method's classes static initialization has been performed. This will result in either ClassA.myB or ClassB.myA being initialized to null.

In practice, cyclic dependencies between statics are less obvious than this. But the fact remains that if there is a cyclic dependency: 1) the Java compiler won't be able to tell you about it, 2) the JVM will not tell you about it. Rather, the JVM will silently pick an initialization order without "understanding" the semantics of what you are trying to do ... possibly resulting in something unexpected / wrong.

EDIT 2 - This is described in the JLS 12.4.1 as follows:

As shown in an example in §8.3.2.3, the fact that initialization code is unrestricted allows examples to be constructed where the value of a class variable can be observed when it still has its initial default value, before its initializing expression is evaluated, but such examples are rare in practice. (Such examples can be also constructed for instance variable initialization; see the example at the end of §12.5). The full power of the language is available in these initializers; programmers must exercise some care. ...