请教一个 ConcurrentHashMap 问题

[agzou]

public class IdGeneratorService {
    private final Map<String, AtomicLong> map = new ConcurrentHashMap<>();

    public long nextId(String key) {
        // 虽然采用了并发安全的容器，但是当 contains 语句通过后,有可能出现多线程先后 put,AtomicLong 值有可能给覆盖?
        if (!map.containsKey(key)) {
            AtomicLong atomicLong = new AtomicLong(0);
            map.put(key, atomicLong);
            return atomicLong.incrementAndGet();
        }
        return map.get(key).incrementAndGet();
    }
}

代码如上，如果并发调用 nextId()，我感觉即使使用了并发安全的容器,实际上这段代码也不是线程安全的，如果多线程访问，还是会出现 nextId()重复的问题，有可能 nextId 会出现多个 1 ？但是实际经过测试，并不会重现这个问题。。请教一下，这段代码是不是线程安全的，是否会生成重复 id?

测试代码

    public static void main(String[] args) throws InterruptedException {
        int count=2000;
        CountDownLatch cdl=new CountDownLatch(count);
        IdGeneratorService service = new IdGeneratorService();
        Map<Long, AtomicLong> countMap=new ConcurrentHashMap<>();
        for(long i=1;i<=count;i++){
            countMap.put(i,new AtomicLong());
        }
        for(int i=0;i<count;i++){
            new Thread(()->{
                long id = service.nextId("test");
                countMap.get(id).incrementAndGet();
                cdl.countDown();
            }).start();
        }
        cdl.await();
        boolean match = countMap.values().stream().mapToLong(AtomicLong::get).anyMatch(l->l>1);
        System.out.printf("id 重复=%b\n",match);
    }

感谢各位！

其实我的疑惑点是，我理解这段代码是会有线程问题的，但是我写的测试方法却没有测出来。

我后面重复运行了10来次测试方法，能测出id重复的情况。

结帖。

[JeromeCui]

public class IdGeneratorService {
private final Map<String, AtomicLong> map = new ConcurrentHashMap<>();

public long nextId(String key) {
if (!map.containsKey(key)) {
synchronized{
if (!map.containsKey(key)) {
AtomicLong atomicLong = new AtomicLong(0);
map.put(key, atomicLong);
}

}
}
return map.get(key).incrementAndGet();
}
}

[JeromeCui]

```
public class IdGeneratorService {
private final Map<String, AtomicLong> map = new ConcurrentHashMap<>();

public long nextId(String key) {
if (!map.containsKey(key)) {
synchronized{
if (!map.containsKey(key)) {
AtomicLong atomicLong = new AtomicLong(0);
map.put(key, atomicLong);
}

}
}
return map.get(key).incrementAndGet();
}
}
```

[justNoBody]

我理解这个和`ConcurrentHashMap`没有关系，因为你用的`incrementAndGet`方法使用了 CAS ，即便是多个线程都同时拿到了这个`AtomicLong`的实例也没有关系

[Georgedoe]

同一个 key 有可能会被 put 多次 , 某个 key 的 contains 和 put 不是原子操作 , 可以去看看 go 的 singleflight 的实现 , 保证一次只有一个线程执行了 set (put) 操作

[JeromeCui]

完了，格式错乱了

[agzou]

@JeromeCui #2 我的问题是，我觉得我这段代码不是线程安全的，但测试却不会生成重复的 id

[wolfie]

1. ID 不重复是因为 AtomicLog 。
2. 初始化 test 小概率重复创建，直接用 computeIfAbsent 。

[agzou]

@justNoBody #3 但是这两句
if (!map.containsKey(key)) {
AtomicLong atomicLong = new AtomicLong(0);
map.put(key, atomicLong);
return atomicLong.incrementAndGet();
}

有可能返回不同的两个 AtomicLong,这样调用 atomicLong.incrementAndGet(),应该会重复返回 1 ，但是我运行我的测试代码并没有重复 id

[Georgedoe]

在你代码里加了点 log , 这是输出 , 很显然有问题

public long nextId(String key) {
// 虽然采用了并发安全的容器，但是当 contains 语句通过后,有可能出现多线程先后 put,AtomicLong 值有可能给覆盖?
if (!map.containsKey(key)) {
AtomicLong atomicLong = new AtomicLong(0);
System.out.println("put twice");
map.put(key, atomicLong);
long l = atomicLong.incrementAndGet();
System.out.println(l);
return l;
}
return map.get(key).incrementAndGet();
}


put twice
put twice
put twice
1
1
2

[Kotiger]

正如四楼大佬所说，contains 和 put 组合在一起就不是安全操作了
public class IdGeneratorService {
private final Map<String, AtomicLong> map = new ConcurrentHashMap<>();

public long nextId(String key) {
// 直接用这个方法
map.computeIfAbsent(key, it->new AtomicLong(0));
return map.get(key).incrementAndGet();
}
}

[BBCCBB]

用 computeIfAbsent ,

有更复杂的场景, 就用 compute 方法, 不过这个方法更加的复杂

[BBCCBB]

你这完美避开了 concurrentHashMap 的特性.

[justNoBody]

@agzou 你的测试代码和你的`nextId()`方法逻辑是不同的，我不是很理解你具体想要问啥。

[documentzhangx66]

资源的并行安全，本质是操作该资源的业务逻辑，在并行中要保证唯一与串行。

当业务逻辑的唯一与串行，能够用 cas api 时，才会出现一行 cas api 语句就够了，比如经典的对同一个资源的 read & set 、compare & set 等等。

但很多业务逻辑，可能需要同时操作不同资源、或者有其他复杂的操作逻辑，此时就不能用 cas 了，而应该老老实实的串行化（锁定）代码段。

[ihuotui]

没有深刻理解原子操作含义，如果理解了就不会有疑问。