Can a ConcurrentHashMap be externally synchronized in Java?-CodePudding

I am using a ConcurrentHashMap, and ~~I need to perform some non atomic modifications to it~~ I need to iterate over all its elements when calculating a new element that is not present yet and do some other modifications possibly over the same map.

I wanted those operations be atomic, and block the ConcurrentHashMap to prevent from getting an exception derived from Concurrency.

The solution I programmed was to synchronize the ConcurrentHashMap object with itself as lock, but sonar throws a Major issue, so I do not know whether that solution is correct

proposed code:

Modification to the original text

public class MyClass<K, V> {
    ConcurrentHashMap<K, V> map = new ConcurrentHashMap<>();

    public V get(K key) {
        return map.computeIfAbsent(key, this::calculateNewElement);
    }

    protected V calculateNewElement(K key) {
        V result;
        // the following line throws the sonar issue:
        synchronized(map) {
            // calculation of the new element (assignating it to result)
            // with iterations over the whole map
            // and possibly with other modifications over the same map
        }
        return result;
    }
}

But this code triggers a Sonar major issue:

"Multi-threading - Synchronization performed on util.concurrent instance

findbugs:JLM_JSR166_UTILCONCURRENT_MONITORENTER

This method performs synchronization on an object that is an instance of a class from the java.util.concurrent package (or its subclasses). Instances of these classes have their own concurrency control mechanisms that are orthogonal to the synchronization provided by the Java keyword synchronized. For example, synchronizing on an AtomicBoolean will not prevent other threads from modifying the AtomicBoolean.

Such code may be correct, but should be carefully reviewed and documented, and may confuse people who have to maintain the code at a later date. "

CodePudding user response：

There is a method provided for atomic updates: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/ConcurrentHashMap.html#compute(K,java.util.function.BiFunction)

ConcurrentHashMap is built to allow a high degree of concurrent access. (See this article describing its inner workings) If an update is made to an entry using the provided means (such as compute, computeIfPresent, etc.) that should lock only the segment the entry is in, not the whole thing.

When you lock the whole map for an update you're not getting the benefit of using this specialized data structure. That's what Sonar is complaining about.

There is also the issue that readers have to do locking too, updater threads aren't the only ones that need to lock. This kind of thing is why CHM was invented in the first place.

CodePudding user response：

Thanks to all answers, I could finally program a solution.

public class MyClass<K, V> {
    ConcurrentHashMap<K, V> map = new ConcurrentHashMap<>();

    public V get(K key) {
        V result = map.get(key);
        if(result == null) {
            result = calculateNewElement(key);
        }
        return 
    }

    public synchronized void put(K key, V value) {
        map.put(key, value);
    }

    protected synchronized V calculateNewElement(K key) {
        V result = map.get(key);
        if(result == null) {
            // calculation of the new element (assignating it to result)
            // with iterations over the whole map
            // and possibly with other modifications over the same map

            put(key, result);
        }
        return result;
    }
}

As in my case put function is called seldom, the synchronization is acceptable.