Как решить findbug Последовательность обращений к java.util.concurrent.ConcurrentHashMap может быть не атомарной

Question

Привет, я получаю сообщение об ошибке "Последовательность обращений к java.util.concurrent.ConcurrentHashMap может быть не атомарным", когда я выполняю поиск ошибок в моем проекте для приведенного ниже кода.

public static final ConcurrentHashMap<String,Vector<Person>> personTypeMap = new ConcurrentHashMap<String, Vector<Person>>();

    private static void setDefaultPersonGroup() {


        PersonDao crud = PersonDao.getInstance();
        List<Person> personDBList = crud.retrieveAll();
        for (Person person : personDBList) {
            Vector<Person> personTypeCollection = personTypeMap.get(person
                    .getGroupId());
            if (personTypeCollection == null) {
                personTypeCollection = new Vector<Person>();
                personTypeMap.put(personTypeCollection.getGroupId(),
                        personTypeCollection);
            }
            personTypeCollection.add(person);
        }
    }

Я сталкиваюсь с проблемой в строке personTypeMap.put(personTypeCollection.getGroupId(), personTypeCollection);

Может ли кто-нибудь помочь мне решить проблему.

Answer 1

Сложные операции небезопасны в параллельной среде.

Какие сложные операции вы выполняете?

• 1) Вы проверяете, Map содержит вектор для ключа
• 2) Вы ставите новый Vector если значение не найдено

Так что это двухэтапное действие и оно сложное, поэтому оно небезопасно.

Почему они не безопасны?

Потому что они не атомные. Подумайте о сценарии, в котором у вас есть два потока.

Рассмотрим этот график:

Thread 1 --- checks for == null -> true                                           puts a new Vector

Thread 2 ---                      checks for ==null -> true    puts a new Vector                        

использование putIfAbsent() метод на ConcurrentHashMap, который предоставляет вам атомарное решение того, что вы пытаетесь выполнить.

ConcurrentHashMap#putIfAbsent()

Рекомендации:

• Правильное использование putIfAbsent

Answer 2

Это сообщение findbugs говорит вам, что в случае многопоточного доступа это небезопасно:

Вы получаете что-то от personTypeMap, проверяя, если это nullзатем добавьте новую запись, если это так. Здесь могут легко чередоваться две темы:

Тема 1: получить с карты
Тема 2: получить с карты
Thread1: проверить возвращаемое значение на ноль
Thread1: поставить новое значение
Thread2: проверить возвращаемое значение на ноль
Thread2: поставить новое значение

(Просто в качестве примера; в действительности порядок не задан - дело в том, что оба потока получают null тогда действуй на нем)

Вы должны создать новую запись, а затем позвонить personTypeMap.putIfAbsent() как это гарантирует атомарность.

Answer 3

В вашем случае ваш код должен выглядеть так:

public static final ConcurrentHashMap<String,Vector<Person>> personTypeMap = new ConcurrentHashMap<String, Vector<Person>>();

private static void setDefaultPersonGroup() {
    PersonDao crud = PersonDao.getInstance();
    List<Person> personDBList = crud.retrieveAll();
    for (Person person : personDBList) {
        // the putIfAbsent works in the same way of your
        //previous code, but in atomic way
        Vector<Person> personTypeCollection = personTypeMap.putIfAbsent(person
                .getGroupId());
        personTypeCollection.add(person);
    }
}