значение из ConcurrentHashMap генерирует исключение NullPointerException

Question

Я поддерживаю многопоточный устаревший код, который использует ConcurrentHashMap.

Существуют операции добавления и удаления другими методами.

В следующем коде в некоторый момент после сбора нескольких значений с карты он выдает NullPointerException при выполнении synchronize(value).

public class MyClass{

    private final Map<MyObj, Map<String, List<String>>> conMap = new ConcurrentHashMap<>();

    //...

    public void doSomthing((MyObj id){
        List<Map<String, List<String>>> mapsList = new LinkedList<>();
        for(MyObj objId: conMap.keySet()){              
            if(objId.key1.equals(id.key1)){
                mapsList.add(conMap.get(objId));
            }
        }

        for(Map<String, List<String>> map: mapsList){
            synchronized(map){                   // <-- NullPointerException here
                //...
            }
    }

    //...

}

У меня такое ощущение, что, возможно, во время итерации в первом цикле записи удаляются. И когда линия:

mapsList.add(conMap.get(objId));

выполняется, objId больше не существует и mapsList добавляет ноль и, как результат, во время второго цикла NullPoinerException выбрасывается.

Есть ли другие причины получить это исключение?

Answer 1

Вы влюбились в анти-паттерн «Проверьте, затем-Действуйте».Это подразумевает проверку условия (например, наличие ключа) с последующим воздействием на него (например, вызовом get), игнорируя возможность того, что условие могло измениться между ними.

Таким образом, вы сталкиваетесь сопределенный ключ при итерации по conMap.keySet(), но к тому времени, когда вы вызываете conMap.get(objId), ключ может больше не отображаться на карте, о чем сообщается, возвращая null.

Настоятельно рекомендуетсяиспользуйте тип ключа, имеющий подходящую реализацию hashCode / equals, поэтому вам не нужно перебирать всю карту, чтобы найти совпадения, но можно использовать один get(id).

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

public void doSomething(MyObj id){
    // see https://stackoverflow.com/q/322715/2711488
    List<Map<String, List<String>>> mapsList = new ArrayList<>();

    for(Map.Entry<MyObj, Map<String, List<String>>> e: conMap.entrySet()){              
        if(e.getKey().key1.equals(id.key1)){
            mapsList.add(e.getValue());
        }
    }

    for(Map<String, List<String>> map: mapsList){
        synchronized(map) {
            //...
        }
    }
}