How to maintain atomicity with ConcurrentHashMap get and put methods?

Question

In a multi-threaded environment, I am performing get and put operations on a ConcurrentHashMap implementation. However, the results are un-expected. Please find below code and output.

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

public class Test {

    private static final List<String> bars = Arrays.asList("1","2","3","4","5","6","7","8","9","10");
    private static final String KEY = UUID.randomUUID().toString();
    private static ExecutorService executorService = null;

    public static void main(String[] args) {
        for (int i = 1; i <= 20; i++) {
            executorService = Executors.newFixedThreadPool(2);
            Map<String, AtomicInteger> map = new ConcurrentHashMap<>();
            performMapOps(map);
            executorService.shutdown();
            try {
                while (!executorService.awaitTermination(1, TimeUnit.SECONDS));
                System.out.println(map.get(KEY));
            } catch (Exception e) {
                e.printStackTrace();
            }
        }
    }

    private static void performMapOps(Map<String, AtomicInteger> map) {
        for (int i = 1; i <= bars.size(); i++) {
            executorService.execute(() -> ops(map));
        }
    }

    private static void ops(Map<String, AtomicInteger> map) {
       if (!map.containsKey(KEY)) {
            AtomicInteger atomicInteger = new AtomicInteger(1);
            map.put(KEY, atomicInteger);
        } else {
            map.get(KEY).set(map.get(KEY).intValue() + 1);
        }
    }
}

OUTPUT - It should be 10 always, However it is not true for the above code. Please find below output.

10
10
10
10
10
10
10
10
10
10
10
10
10
10
10
10
10
9
10
10

Sometimes, it gives me value other than 10. Please help to understand, why this un-expected behaviour and how to fix this?

Answer 1

Your code has a race condition. Two threads can decide at same time that !map.containsKey(KEY) is false and assign different new AtomicInteger(1) to the map.

Two threads can decide at same time that !map.containsKey(KEY) is true and they both could evaluate map.get(KEY) as the same value, so store same new value with .set(map.get(KEY).intValue() + 1).

An atomic operation for update can be achieved by use of Map.computeIfAbsent() with AtomicInteger.incrementAndGet() to ensure consistent counter incrementing without the value being updated by another thread:

private static void ops(Map<String, AtomicInteger> map) { 
    map.computeIfAbsent(KEY, k -> new AtomicInteger()).incrementAndGet();
}