Is this Command Pattern Thread-safe?

Question

Hi Stackoverflow Community,

I have a Question about Thread-safety. If I has a static Map and fill it out with different Object, are these Object Thread-safe, if i has only method they don't write in it?

I create a small Example: Is the return value of getCommand thread safe in this case?

How can I test Thread-safety with JUnit?

Controller

public class CommandController {

    private static Map<String, Command> commandMap = initMap();


    public static Map<String, Command> initMap() {
         return new HashMap<String, Command>() {{
             put("A", new CommandA());
             put("B", new CommandB());
         }};
    }


    public Command getCommand(String key) {
        if(commandMap.containsKey(key)) {
            return commandMap.get(key);
        }
        return null;
    }

}

Abstract Class

public abstract class Command {

    public abstract int calc(int value);

}

Command A

public class CommandB extends Command {
    @Override
    public int calc(int value) {
        value = value * 4;
        return value; 
    }
}

Command B

public class CommandA extends Command {
    private int ab = 5;

    @Override
    public int calc(int value) {
        return value * ab;
    }
}

Answer 1

This is thread-safe for two reasons. In this case, both need to be accounted for in order to have pure thread-safety

1. The map is immutable (since it's read only).
2. It's initialized with the class. Since class initialization is thread-safe and guarantees publication, visibility is not an issue.

Note: Slaks does bring up a good point. You should use final. Usually if you are worried about thread-safety and the field is neither final nor volatile, there is probably something wrong. Though making it final in this case does not make it more thread safe it just prevents something thread-unsafe happening in the future (like re assigning it).

Answer 2

Yes, this is thread safe, because class initialization is guaranteed to be visible to all threads that use the class, and your map is "effectively immutable"—its state doesn't change after class initialization.

However, if you initialized the map from some static method that your program invoked explicitly during a setup phase, you'd have to implement your own memory barrier to make sure other threads could see the correct state of the map. So, make sure the map is fully initialized during class initialization; that's what makes this work.