Strange behavior in a code to test a thread-safe singleton

Question

I'm rather new to multi-threading in Java. As I need a thread-safe singleton (which I implemented as an enum), I wrote a small test-code which produces a strange output.

The Code:

public enum EnumSingleton {
    INSTANCE;


    /** state variables */
    private String message;

    /** Constructor */
    private EnumSingleton() {
    }

    /** add well-known accessor for the instance  (is NOT necessary) */
    public static EnumSingleton getInstance() {
        return INSTANCE;
    }


    /** Accessors */
    public String getMessage() {
        return message;
    }

    public void setMessage(String name) {
        this.message = name;
    }
}

public class App {

    public static void main(String[] args) {

        for (int i = 0; i < 10; i++) {
            final int b = i;
            Thread thread = new Thread("Thread #" + b) {
                @Override
                public void run() {
                    EnumSingleton singleton = EnumSingleton.getInstance();
                    singleton.setMessage("Message written by "+this.getName());
                    System.out.println("Current thread "+this.getName() + ": "+singleton.getMessage());
                }
            };
            thread.start();
        }
    }
}

So every thread writes its name in the property "message" of the enum which is then printed to STDOUT. I get the following output which I find strange:

Current thread Thread #6: Message written by Thread #3
Current thread Thread #1: Message written by Thread #1
Current thread Thread #8: Message written by Thread #8
Current thread Thread #5: Message written by Thread #1
Current thread Thread #4: Message written by Thread #4
Current thread Thread #9: Message written by Thread #9
Current thread Thread #7: Message written by Thread #3
Current thread Thread #2: Message written by Thread #3
Current thread Thread #0: Message written by Thread #3
Current thread Thread #3: Message written by Thread #3

What I expected is that i get for every loop counter (0-9) a message. But in this example I have multiple messages written by Thread #3, how can that be? Is there a race-condition?

If my code is crap: How do I correctly test my singleton for thread-safety?

Answer 1

You have a clear race condition here as you have a variable message inside the singleton instance of the enum. Your threads are all writing to and reading from that variable at the same time so you would expect to see results like this.

The enum construct means that the creation of your singleton object is thread safe, however calls to methods within it still need to be properly handled.

The way to do what you are looking for would be to have message be a thread local variable or to place the setting of the message and the reading of it inside a single synchronized block, probably locking on the singleton object.

Answer 2

You singleton is not thread safe - you have not done anything to guarantee the visibility of the message variable.

You could make it volatile for that. Note however that the output could be many different things - in particular you won't necessarily get one Message written by Thread #i for each i.