Search code examples
javasingleton

Why does Singleton fail to run?


I'm attempting to make a thread-safe singleton. The program will run if I remove the synchronized function and uncomment the condition but then it won't be thread-safe, correct? I'm java a newbie (and new to working with threads).

class Singleton {
    volatile public static Singleton instance = null;
    private String someText;

    private Singleton() {
        this.someText = "Only one instance of this class can be created!";
    }

    public static Singleton getInstance() {

        synchronized (instance) {
            if(instance == null) {
                instance = new Singleton();
            }
        }

        // this will run
        // if (instance == null) {
        //  instance = new Singleton():
        // }

        return instance;
    }

    public String getSomeText() {
        return this.someText;
    }

    public void setSomeText(String text) {
        this.someText = text;
    }
}

public class Assignment1 {
    /*
        Assignment 1
        Implement a Singleton with double-checked locking.
    */

    public static void main(String[] args) {
        Singleton singleton1 = Singleton.getInstance();
        Singleton singleton2 = Singleton.getInstance();
        System.out.println(singleton1.getSomeText());
        System.out.println(singleton2.getSomeText());
        singleton1.setSomeText("Here's our new text!");
        System.out.println(singleton1.getSomeText());
        System.out.println(singleton2.getSomeText());
    }
}

Solution

  • The first time getInstance() is executed, the instance is null. You can't synchronize on a null object. The solution is to synchronize on the class itself.

    Try this:

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

    I'd recommend adding an additional check to prevent locking in case the instance is not null:

      if (instance == null) {
            synchronized (Singleton.class) {
                if (instance == null) {
                    instance = new Singleton();
                }
            }
        }