Reputation: 635
Given the following class, if multiple threads are executing the testComputeIfPresentAndAbsent
method simultaneously, is the code thread safe? :
public class ComputeIfPresentAndAbsent {
private ConcurrentHashMap<String, MyPojo> map = new ConcurrentHashMap<>();
private void testComputeIfPresentAndAbsent(String key, MyPojo newObj) {
map.computeIfPresent(key, (k, existingObj) -> aggregate(existingObj, newObj));//Line 1
map.computeIfAbsent(key, k -> newObj);//Line 2
}
private MyPojo aggregate(MyPojo existingObj, MyPojo newPojo) {
newPojo.getField1().add(existingObj.getField1());
newPojo.getField2().add(existingObj.getField2());
return newPojo;
}
class MyPojo {
private BigDecimal field1;
private BigDecimal field2;
public BigDecimal getField1() {
return field1;
}
public void setField1(BigDecimal field1) {
this.field1 = field1;
}
public BigDecimal getField2() {
return field2;
}
public void setField2(BigDecimal field2) {
this.field2 = field2;
}
}
}
In other words, is calling computeIfPresent
and computeIfAbsent
one after another going to be an atomic operation or Is there a possibility of a race condition still occurring in this scenario?
If I had to simplify the question, consider the following chronology of events :
computeIfPresent
) for key 1. Since key 1 is not present, aggregate
function is not called on key 1 by Thread A.computeIfAbsent
) for key 1 and is in the process of adding the object against key 1. At the same time, Thread B comes in and executes line 1 (computeIfPresent
) for key 1.Question: Will thread B wait at Line 1 until thread A finishes executing Line 2 (computeIfAbsent
) and only then execute the aggregate
function? Or will Thread B immediately move on to Line 2 without waiting at Line 1? (Assuming both threads operate on the same key)
My understanding is that Thread B will not wait at Line 1 while Thread A is executing Line 2 for the same key. Is this understanding correct? If yes, then this code is not thread safe as multiple threads can miss calling the aggregate method altogether. Even if I was able to prove this theory through some sample program calling the testComputeIfPresentAndAbsent
method in 10000 threads, I am primarily interested in understanding why this code is not thread safe and whether my understanding is correct?
Upvotes: 3
Views: 900
Reputation:
Yes, your understanding is correct. computeIfAbsent()
/computeIfPresent()
methods on ConcurrentMap
do not employ locking. Instead, they work concurrently with other modifications.
Also note that the following is both non-thread-safe and bad practice:
private ConcurrentHashMap<String, MyPojo> map = new ConcurrentHashMap<>();
You have to mark the field final
if it will be accessed by other threads.
Also collections should be referenced by their interface:
private final ConcurrentMap<String, MyPojo> map = new ConcurrentHashMap<>();
Upvotes: 1