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

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

Why do we need to lock the add method after changing to ConcurrentHashMap? ? ?

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

It’s not locked.

EDIT:

There are indeed concurrency traps. Consider this situation:

1. Thread A executes map.get(key);if(value == null) to get the result true, and then hands over the CPU time.

2. At this time, thread B also executes to the same place, and the result is also true, because thread A has not yet executed map.put(key, 1), and thread B executes map.put(key, 1). At this time, map There is already a key value in it.

3. Thread A gets CPU time to continue execution. Because the previous judgment result was true, thread A put again. The final result is that both threads execute map.put(key, 1) once. At this time, the value of key is still 1, but it should actually be 2.

The reason why this problem exists is that a single operation of ConcurrentHashMap is atomic, but your external call is not atomic. map.get and map.put are two operations, independent of each other, so if you want to ensure thread Security still requires locking your code to ensure the atomicity of get and put operations.

Answer 2

ConcurrentHashMap only guarantees that its internal data can remain consistent under concurrent conditions, which HashMap cannot do.

But the add method is not thread-safe because it is a typical Check-Then-Act or Read-Modify-Write.

You can think about this question, if all fields in a class are thread-safe classes, then whether this class is thread-safe. The answer is obviously no.