HashMap iteration/removal getting java.util.ConcurrentModificationException

Question

i have a HashMap of ClientSocket and Client Object.

i'm iterating throw them using for loop, but sometimes new lines are added to the hashmap and then i get the java.util.ConcurrentModificationException error. i accutally understand why it happens, but i don't understand how to solve it. i've tried to make a new copy of my HashMap before the iteration starts but yet - i still got the error.

my code:

private volatile HashMap<ClientSocket, Client> clientsMap = new HashMap<ClientSocket, Client>();
private volatile HashMap<ClientSocket, Client> iteratorClientsMap = new HashMap<ClientSocket, Client>();
private volatile ClientsMapIterator iterator;

iterator = new ClientsMapIterator(clientsMap);
iteratorClientsMap = iterator.getItreator();

for (Map.Entry<ClientSocket, Client> entry : iteratorClientsMap.entrySet()) {                                                                   
    ClientSocket key = entry.getKey();
    //Client value = entry.getValue();              
    long diff = currentTime - key.getLastOnline();
    boolean isAvailable = false;

    try {
        isAvailable = (key.getSocket().getInputStream().available() > 0);
    } catch (IOException e) {
        e.printStackTrace();
    }               

    if ( diff > keepAlive)              
        removeClientSocket(key);
}

public synchronized void addClientSocket(ClientSocket clientSocket) {
    clientsMap.put(clientSocket, null);                 
}

addClientSocket is the function that because of it i'm getting the error.

Show the code of removeClientSocket

assylias
– assylias

2012-05-13 13:17:19 +00:00
Commented May 13, 2012 at 13:17 — assylias
– assylias, Commented May 13, 2012 at 13:17

Peter Lawrey · Accepted Answer · 2012-05-13 13:14:13Z

2

You are modifying the collection while iterating over it. This is flagged as a concurrent modification.

The simplest solution is to use a ConcurrentHashMap which doesn't trigger a CME.

answered May 13, 2012 at 13:14

Peter Lawrey

535k83 gold badges770 silver badges1.2k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Asaf Nevo Over a year ago

when i've changed it to ConcurrentHashMap i get the java.lang.NullPointerException at java.util.concurrent.ConcurrentHashMap.put when trying to add new line to the map. why does my duplication isn't good (nor working) ? i'm copying the current hashmap and iterating on the copy instead of the origina

Peter Lawrey Over a year ago

You can't add a value value to a ConcurrentHashMap. Why are you setting it to null when you method is called addXxxx? Perhaps you can provide an actual object?

Asaf Nevo Over a year ago

i'm passing only the value as null, the key is set.. it is actually pretty important, it will make the differenece for me later if the client has a Client object (which means he had been logged in) or not...

Peter Lawrey Over a year ago

I would re think your design. I would just have one thread and one handler object per connection and you wouldn't to maintain the data structure.

Asaf Nevo · Accepted Answer · 2012-05-13 14:12:35Z

i found a solution i don't know if it is the best one:

synchronized (this) {
            iterateClientsMap = new HashMap<ClientSocket, Client>(clientsMap);  
        }           

        for (Map.Entry<ClientSocket, Client> entry : iterateClientsMap.entrySet())      
        {                                                                                                   
            ClientSocket key = entry.getKey();
            //Client value = entry.getValue();              
            long diff = currentTime - key.getLastOnline();
            boolean isAvailable = false;
            try {
                isAvailable = (key.getSocket().getInputStream().available() > 0);
            } catch (IOException e) {
                e.printStackTrace();
            }               
            if ( diff > keepAlive)              
                removeClientSocket(key);
}

i've duplicated my HashMap and iterated it on the copy, and before each iterating process, i'm blocking it to other threads (using the synchronized title) so it won't be interrupted during the copying

mprabhat · Accepted Answer · 2012-05-13 15:13:50Z

The issue seems to be propagating from removeClientSocket(key);

where it seems collection is being modified while it is being iterated too.

One way to fix this issue is Pass the iterator to this method

removeClientSocket(iterator, key);

remove the key from this iterator by calling iterator.remove() instead of removing from the collection itself while in mid of iteration.

Seemingly your issue is with multiple threads, either your synchronize access to both add and remove on same lock like:

public void removeClientSocket(iterator, key){
    synchronized(clientMap){
      //now remove
    }
}

and

public void addClientSocket(ClientSocket clientSocket) {
    synchronized(clientsMap){
        clientsMap.put(clientSocket, null);    
    }             
}

or use java.util.Concurrent package for automatic concurrency control. You can specifically use ConcurrentHashMap.

Collectives™ on Stack Overflow

HashMap iteration/removal getting java.util.ConcurrentModificationException

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related