2

I know that ConcurentHasMap's compute method is atomic, but does it access the value under the same lock(node lock) every time? So more specifically are the following idioms equivalent regarding to the thread safe access of the map value, in this case Set? Can I fully replace the idiom3 with the idiom1?

 ConcurrentHashMap<String, Set<String>> concurrentMap = new ConcurrentHashMap<>();
Object setLock = new Object();

String testValueToAdd = "valueToAdd";
String testValueToRemove = "valueToRemove";

// idiom 1
// adding
concurrentMap.compute("test", (key, setForTheKey) -> {
  if(setForTheKey == null) {
    setForTheKey = new HashSet<>();
  }
  setForTheKey.add(testValueToAdd);
  return setForTheKey;
});

// removing
concurrentMap.compute("test", (key, setForTheKey) -> {
  if(setForTheKey != null) {
    setForTheKey.remove(testValueToRemove);
  }
  return setForTheKey;
});

// idiom 2, concurrent HashSet
// adding
concurrentMap.putIfAbsent("test", ConcurrentHashMap.newKeySet());
concurrentMap.get("test").add(testValueToAdd);

// removing
Set<String> set = concurrentMap.get("test");
if(set != null) {
  set.remove(testValueToRemove);
}

// idiom 3, normal Set
// adding
concurrentMap.putIfAbsent("test", new HashSet<>());
Set<String> normalSet = concurrentMap.get("test").
synchronized(setLock) {
  normalSet.add(testValueToAdd);
}

// removing
normalSet = concurrentMap.get("test");
if(normalSet!= null){ 
    synchronized(setLock) {
    normalSet.remove(testValueToRemove);
   }
}

If the following idioms are equivalent, which of them should be preferred? idiom3) manual locking -> error prone idiom2) concurrent hash set is little bit heavy, use locking inside idiom1) should be the same efficiency as idiom3, but without manual locking looks best to me.

HPCS
  • 1,434
  • 13
  • 22
  • 1
    If you ever need to do `concurrentMap.get(...).contains(...)` or some other operation on the inner `Set` object, then a lock around the computation function won't protect you: you need to lock the `Set` anyway, or else use a concurrent set. – Daniel Pryden Feb 23 '18 at 14:51
  • Yes, this I know. But from the compute source code method I am not sure, if its always lock the value under the same lock. Or if there is difference if from the compute method I returned some new value, or I returned the updated value like in this case. – HPCS Feb 23 '18 at 14:56
  • Your idioms 2 & 3 misuse #putIfAbsent. That method returns the previous value associated with the key, not the current value. Since the mapping is always null the first time you call it you'll always throw a NPE. – John H Feb 24 '18 at 02:49
  • Thanks I fix the bug. – HPCS Feb 24 '18 at 14:08
  • Possible duplicate of [Recursive ConcurrentHashMap.computeIfAbsent() call never terminates. Bug or "feature"?](https://stackoverflow.com/questions/28840047/recursive-concurrenthashmap-computeifabsent-call-never-terminates-bug-or-fea) – Vadzim Jun 30 '18 at 21:10

0 Answers0