With double-checked locking, does a put to a volatile ConcurrentHashMap have happens-before guarantee?

Question

So far, I have used double-checked locking as follows:

class Example {
  static Object o;
  volatile static boolean setupDone;

  private Example() { /* private constructor */ }

  getInstance() {
    if(!setupDone) {
      synchronized(Example.class) {
        if(/*still*/ !setupDone) {
          o = new String("typically a more complicated operation"); 
          setupDone = true;
        }        
      }
    }
    return o;
  }
}// end of class

Now, because we have groups of threads that all share this class, we changed the boolean to a ConcurrentHashMap as follows:

class Example {
  static ConcurrentHashMap<String, Object> o = new ConcurrentHashMap<String, Object>();
  static volatile ConcurrentHashMap<String, Boolean> setupsDone = new ConcurrentHashMap<String, Boolean>();

  private Example() { /* private constructor */ }

  getInstance(String groupId) {
    if (!setupsDone.containsKey(groupId)) {
      setupsDone.put(groupId, false);
    }
    if(!setupsDone.get(groupId)) {
      synchronized(Example.class) {
        if(/*still*/ !setupsDone.get(groupId)) {
          o.put(groupId, new String("typically a more complicated operation")); 
          setupsDone.put(groupId, true); // will this still maintain happens-before?
        }        
      }
    }
    return o.get(groupId);
  }
}// end of class

My question now is: If I declare a standard Object as volatile, I will only get a happens-before relationship established when I read or write its reference. Therefore writing an element within that Object (if it is e.g. a standard HashMap, performing a put() operation on it) will not establish such a relationship. Is that correct? (What about reading an element; wouldn't that require to read the reference as well and thus establish the relationship?)

Now, with using a volatile ConcurrentHashMap, will writing an element to it establish the happens-before relationship, i.e. will the above still work?

_{Update: The reason for this question and why double-checked locking is important: What we actually set up (instead of an Object) is a MultiThreadedHttpConnectionManager, to which we pass some settings, and which we then pass into an HttpClient, that we set up, too, and that we return. We have up to 10 groups of up to 100 threads each, and we use double-checked locking as we don't want to block each of them whenever they need to acquire their group's HttpClient, as the whole setup will be used to help with performance testing. Because of an awkward design and an odd platform we run this on we cannot just pass objects in from outside, so we hope to somehow make this setup work. (I realise the reason for the question is a bit specific, but I hope the question itself is interesting enough: Is there a way to get that ConcurrentHashMap to use "volatile behaviour", i.e. establish a happens-before relationship, as the volatile boolean did, when performing a put() on the ConcurrentHashMap? ;)}

Answer 1

Yes, it is correct. volatile protects only that object reference, but nothing else.

No, putting an element to a volatile HashMap will not create a happens-before relationship, not even with a ConcurrentHashMap.

Actually ConcurrentHashMap does not hold lock for read operations (e.g. containsKey()). See ConcurrentHashMap Javadoc.

Update:

Reflecting your updated question: you have to synchronize on the object you put into the CHM. I recommend to use a container object instead of directly storing the Object in the map:

public class ObjectContainer {
    volatile boolean isSetupDone = false;
    Object o;
}

static ConcurrentHashMap<String, ObjectContainer> containers = 
    new ConcurrentHashMap<String, ObjectContainer>();

public Object getInstance(String groupId) {
  ObjectContainer oc = containers.get(groupId);
  if (oc == null) {
    // it's enough to sync on the map, don't need the whole class
    synchronized(containers) {
      // double-check not to overwrite the created object
      if (!containers.containsKey(groupId))
        oc = new ObjectContainer();
        containers.put(groupId, oc);
      } else {
        // if another thread already created, then use that
        oc = containers.get(groupId);
      }
    } // leave the class-level sync block
  }

  // here we have a valid ObjectContainer, but may not have been initialized

  // same doublechecking for object initialization
  if(!oc.isSetupDone) {
    // now syncing on the ObjectContainer only
    synchronized(oc) {
      if(!oc.isSetupDone) {
        oc.o = new String("typically a more complicated operation"));
        oc.isSetupDone = true;
      }        
    }
  }
  return oc.o;
}

Note, that at creation, at most one thread may create ObjectContainer. But at initialization each groups may be initialized in parallel (but at most 1 thread per group).

It may also happen that Thread T1 will create the ObjectContainer, but Thread T2 will initialize it.

Yes, it is worth to keep the ConcurrentHashMap, because the map reads and writes will happen at the same time. But volatile is not required, since the map object itself will not change.

The sad thing is that the double-check does not always work, since the compiler may create a bytecode where it is reusing the result of containers.get(groupId) (that's not the case with the volatile isSetupDone). That's why I had to use containsKey for the double-checking.

Answer 2

Therefore writing an element within that Object (if it is e.g. a standard HashMap, performing a put() operation on it) will not establish such a relationship. Is that correct?

Yes and no. There is always a happens-before relationship when you read or write a volatile field. The issue in your case is that even though there is a happens-before when you access the HashMap field, there is no memory synchronization or mutex locking when you are actually operating on the HashMap. So multiple threads can see different versions of the same HashMap and can create a corrupted data structure depending on race conditions.

Now, with using a volatile ConcurrentHashMap, will writing an element to it establish the happens-before relationship, i.e. will the above still work?

Typically you do not need to mark a ConcurrentHashMap as being volatile. There are memory barriers that are crossed internal to the ConcurrentHashMap code itself. The only time I'd use this is if the ConcurrentHashMap field is being changed often -- i.e. is non-final.

Your code really seems like premature optimization. Has a profiler shown you that it is a performance problem? I would suggest that you just synchronize on the map and me done with it. Having two ConcurrentHashMap to solve this problem seems like overkill to me.