下面这段代码存在并发陷阱？？？

Question

曾宪杰的《大型网站系统与Java中间件实践》第一章第1.2.2.3小节给出以下代码示例：

使用HashMap数据被进行统计;

public class TestClass
{
    private HashMap<String, Integer> map = new HashMap<>();
    
    public synchronized void add(String key)
    {
        Integer value = map.get(key);
        if(value == null)
        {
            map.put(key, 1);
        }
        else
        {
            map.put(key, value + 1);
        }
    } 
}

使用ConcurrentHashMap保存数据并进行统计;

public class TestClass
{
    private ConcurrentHashMap<String, Integer> map = new ConcurrentHashMap<>();
    
    public void add(String key)
    {
        Integer value = map.get(key);
        if(value == null)
        {
            map.put(key, 1);
        }
        else
        {
            map.put(key, value + 1);
        }
    } 
}

使用HashMap时，对add方法加锁，此时该方法是线程安全的，为何换为ConcurrentHashMap之后，原书中说存在并发陷阱？？？

Answer 1

为何换为ConcurrentHashMap之后，还要对add方法进行加锁？？？

public void add(String key)
    {
        Integer value = map.get(key);
        if(value == null)
        {
            map.put(key, 1);
        }
        else
        {
            map.put(key, value + 1);
        }
    } 

没加锁啊。

EDIT：

确实是存在并发陷阱。考虑一下这种情况：

1. 线程A执行map.get(key);if(value == null)得到结果true， 然后交出cpu时间。

2. 此时， 线程B也执行到同一个地方， 得到结果也为true， 因为线程A还没执行map.put(key, 1)， 线程B执行map.put(key, 1)， 此时map中已经有key的值了。

3. 线程A得到CPU时间继续执行， 因为之前判断结果是true， 所以线程A又put了一次。最终的结果就是两个线程都执行了一次map.put(key, 1)， 此时key的值依旧为1， 但实际上应该是2。

存在这个问题是原因是， ConcurrentHashMap的单个操作是原子性的， 但是你外部调用并不是原子性的， map.get 和 map.put 是两个操作， 相互独立的操作， 所以你如果要保证线程安全依旧需要在你的代码上加锁， 保证get和put两个操作的原子性。

Answer 2

ConcurrentHashMap只保证在并发情况下其内部数据能够保持一致，而这一点HashMap是做不到的。

但是add方法并不是线程安全，因为这是一个典型的Check-Then-Act或者Read-Modify-Write。

你可以思考这样一个问题，如果一个类里的所有field都是线程安全的类，那么这个类是否线程安全。答案显然是否定的。