CODEWITHSUNDEEP
javaconcurrencyhashmap
BJ Dela Cruz
BJ Dela Cruz

Reputation: 5354

Necessary to synchronize a concurrent HashMap when calling values()?

In the following code:

private final Map<A, B> entriesMap = Collections
  .synchronizedMap(new HashMap<A, B>());

// ...

List<B> entries = new ArrayList<>(this.entriesMap.values());

If entriesMap is being accessed/modified by multiple threads in other methods, is it necessary to synchronize on entriesMap? In other words:

List<B> entries;

synchronize (this.entriesMap) {
  entries = new ArrayList<>(this.entriesMap.values());
}

If I am correct, values() is not an atomic operation, unlike put() and get(), right?

Thanks!

Upvotes: 2

Views: 515

Answers (4)

Thilo
Thilo

Reputation: 262684

Well, calling values might well be an atomic operation, but the Collection it returns is not a snapshot copy, but backed by the underlying Map, so it will croak when there are concurrent modifications to the Map as you iterate it afterwards (when copying it into the ArrayList).

Note that this (ConcurrentModificationException) also happens when there is just one thread, as long as you iterate the values and modify the Map in an interleaved fashion, so this is not really a problem of thread synchronization.

Further note that there is ConcurrentHashMap, which does provide for a snapshot iterator, that you can iterate while modifying the Map (modifications are not reflected in the iterator). But even with ConcurrentHashMap, the Collection of values() is not a snapshot, it works just like for the normal HashMap.

Upvotes: 1

amicngh
amicngh

Reputation: 7899

Collections.synchronizedMap() guarantees that each atomic operation you want to run on the map will be synchronized. and values() is also atomic , but when you are putting values to ArrayList that will not be synchronized in this case you need to synchronized the list also.

Upvotes: 0

Benoit
Benoit

Reputation: 1993

Yes, you are right.

When put the values of the map in the ArrayList, an iteration is performed on the values. So you need the synchronized block. See this page.

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1502056

The problem is that even if values() itself were atomic, the act of iterating over it is isn't. The ArrayList constructor can't take a copy of the values in an atomic way - and the iterator will be invalidated if another thread changes the map while it's copying them.

Upvotes: 1

Related Questions