Java ConcurrentHashMap.computeIfPresent value modification visibility

Question

Let's say I have a concurrent map with collections as value:

Map<Integer, List<Integer> map = new ConcurrentHashMap<>();
map.putIfAbsent(8, new ArrayList<>());

and I update the value as follows:

map.computeIfPresent(8, (i, c) -> {
    c.add(5);
    return c;
});

I know that computeIfPresent entire method invocation is performed atomically. However, considering this map is accessed by multiple threads concurrently, I'm a little bit concerned about data visibility of the modifications done to the underlying collection. In this case will the value 5 be seen in the list after calling map.get

My question is will change to the list be visible in other threads upon calling map.get if changes are performed within computeIfPresent method call.

Please note that I am aware that changes to the list will not be visible if I were to take reference to the list before doing the update operation. I am unsure if the changes to the list will be visible if I take reference to the list (by calling map.get) after the update operation.

I am unsure how to interpret the docs, but it seems to me that happens-before relationship will guarantee visibility of the changes to the underlying collection in this particular case

More formally, an update operation for a given key bears a happens-before relation with any (non-null) retrieval for that key reporting the updated value

Answer 1

The fact that that method is documented to be atomic, does not mean much about visibility (unless that is part of the documentation). For example to make this simpler:

// some shared data
private List<Integer> list = new ArrayList<>();

public synchronized void addToList(List<Integer> x){
     list.addAll(x);
}

public /* no synchronized */ List<Integer> getList(){
     return list;
}

We can say that addToList is indeed atomic, only one thread at a time can call it. But once a certain thread calls getList - there is simply no guarantee about visibility (because for that to be established, it has to happens on the same lock ). So visibility is something happens before is concerned with and computeIfPresent documentation does not say anything about that, at all.

Instead the class documentation says:

Retrieval operations (including get) generally do not block, so may overlap with update operations (including put and remove).

The key point here is obviously overlap, so some other threads calling get (thus getting a hold of that List), can see that List in some state; not necessarily a state where computeIfPresent started (before you actually called get). Be sure to read further to understand what that some actually might mean.

And now to the trickiest part of that documentation:

Retrievals reflect the results of the most recently completed update operations holding upon their onset. More formally, an update operation for a given key bears a happens-before relation with any (non-null) retrieval for that key reporting the updated value.

Read that sentence about completed again, what it says is that the only thing you can read when a thread does get is the last completed state that List was in. And now the next sentence says that there is a happens before established between two actions.

Think about it, a happens-before is established between two subsequent actions (like in the synchronized example above); so internally when you update a Key there could be a volatile written signaling that update has finished (I am pretty sure it's not done this way, just an example). For the happens before to actually work, get has to read that volatile and see the state that was written to it; if it sees that state it means that happens before has been established; and I guess that by some other technique this is actually enforced.

So to answer your question, all the threads calling get will see the last completed action that happened on that key; in your case if you can guarantee that order, I'd say, yes, they will be visible.

Answer 2

c.add(5) is not thread safe, the internal state of the c is not protected by the map.

The exact way of making individual values and insert-use-remove combinations thread safe and race condition free depends on the usage pattern (synchronized wrapper, copy-on-write, lock free queue, etc).

Answer 3

To clarify your question:

You're providing some external guarantee such that Map.computeIfPresent() is called before Map.get().

You haven't stated how you're doing this, but let's say you're doing it by using something with happens-before semantics provided by the JVM. If this is the case then that alone guarantees List.add() is visible to the thread calling Map.get() simply by association of the happens-before relationship.

Now to answer the question you're actually asking: As you've noted, there's a happens-before relation between update operation ConcurrentHashMap.computeIfPresent() and the subsequent calling of the access method ConcurrentMap.get(). And naturally, there's a happens-before relation between List.add() and the end of ConcurrentHashMap.computeIfPresent().

Put together, answer is yes.

There's a guarantee the other thread will see 5 in the List obtained through Map.get(), provided you're guaranteeing Map.get() is actually called after computeIfPresent() ends (as stated in the question). If the latter guarantee goes foul and Map.get() is somehow called before computeIfPresent() ends, there's no guarantees to what the other thread will see since ArrayList is not thread-safe.