Safely iterate an ConcurrentHashMap that is accessed by multiple thread

Question

I have a ConcurrentHashMap object which is shared by multiple threads to access:

Map<String, MyConnection> myMap = new ConcurrentHashMap<String, MyConnection>();

The class MyConnection contains some connection to the datasource.

At a later stage, I need to iterate the ConcurrentHashMap, call MyConnection.close() method and remove it from the Map. Something like this:

for(String key : myMap.ketSet()) {
    MyConnection connection = myMap.get(key);
    connection.close();
    myMap.remove(key);
}

However, the myMap is shared by multiple thread, which can be adding, removing from the ConcurrentHashMap at the same time.

How can I make sure the for loop above is thread safe to run? There is not requirement that the loop has to remove all the entries in the map. The loop only needs to remove all the elements at the time myMap.keySet() is called. Any subsequent elements added to the map does not have to be removed.

Obviously I can lock the entire for loop and prevents other thread from touching the map, but I think it is not performance efficient.

Can someone please share your opinion?

EDIT1 : And How about this? Is this thread safe?

for(MyConnection connection : myMap.values()) {
    connection.close();
    myMap.remove(connection.getId());
}

Each connection object has a ID, which is also the key of that entry.

This is most likely a duplicate of the following. In short you can use an iterator and you need to call the remove method on this iterator (not the map): http://stackoverflow.com/questions/3768554/is-iterating-concurrenthashmap-values-thread-safe — eckes, Sep 28 '14 at 19:52
@eckes I disagree. The accepted answer for that question claims that iterating over a ConcurrentHashMap is usually safe, which is correct for a given value of "usually". But Kevin's usage is not safe, so he is right to ask a separate question. — MikeFHay, Sep 28 '14 at 20:01
Thanks for your reply. I certainly get some hints from the other question @eckes mentioned. My requirement here is simple. All I need is to safely remove the objects and leave the map in a valid state. If another thread added a new entry while the other thread is looping, it is fine to leave the newly added entry in the map. If so, is the loop above safe? Or I have to use iterator as mentioned in the other question? — Kevin, Sep 28 '14 at 20:12

score 1 · Answer 1 · answered May 17 '19 at 13:36

1

The correct way to do it is to use the return value of remove:

for(String key : myMap.ketSet()) {
    MyConnection connection = myMap.remove(key);
    if (connection != null)
        connection.close();
}

answered May 17 '19 at 13:36

ciamej

6,918
2
29
39

adhg · Answer 2 · 2014-09-28T21:25:15.143

Iterator/looping the way you presented are not thread safe in the sense that you're not iterating the most up-to-date map. But this is not your concern because you're simply removing an element from the hashmap - and that IS thread safe (due to ConcurrentHashMap). No need to put a lock or synchronized it makes no sense.

Both of your iteration are ok since your only removing it and you wrote:

The loop only needs to remove all the elements at the time myMap.keySet() is called.

Your code is just fine. You may need to check that your connection is not null before closing it (as suggested)

Safely iterate an ConcurrentHashMap that is accessed by multiple thread

2 Answers2