Is it necessary to add the `volatile` keyword besides the `synchronized` one to make this class threadsafe?

Question

First, I had the following (here simplifed) class:

public class MyClass {

    private static Map<String, Object> objects = new HashMap<String, Object>();

    public static Object get(String key) {
        return objects.get(key);
    }

    public static void set(String key, Object object) {
        objects.put(key, object);
    }

}

Then, I wanted to make it treahsafe, so I tried the synchronized keyword as follow:

public class MyClass {

    private static Map<String, Object> objects = new HashMap<String, Object>();

    public static synchronized Object get(String key) {
        return objects.get(key);
    }

    public static synchronized void set(String key, Object object) {
        objects.put(key, object);
    }

}

The question is, is the synchronized keyword sufficient in my case, or is it necessary to add the volatile one, i.e.:

public class MyClass {

    private static volatile Map<String, Object> objects = new HashMap<String, Object>();

    public static synchronized Object get(String key) {
        return objects.get(key);
    }

    public static synchronized void set(String key, Object object) {
        objects.put(key, object);
    }

}

?

Answer 1

Making objects volatile will only have an impact if you reassign objects. In your example you don't so it won't make a difference and is unnecessary.

Note that it is good practice to enforce "non-reassignability" by making objects final.

In your case, you could simply delegate thread safety by using a thread safe map implementation (which would certainly scale better than your synchronized implementation).