0

I'm creating a multithread chat in java. When user u1 sends a message to user u2 but user u2 is not connected, user u1 sends the message to the server and user u2 will receive the message once he connects to the server. The messages who are not sent are added to an ArrayList. Once a user connects, he checks if he's the recipient of a pending message. If he is, the message is sent to him and then removed from the pending messages list. This is how I do it:

for(Iterator<String> itpendingmsgs = pendingmsgs.iterator(); itpendingmsgs.hasNext();) {
    String pendingmsg = itpendingmsgs.next();
    String dest = pendingmsg.substring(4);              
    if (protocol.author.equals(dest)) {
        sendMsg(msg);
        pendingmsgs.remove(pendingmsg);
    }
}

this is what I get:

Exception in thread "Thread-3" java.util.ConcurrentModificationException
at java.util.AbstractList$Itr.checkForComodification(Unknown Source)
at java.util.AbstractList$Itr.next(Unknown Source)
at ChatServer$ClientConnection.run(ChatServer.java:383)
at java.lang.Thread.run(Unknown Source)

How do I fix it? Is it because I'm using the iterator?

tony danza
  • 306
  • 7
  • 22

4 Answers4

3

Instead of this 

pendingmsgs.remove(pendingmsg);

use 

itpendingmsgs.remove();

Iterator of ArrayList is fail fast , so while you are iterating over the ArrayList using the Iterator if underlying ArrayList is modified by any method other than add and remove provided by Iterator itself it will throw ConcurrentModificationException and will bail out.

In your current implementation while you are looping through the list on certain condition you are also modifying the list by calling remove on the underlying ArrayList, instead call remove method of the Iterator.

From the Java Docs:

The iterators returned by this class's iterator and listIterator methods are fail-fast: if the list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

Note that the fail-fast behavior of an iterator cannot be guaranteed as it is, generally speaking, impossible to make any hard guarantees in the presence of unsynchronized concurrent modification. Fail-fast iterators throw ConcurrentModificationException on a best-effort basis. Therefore, it would be wrong to write a program that depended on this exception for its correctness: the fail-fast behavior of iterators should be used only to detect bugs.

mprabhat
  • 20,107
  • 7
  • 46
  • 63
1

You are not allowed to modify a list while iterating through it, except through the iterator instance itself. You must call itpendingmsgs.remove().

Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436
1

Instead of

pendingmsgs.remove(pendingmsg);

use

itpendingmsgs.remove();

See:

The behavior of an iterator is unspecified if the underlying collection is modified while the iteration is in progress in any way other than by calling this method.

Source: Java API

yankee
  • 38,872
  • 15
  • 103
  • 162
1

Based on the documentation ArrayList api The iterators returned by this class's iterator and listIterator methods are fail-fast: if list is structurally modified at any time after the iterator is created, in any way except through the iterator's own remove or add methods, the iterator will throw a ConcurrentModificationException.

You shouldn't remove from the collection while you are iterating it. You should use iterator's remove method instead.

for(Iterator<String> itpendingmsgs = pendingmsgs.iterator(); itpendingmsgs.hasNext();) {
String pendingmsg = itpendingmsgs.next();
String dest = pendingmsg.substring(4);              
if (protocol.author.equals(dest)) {
    sendMsg(msg);
    itpendingmsgs.remove();
}

}

HPCS
  • 1,434
  • 13
  • 22