Ping
Ping

Reputation: 635

Are computeIfPresent and computeIfAbsent when used one after another considered as atomic operations?

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 :

  1. Thread A executes Line 1 (computeIfPresent) for key 1. Since key 1 is not present, aggregate function is not called on key 1 by Thread A.
  2. Thread A executes Line 2 (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

Answers (1)

user3453226
user3453226

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

Related Questions