Why does Singleton fail to run?

Question

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());
    }
}

By the way, an enum with one element is generally the best way to write a singleton in Java. — Basil Bourque, Oct 19 '21 at 21:34
thank you for the tip! I just took a quick glance at enums and have a question. Are they structured similarly to a class? Can an enum have a constructor, getter and to string method? — Mint, Oct 19 '21 at 21:46
Yes enums in java are basically just classes with a set of predifend instances. — magicmn, Oct 19 '21 at 22:33
@Mint See: [*What is an efficient way to implement a singleton pattern in Java?*](https://stackoverflow.com/q/70689/642706) — Basil Bourque, Oct 20 '21 at 01:10

score 3 · Accepted Answer · answered Oct 19 '21 at 19:42

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();
            }
        }
    }

Makes perfect since, thank you for your time! – Mint Oct 19 '21 at 21:13 — Mint, Oct 19 '21 at 21:13

Why does Singleton fail to run?

1 Answers1