java.util.ConcurrentModificationException using an Iterator

Question

I have looked around and cannot find anything that matches or that fixes my problem. I am trying to make a very basic server system right now that will go onto something more advanced. For some reason if more than one client tries to join it will crash with this error message.

Exception in thread "messageRecieveThread" java.util.ConcurrentModificationException
at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:819)
at java.util.ArrayList$Itr.next(ArrayList.java:791)
at zx.server.threads.connections.Message.run(Message.java:23)
at java.lang.Thread.run(Thread.java:724)

Here is my message class so far:

package zx.server.threads.connections;

import java.io.IOException;
import java.net.Socket;
import java.util.Iterator;

import zx.server.communication.handle.Processing;
import zx.server.storage.severSide.Store;

public class Message implements Runnable {
private Thread thread;

public Message() {
    thread = new Thread(this);
    thread.setName("messageRecieveThread");
    thread.start();
}

@Override
public void run() {
    while (true) {
        for (Iterator<Socket> it = Store.getAll().iterator(); it.hasNext();) {
            Socket s = it.next();
            try {
                if (s.getInputStream().available() != 0) {
                    byte[] buffer = new byte[s.getInputStream().available()];
                    s.getInputStream().read(buffer);
                    new Processing(s, buffer);
                }
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}

}

The line that is supposedly causing the error is this one:

Socket s = it.next();

Any help would be immensely appreciated.
Thanks,
~Ryan

`I have looked around and cannot find anything that matches or that fixes my problem.` Look towards the right end of your monitor, in the `Related` section. — Sotirios Delimanolis, Feb 28 '14 at 20:06
See the [javadoc](http://docs.oracle.com/javase/7/docs/api/java/util/ArrayList.html). It explains (near the top of the page) when this exception would be raised. — ajb, Feb 28 '14 at 20:07
This means that the `Collection` returned by `Store.getAll()` is modified by another thread -- but this is not shown in the code you have provided so far — fge, Feb 28 '14 at 20:15
You could synchronize the arraylist to avoid the arraylist to begin edited from other threads.. (Anyway ConcurrentModificationException is throw when the arraylist is edit outside the iterator (from another thread) while you are still iterate it.) — Marco Acierno, Feb 28 '14 at 20:16
The code you've posted looks ok in isolation, the problem is more than likely something _else_ modifying the collection you get from `Store.getAll()` while you're still iterating over it. — Ian Roberts, Feb 28 '14 at 20:17
@MarcoAcierno This only works if you do that around every block of code using the the ArrayList. — David Ehrmann, Feb 28 '14 at 20:17
Everyone's right about the concurrent modification, but you're also holding that iterator for a really long time if you have IO going on on each iteration. That's probably the only reason you noticed the bug, and a huge reason to not just synchronize the access. — David Ehrmann, Feb 28 '14 at 20:19
@DavidEhrmann Then he could use `Collections.syncronizedList`.. But i never used it.. i don't know how it works and if it's OK here. So i said what could help him in this specific code section. — Marco Acierno, Feb 28 '14 at 20:19
@MarcoAcierno that will not work; it will return a reference to a synchronized `List`, yes, but only for that thread; it will never be visible to other threads. We really need more code here — fge, Feb 28 '14 at 20:22
@MarcoAcierno ConcurrentLinkedQueue is probably a better choice, here, but I'd need to see more code. — David Ehrmann, Feb 28 '14 at 20:22
@fge Here are some links to the other classes: [Processing](http://hastebin.com/kibohuvoko.avrasm) [Main](http://hastebin.com/dinecedopa.avrasm) [Id](http://hastebin.com/muveyetado.avrasm) [Store](http://hastebin.com/rinawijefe.coffee) [Accept](http://hastebin.com/xupuxukowi.avrasm) [Message](http://hastebin.com/resajuvame.avrasm) — ZXSkelobrine, Feb 28 '14 at 21:41

score 2 · Accepted Answer · answered Feb 28 '14 at 20:44

Your problem isn't on the shown code. Your problem is on you class Store.

EXPLANATION: What is happening is that while you iterate over Store.getAll() sockets, another socket is being inserted or removed from the same structure retrieved by Store.getAll() (much likely subclass of java.util.Collection - e.g.: LinkedList, ArrayList,HashMap, etc..).

REASON: You cannot iterate over Collection while the same collection is being modified. See ConcurrentModificationException javadocs

SOLUTION: Anyway, to solve your problem on the snippet shown, I suggest that you clone the structure returned by Store.getAll(). But be aware that this structure will be obsolete on its content, because there are sockets being inserted or removed in your Store class.

TIP: (this won't solve your problem but will help you to program fashionably) To iterate over elements from some structure use this in Java:

for(Socket s : Store.getAll()) {
    try {
        if (s.getInputStream().available() != 0) {
            byte[] buffer = new byte[s.getInputStream().available()];
            s.getInputStream().read(buffer);
            new Processing(s, buffer);
        }
    } catch (IOException e) {
        e.printStackTrace();
    }
}

java.util.ConcurrentModificationException using an Iterator

1 Answers1