ConcurrentHashMap的非线程安全使用

问题

业务场景：应用会创建一个<name,id>的Map并缓存，其中key,value会被其他业务模块调用，最终数据落盘到HDFS上。

问题：发现一个奇怪的bug：id在Map中的值和业务表中的值有时候对不上，比如在业务表中查到一个id=100,但是在Map中找不到这个值。

经过分析定位，发现问题代码在这里：（大概逻辑为，如果key存在就获取value,如果不存在就+1并保存。）

Map<String, Integer> map = new ConcurrentHashMap<>();
AtomicInteger maxId = new AtomicInteger(0);

// init map and maxId ...

public int process(String name) {
    int id;
    if (map.containsKey(name)) {
        id = map.get(name);
    } else {
        id = maxId.incrementAndGet();
        map.put(name, id);
    }
    return id;
}

这里虽然使用了线程安全的ConcurrentHashMap和AtomicInteger,但是if判断条件会使线程安全失效。

举例，如果两个线程的name都是alice，依次处理，期望结果应该是Map中put了一条数据<alice,1>，然后两个线程的返回值都是1。
但是，如果两个线程同时进入if条件判断，然后都走到了else中，那么Map会put两次，最终留存下来一条数据<alice,2>,然后线程的返回值分为被id=1,id=2。

最终，id=1的这一条数据被覆盖了！

测试

使用UT可以重现这种情况：

  ExecutorService es = Executors.newFixedThreadPool(3);
  Future<Integer> f1 = es.submit(() -> module.process("alice"));
  Future<Integer> f2 = es.submit(() -> module.process("alice"));
  Future<Integer> f3 = es.submit(() -> module.process("bob"));

  logger.info(String.value0f(f1.get()));
  logger.info(String.value0f(f2.get()));
  logger.info(String.value0f(f3.get()));
  logger.info("===");
  module.getMap().forEach((k,v) -> logger.info(k+","+v));

结果如下：

1
2
3
===
alice,1
alice,2
bob,3

线程安全的话，alice对应的id应该是一样的，由此可见上述代码有漏洞。

修正

最直观的修正方法，就是把这段逻辑用synchronized包起来，然后ConcurrentHashMap和AtomicInteger都可以还原为简单的Map和Integer。但是这样的处理效率会低一些。

synchronized (this){
  // logic
}

另外，还可以使用ConcurrentHashMap的computeIfAbsent方法，它内部也使用了synchronized来保证线程安全，但是加锁的力度要比第一种方法细很多，效率也高一些。

return map.computeIfAbsent(name, v -> maxId.incrementAndGet());

详见 - https://enlear.academy/some-bugs-in-concurrenthashmap-you-should-know-eacc5e3cc209