Multithreaded Java singleton keeps resetting

Question

I'm currently doing a Java exercise in the book "Head First Design Patterns" page 175 "The Chocolate Factory", and to test the theory that a singleton is thread safe I've implemented my own multithreaded driver class.

The exercise states: create a singleton chocolate boiler with 2 boolean variables: empty and boiled with a default state of empty=true and boiled=false. And three methods that write to the variables: fill(), drain() and boil().

However a problem arises when either reading or writing to the variables "empty" and "boiled". After thread #1 fills the ChocolateBoiler, it sets empty=false. Then thread #2 is launched and it says that empty is set to its default of true. How is this possible? Did thread #1 not update it? Or perhaps the output is incorrect, but the change propagated? I have configured double-checked locking on all accessor methods and the variables are set to static volatile.

I have appended the code below:

Client.Java

package creational.singleton.chocolatefactory;

public class Client extends Thread{

    public void run() {
        ChocolateBoiler boiler = ChocolateBoiler.getInstance();
        System.out.println("new Boiler " + Thread.currentThread().getId() + "= isBoiled: " + boiler.isBoiled() + ", isEmpty: " + boiler.isEmpty());
        boiler.fill();
        System.out.println("filled Boiler " + Thread.currentThread().getId() + "= isBoiled: " + boiler.isBoiled() + ", isEmpty: " + boiler.isEmpty());
        boiler.boil();
        System.out.println("boiled Boiler " + Thread.currentThread().getId() + "= isBoiled: " + boiler.isBoiled() + ", isEmpty: " + boiler.isEmpty());
    }

    public static void main(String[] args) {
        Client obj = new Client();
        Thread t1 = new Thread(obj);
        Thread t2 = new Thread(obj);
        Thread t3 = new Thread(obj);

        t1.start();
        t2.start();
        t3.start();
    }
}

and ChocolateBoiler.java

package creational.singleton.chocolatefactory;

public class ChocolateBoiler {
    //volatile guarantees visibility of changes to variables across threads
    //eager initialization for better thread safety
    private volatile static ChocolateBoiler uniqueInstance = new ChocolateBoiler();
    private volatile static boolean empty = true;
    private volatile static boolean boiled = false;

    private ChocolateBoiler() {}

    public static ChocolateBoiler getInstance() {
        return uniqueInstance;
    }

    public void fill() {
        if (isEmpty()) {
            synchronized(uniqueInstance){
                if (isEmpty()) {
                    ChocolateBoiler.empty = false;
                    ChocolateBoiler.boiled = false;
                }
            }
        }
    }

    public void drain(){
        if (!isEmpty() && isBoiled()) {
            synchronized(uniqueInstance){
                if (!isEmpty() && isBoiled()) {
                    ChocolateBoiler.empty = true;
                }
            }
        }
    }

    public void boil(){
        if (!isEmpty() && !isBoiled()) {
            synchronized(uniqueInstance){
                if (!isEmpty() && !isBoiled()) {
                    ChocolateBoiler.boiled = true;
                }
            }
        }
    }

    public boolean isEmpty() {
        synchronized(uniqueInstance){
            return ChocolateBoiler.empty;
        }
    }

    public boolean isBoiled() {
        synchronized(uniqueInstance){
            return ChocolateBoiler.boiled;
        }
    }
}

The output is as follows:

new Boiler 20= isBoiled: false, isEmpty: true
filled Boiler 20= isBoiled: false, isEmpty: false
new Boiler 19= isBoiled: false, isEmpty: true

Notice the last line says: isEmpty: true

Shouldnt it say: isEmpty: false

========================================

= Solution

The problem was with the Client.java

package creational.singleton.chocolatefactory;

public class Client extends Thread{
    ChocolateBoiler boiler = ChocolateBoiler.getInstance();

    public void run() {
        printState("new");
        boiler.fill();
        printState("filled");
        boiler.boil();
        printState("boiled");
    }

    public synchronized void printState(String state){
        System.out.println(state + " Boiler " + Thread.currentThread().getId() + "= isBoiled: " + boiler.isBoiled() + ", isEmpty: " + boiler.isEmpty());
    }

    public static void main(String[] args) {
        Client obj = new Client();
        Thread t1 = new Thread(obj);
        Thread t2 = new Thread(obj);
        Thread t3 = new Thread(obj);

        t1.start();
        t2.start();
        t3.start();
    }
}

It now outputs as follows:

new Boiler 19= isBoiled: false, isEmpty: true
filled Boiler 19= isBoiled: false, isEmpty: false
new Boiler 20= isBoiled: true, isEmpty: false
new Boiler 21= isBoiled: true, isEmpty: false
filled Boiler 21= isBoiled: true, isEmpty: false
filled Boiler 20= isBoiled: true, isEmpty: false
boiled Boiler 19= isBoiled: true, isEmpty: false
boiled Boiler 20= isBoiled: true, isEmpty: false
boiled Boiler 21= isBoiled: true, isEmpty: false

Answer 1

Although individual methods are synchronized, the line System.out.println("boiled Boiler " + Thread.currentThread().getId() + "= isBoiled: " + boiler.isBoiled() + ", isEmpty: " + boiler.isEmpty()); makes two separate method calls, and in a concurrent context, nothing can be guaranteed.

Try calling System.out.println in a synchronized block too.