In Java, is it safe to change a reference to a HashMap read concurrently

I hope this isn't too silly a question...

I have code similar to the following in my project:

public class ConfigStore {

    public static class Config {

        public final String setting1;
        public final String setting2;
        public final String setting3;

        public Config(String setting1, String setting2, String setting3) {
            this.setting1 = setting1;
            this.setting2 = setting2;
            this.setting3 = setting3;
        }

    }

    private volatile HashMap<String, Config> store = new HashMap<String, Config>();

    public void swapConfigs(HashMap<String, Config> newConfigs) {
        this.store = newConfigs;
    }

    public Config getConfig(String name) {
        return this.store.get(name);
    }

}

As requests are processed, each thread will request a config to use from the store using the getConfig() function. However, periodically (every few days most likely), the configs are updated and swapped out using the swapConfigs() function. The code that calls swapConfigs() does not keep a reference to the Map it passes in as it is simply the resul开发者_开发百科t of parsing a configuration file.

• In this case, is the volatile keyword still needed on the store instance variable?
• Will the volatile keyword introduce any potential performance bottlenecks that I should be aware of or can avoid given that the rate of reads greatly exceeds the rate of writes?

Thanks very much,

---

Since changing references is an atomic operation, you won't end up with one thread modifying the reference, and the other seeing a garbage reference, even if you drop volatile. However, the new map may not get instantly visible for some threads, which may consequently keep reading configuration from the old map for an indefinite time (or forever). So keep volatile.

Update

As @BeeOnRope pointed out in a comment below, there is an even stronger reason to use volatile:

"non-volatile writes [...] don't establish a happens-before relationship between the write and subsequent reads that see the written value. This means that a thread can see a new map published through the instance variable, but this new map hasn't been fully constructed yet. This is not intuitive, but it's a consequence of the memory model, and it happens in the real word. For an object to be safely published, it must be written to a volatile, or use a handful of other techniques.

Since you change the value very rarely, I don't think volatile would cause any noticeable performance difference. But at any rate, correct behaviour trumps performance.

---

No, this is not thread safe without volatile, even apart from the issues of seeing stale values. Even though there are no writes to the map itself, and reference assignment is atomic, the new Map<> has not been safely published.

For an object to be safely published, it must be communicated to other threads using some mechanism that either establishes a happens-before relationship between the object construction, the reference publication and the reference read, or it must use a handful of narrower methods which are guaranteed to be safe for publishing:

• Initializing an object reference from a static initializer.
• Storing a reference to it into a final field.

Neither of those two publication specific ways applies to you, so you'll need volatile to establish happens-before.

Here is a longer version of this reasoning, including links to the JLS and some examples of real-world things that can happen if you don't publish safely.

More details on safe publication can be found in JCIP (highly recommended), or here.

---

Your code is fine. You need volatile, otherwise your code would be 100% thread-safe (updating a reference is atomic), however the change might not be visible to all the threads. It means some threads will still see the old value of store.

That being said volatile is obligatory in your example. You might consider AtomicReference, but it won't give you anything more in your case.

You cannot trade correctness for performance so your second question is not really valid. It will have some performance impact, but probably only during update, which happens very rarely as you said. Basically JVM will ensure the change is visible to all the threads by "flushing" it, but after that it will be accessible as any other local variable (up until next update).

BTW I like Config class being immutable, please also consider immutable Map implementation just in case.

---

Would it work for you to use a ConcurrentHashMap and instead of swapping the entire config update the affected values in the hash map?