javaconcurrency

Reading mutable objects in a ConcurrentHashMap


As per the JavaSE 8 docs on the ConcurrentHashMap

... even though all operations are thread-safe, retrieval operations do not entail locking....

How does the above play with the compute function of the ConcurrentHashMap ? Consider the following simple object

final class PlainObject{
  int a;
  int b;
}

Consider the following mapping

ConcurrentHashMap<String, PlainObject> myMap;

Consider the following function that has access to myMap

void act(){

  myMap.compute("abc", (k, v) -> {
    v.a = 21;
    // Other work 
    v.b = 18;

    return v;
  });
}

Can I ever perform a read on myMap that will print a partial result (i.e. only a has been modified) ? It seems the answer is yes because retrieving data does not entail locking and we are dealing with a mutable object. Is this conclusion correct?


Solution

  • You are correct. The get() operation is "thread safe" in the sense that it is guaranteed either to return the PlainObject that was present for the given k before the map was updated or, return the new object that was added under that k. It will never return some other object, or cause a program crash.

    BUT

    The fact that the map itself is thread safe in no way protects the objects that you put in the map. If you want your program to see consistent values of v.a and v.b when one thread wants to examine the object concurrently with some other thread that modifies the object, then that's entirely up to you to provide that protection.


    update:

    Your example does not use compute in the way it was meant to be used. The function you provide never returns a different v from the one that already was in the map. The code that you wrote doesn't do anything different from this version:

    void act(){
      PlainObject v = myMap.get("abc");
      if (v != null) {
        v.a = 21;
        // Other work
        v.b = 18;
      }
    }
    

    The purpose of compute is to let you write a function that decides whether or not the object in the map should be replaced with a different object:

    
    void foo() {
      myMap.compute("abc", (k, v) -> {
        if ((v == null) || ...other reason not to update...) {
          return /*the original, UNMODIFIED*/ v;
        }
        else {
          PlainObject new_v = new PlainObject();
          new_v.a = 21;
          // Other work 
          new_v.b = 18;
          return new_v;
        }
      });
    

    In this case, if some other thread calls myMap.get("abc"), it is guaranteed either to get the original unmodified PlainObject or, to get the new object. And, if your PlainObject instances are all effectively immutable,* then the new object already will have been safely published† by the time the get() function returns it.

    * It would be "effectively immutable" if no thread ever modifies a PlainObject after the object has been made available to other threads.

    † "Safely published" means there is a "happens before" relationship between the initialization of the new object and the first opportunity that any other thread has to access it.