How to populate concurrenthashmap from multiple threads?

Question

I have a ConcurrentHashMap which I am populating from multiple threads.

private static Map<DataCode, Long> errorMap = new ConcurrentHashMap<DataCode, Long>();

public static void addError(DataCode error) {
    if (errorMap.keySet().contains(error)) {
        errorMap.put(error, errorMap.get(error) + 1);
    } else {
        errorMap.put(error, 1L);
    }
}

My above addError method is called from multiple threads which populates errorMap. I am not sure whether this is thread safe? Is there anything wrong I am doing here?

Any explanation of why it can skip updates will help me to understand better.

Answer 1

Whether this is safe depends on what you mean. It won't throw exceptions or corrupt the map, but it can skip updates. Consider:

1. Thread1: errorMap.get(error) returns 1
2. Thread2: errorMap.get(error) returns 1
3. Thread1: errorMap.put(error, 1+1);
4. Thread2: errorMap.put(error, 1+1);

A similar race exists around the keySet().contains(error) operation. To fix this you'll need to use atomic operations to update the map.

On Java 8, this is easy:

errorMap.compute(error, oldValue -> oldValue == null ? 1L : oldValue + 1L);

On older versions of Java you need to use a compare-and-update loop:

Long prevValue;
boolean done;
do {
  prevValue = errorMap.get(error);
  if (prevValue == null) {
    done = errorMap.putIfAbsent(error, 1L);
  } else {
    done = errorMap.replace(error, prevValue, newValue);
  }
} while (!done);

With this code, if two threads race one may end up retrying its update, but they'll get the right value in the end.

Alternately, you can also use Guava's AtomicLongMap which does all the thread-safety magic for you and gets higher performance (by avoiding all those boxing operations, among other things):

errorAtomicLongMap.incrementAndGet(error);