CODEWITHSUNDEEP
javamultithreading
Kevin
Kevin

Reputation: 6302

Safely iterate an ConcurrentHashMap that is accessed by multiple thread

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.

Upvotes: 2

Views: 1967

Answers (2)

ciamej
ciamej

Reputation: 7068

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();
}

Upvotes: 1

adhg
adhg

Reputation: 10903

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)

Upvotes: 0

Related Questions