CODEWITHSUNDEEP
javaconcurrencysynchronizationfindbugs
Sachin Gorade
Sachin Gorade

Reputation: 1467

Findbugs - Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic

I have following code in my project

// Declaration

private ConcurrentHashMap<Long, ConcurrentHashMap<String, Object>> instanceSpecificBeanMap = new ConcurrentHashMap<>();

Use of map

ConcurrentHashMap<String, Object> beanMapForInstance = instanceSpecificBeanMap.get(currentInstanceId);
    Object beanObject = null;
    if(beanMapForInstance == null){
        synchronized (initLockObject) {
            beanMapForInstance = instanceSpecificBeanMap.get(currentInstanceId);
            if(beanMapForInstance == null){
                beanMapForInstance = new ConcurrentHashMap<>();
                instanceSpecificBeanMap.put(currentInstanceId, beanMapForInstance);
            }
        }
    }

I think above operation is atomic, although findbugs is showing it as issue? am I missing something here?

EDIT

beanMapForInstance is accessed from this class only. No other class has access to this variable.

Also, Currently we are using Java 7

Upvotes: 3

Views: 896

Answers (2)

Olivier Croisier
Olivier Croisier

Reputation: 6149

Use the putIfAbsent method. It is precisely intended to atomically put a new value in the map if none exist for the given key.

ConcurrentHashMap<String, Object> newMap = new ConcurrentHashMap<>();
ConcurrentHashMap<String, Object> beanMapForInstance = instanceSpecificBeanMap.putIfAbsent(currentInstanceId, newMap);
if (beanMapForInstance == null) {
    beanMapForInstance = newMap;
}
(... work with beanMapForInstance ...)

Upvotes: 3

David Harkness
David Harkness

Reputation: 36552

You can simplify this and solve the problem using computeIfAbsent from Java 8:

ConcurrentHashMap<String, Object> beanMapForInstance = 
        instanceSpecificBeanMap.computeIfAbsent(
            currentInstanceId, 
            k -> new ConcurrentHashMap<>()  // might need explicit generics
        );

Upvotes: 2

Related Questions