I am trying to implement the following feature: each key in str array should be associated with an Integer which starts from 0 and will be stored in map. after execution the map should contains all keys in str and the count should be consistent with final value of 9. However the result varies from 8 to 12. What did I do wrong?
import java.util.HashMap;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicInteger;
public class Main {
public static final ConcurrentMap<String, Integer> map = new ConcurrentHashMap<>();
public static final AtomicInteger count = new AtomicInteger(0);
public static final String[] str = {
"a", "b", "c", "d", "e", "f", "g", "h"
};
public static void main(String[] args) throws InterruptedException {
for (int i = 0; i < 10; i++) {
ExecutorService exe = Executors.newFixedThreadPool(4);
for (int j = 0; j < 100; j++) {
exe.execute(() -> {
for (int k = 0; k < 1000; k++) {
int index = k % 8;
String key = str[index];
Integer value = map.get(key);
if (value == null) {
Integer next = count.incrementAndGet();
map.putIfAbsent(key, next);
}
}
});
}
exe.shutdown();
exe.awaitTermination(5, TimeUnit.SECONDS);
System.out.println("count = " + count.get());
}
}
}
You have a race condition here:
Integer value = map.get(key); // read
if (value == null) {
Integer next = count.incrementAndGet();
map.putIfAbsent(key, next); // write
}
If the key is set by another thread after the read and before the write, incrementAndGet()
will be executed, though it won't actually be inserted because putIfAbsent()
is atomic. You can do the conditional increment atomically with respect to the map by using computeIfAbsent()
:
map.computeIfAbsent(key, k -> count.incrementAndGet());