Nullpointer with singleton and factory pattern

Question

I have a problem which I can not understand for the life of me. Why do I get a null pointer here?

In principle I have three class. A singleton class, a factory class and a main. In the Main I call the Singleton class once and let me output something.

Then I call a static method on the factory. Within this factory class there is a static block, which in turn accesses the singleton class. At this moment, however, the instance of the singleton class is null.

I don't really understand this, because with the debugger I also see that the variable is null, but the flow of the calls is actually as expected.

Main.java

package testing;

public class Main {

    public static void main(String[] args) {
        Singleton.getInstance().getValue();

        Factory.build();
    }
}

Singleton.java

package testing;

public class Singleton {

    private static final Singleton INSTANCE = new Singleton();

    private String value;

    public static Singleton getInstance() {
        return INSTANCE;
    }

    private Singleton() {
        configureSingleton();

        Factory.build();
    }

    private void configureSingleton() {
        value = "TEST";
    }

    public String getValue() {
        return value;
    }

    public void setValue(String value) {
        this.value = value;
    }
}

Factory.java

package testing;

public class Factory {

    static {
        Singleton instance = Singleton.getInstance();

        System.out.println(instance.getValue());
    }

    private Factory() {

    }

    public static void build() {
        System.out.println("Building now");
    }
}

Answer 1

The problem is that you are calling Factory.build(); inside of the Singleton constructor. There could be a problem with the sequence of execution.

In particular: Factory.build(); is called before the assignment to INSTANCE happens.

Answer 2

Debunk

There is no point in using a method that is synchronized and does lazy loading. We know that code within the static block and static fields will be initialized when the class is loaded, and according to this answer, classes will only be loaded when you access a static field or invoke a static method on the Singleton. By accessing static fields/methods in @aran's answer, you are loading the class, therefore causing initialization. This is the same as calling getInstance() to test if lazy loading works, which does not make sense.

So how should I test it, anyways?

You could, use reflection on the system classloader to test if the class will be loaded, but @aran thinks it involves a "custom classloader". The more idiomatic approach would be to include a static block on the class that prints a text. See if it will print the text out:

public class Main {
    public static void main(String[] args) {
        System.out.println("Hello, world!");
    }
}

class Singleton {
    public static Singleton INSTANCE = new Singleton();
    private Singleton() {}
    static {
        System.out.println("This should not happen.");
    }
}

Original Answer

More detailed answer and how @aran's answer is not the root cause of the problem:

1. The constructor of the singleton is called when the class is loaded.
2. The constructor of the singleton class calls Factory.build()
3. After Factory.build in the constructor returns, the values is then assigned to the INSTANCE field.

The problem is Factory.build uses the INSTANCE field, before it is assigned (i.e. after the constructor returns.)

In general, private static Singleton INSTANCE = new Singleton(); works. Writing lazy loading yourself is more verbose, but not necessary. If you have other static methods that you will call during execution (which you shouldn't have because you have a singleton), then sure, @aran's answer will work for you.

Note

Please note that native implementations do need synchronization because it is not provided from the runtime. However, when you are operating on the JVM, class loading is already synchronized and initializing variable during class load is the best approach if you don't have bad practices like having static members of a singleton other than the INSTANCE field.

Answer 3

As a workaround, you could modify the behaviour of the Singleton class in order to avoid execution flow based success/fail. The modifications to be made are resumed below:

 public class Singleton {    

   private static Singleton INSTANCE = null;

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

   (...)
}

This avoids the eager initialization and the issues within execution flow. As a lazy-initialized singleton, this creates the instance when the first getInstance() is called, and only if this method is called. Synchronizing the method will guarantee thread safety.

This approach is based on Android's Singleton base source code below. Be aware that it could not be the best approach to solve your issue based on your specific context, which is a factory pattern, but it should work.

---

Android's Singleton base code

This is the source code of core.java.android.util.Singleton.java:

public abstract class Singleton<T> 
{
    @UnsupportedAppUsage
    public Singleton() {}

    @UnsupportedAppUsage
    private T mInstance;

    protected abstract T create();

    @UnsupportedAppUsage
    public final T get() 
    {
       synchronized (this) 
       {
            if (mInstance == null) 
                mInstance = create();
            
            return mInstance;
        }
    }
}