multithreading - Java multi-threading atomic reference assignment

Question

I have a cache which I implemented using a simeple HashMap. like -

HashMap<String,String> cache = new HashMap<String,String>();

This cache is used most of the time to read values from it. I have another method which reloads the cache and inside of this method I basically create a new cache and then assign the reference. As I understand assignment of object reference is Atomic in Java.

public class myClass {
     private HashMap<String,String> cache = null;
    public void init() {
       refreshCache();
    }
    // this method can be called occasionally to update the cache.
    public void refreshCache() {
        HashMap<String,String> newcache = new HashMap<String,String>();
       // code to fill up the new cache
       // and then finally
       cache = newcache; //assign the old cache to the new one in Atomic way
    }
}

I understand that if I do not declare cache as volatile, other threads will not be able to see the changes but it is not time critical for my use case to propagate the change in cache to other threads and they can continue to work with old cache for extended time.

Do you see any threading issue? Consider that many threads are reading from the cache and only at times the cache is reloaded.

EDIT- My main confusion is I do not have to use AtomicReference here as the assignment operation itself is atomic?

EDIT - I understand that to make the ordering proper, I should mark cache as volatile. But If refreshCache method is marked as synchronized, I do not have to make cache as volatile, as Synchronized block will take care of ordering as well as visibility?

Answer 1

It is not safe without a proper memory barrier.

One would think that the assignment of cache (cache = newCache) would happen after the steps to populate the cache. However, other threads may suffer from reordering of these statements so that the assignment may appear to happen before populating the cache. Thus, it is possible to grab the new cache before it's fully constructed or even worse see a ConcurrentModificationException.

You need to enforce the happens-before relationship to prevent this reordering, and declaring the cache as volatile would achieve that.