Исправить проблемы в коде

17. Исправить проблемы в коде

Условие задачи:
🔥 В коде используется ConcurrentHashMap, но операция добавления значения написана некорректно с точки зрения многопоточности. Нужно понять, почему код может работать неправильно на проде, и исправить его.

import java.util.concurrent.ConcurrentHashMap;

class Scratch {
    void main() {

        var concurrentMap = new ConcurrentHashMap<String, String>();

        new Thread(() -> putValue(concurrentMap, "key", "value1")).start();
        new Thread(() -> putValue(concurrentMap, "key", "value2")).start();

    }

    private void putValue(ConcurrentHashMap<String, String> map, String key, String value) {
        if (!map.containsKey(key)) {
            map.put(key, value);
        }
    }
}

Спойлеры к решению

Подсказки

💡 ConcurrentHashMap делает отдельные операции потокобезопасными, но не делает атомарной связку containsKey() + put().
💡 Между проверкой containsKey() и вызовом put() другой поток может успеть записать значение по тому же ключу.
💡 В результате один поток может перезаписать значение другого.
💡 Для атомарной операции “положить, если ключа ещё нет” нужно использовать putIfAbsent().
💡 Альтернатива — использовать computeIfAbsent(), если значение нужно вычислять лениво.

Решение

import java.util.concurrent.ConcurrentHashMap;

class Scratch {
    void main() {
        var concurrentMap = new ConcurrentHashMap<String, String>();

        new Thread(() -> putValue(concurrentMap, "key", "value1")).start();
        new Thread(() -> putValue(concurrentMap, "key", "value2")).start();
    }

    private void putValue(ConcurrentHashMap<String, String> map, String key, String value) {
        map.putIfAbsent(key, value);
    }
}

Что было не так #

Проблема в том, что этот код выглядит потокобезопасным, потому что используется ConcurrentHashMap:

if (!map.containsKey(key)) {
    map.put(key, value);
}

Но на самом деле потокобезопасны только отдельные операции containsKey() и put(), а не их комбинация.

Например:

Thread 1: проверил containsKey("key") → false
Thread 2: проверил containsKey("key") → false
Thread 1: put("key", "value1")
Thread 2: put("key", "value2")

В итоге value2 перезапишет value1, хотя логика метода подразумевает: “записать значение только если ключа ещё нет”.

Правильное решение — использовать атомарный метод:

map.putIfAbsent(key, value);

Он гарантирует, что проверка наличия ключа и запись значения выполняются как одна атомарная операция.

Дополнительный вариант #

Если значение нужно не просто передать, а вычислить только при отсутствии ключа, можно использовать computeIfAbsent():

private void putValue(ConcurrentHashMap<String, String> map, String key, String value) {
    map.computeIfAbsent(key, k -> value);
}

Но для данного примера достаточно putIfAbsent().

Улучшенный вариант с ожиданием потоков #

Чтобы пример был более корректным для демонстрации, можно дождаться завершения потоков:

import java.util.concurrent.ConcurrentHashMap;

class Scratch {
    void main() throws InterruptedException {
        var concurrentMap = new ConcurrentHashMap<String, String>();

        Thread t1 = new Thread(() -> putValue(concurrentMap, "key", "value1"));
        Thread t2 = new Thread(() -> putValue(concurrentMap, "key", "value2"));

        t1.start();
        t2.start();

        t1.join();
        t2.join();

        System.out.println(concurrentMap);
    }

    private void putValue(ConcurrentHashMap<String, String> map, String key, String value) {
        map.putIfAbsent(key, value);
    }
}

Так в мапе останется только одно из значений — то, которое первый поток успел записать. Главное: второе значение уже не сможет перезаписать первое.