Search code examples
javachatconcurrentmodification

Getting a ConcurrentModificationException chat program


I'm getting a ConcurrentModificationException when I call the send() method. I'm only accessing the data, there's no modification, so I'm not sure what the issue is. Any suggestions?

public class chatServer{

public static LinkedList<serverListener> threadList = new LinkedList<serverListener>();
public static ListIterator<serverListener> li = threadList.listIterator();

public static void main (String [] args)throws IOException{
    ServerSocket incoming = new ServerSocket(58667); //create new ServerSocket
    Socket servSock;
    BufferedReader buffRead;
    while(true){
        servSock = incoming.accept(); //accept incoming connections from clients
        serverListener c1 = new serverListener(servSock); //create a new chat listener object, passes in servSock socket
        c1.start();
        threadList.addLast(c1);
    }//while(running)
    //incoming.close();
}//main

public static synchronized void send(String m) throws IOException{
    while(li.hasNext()){
        li.next().getOS().writeBytes(m);
    }//while
}//send

public static synchronized void removeClient(DataOutputStream d){
    while(li.hasNext()){
        if (li.next().getOS()==d){
            li.remove();
        }//if
    }//while
}//removeClient
}//chatServer

public class serverListener extends Thread{
private Socket sock; //socket used to communicate (set from passed in value)
private static String incoming; //string to store incoming message
private BufferedReader dataIn; //BufferedReader to read input message
private static DataOutputStream dataOut; //data stream to send message

//chatListener constructor, accepts socket as parameter 
public serverListener(Socket s) throws IOException{
    sock = s; //sets sock to parameter value s
    dataIn = new BufferedReader(new InputStreamReader(sock.getInputStream())); //creates input stream reader from socket sock
    dataOut = new DataOutputStream(sock.getOutputStream()); //creates a new output stream from sock
}//listen constructor

//server listener thread, receives messages from connected client, runs until EXITEXIT is received
public void run(){
    do{
        try{
            setMessage(dataIn.readLine());} //read incoming message, store value in incoming string
        catch(IOException e){ //catches error where a message is unable to be read
        }

        if(getMessage().equals("EXITEXIT")){ //if incoming message is EXITEXIT, set running to false, will not print value
            chatServer.removeClient(dataOut);
            try {
                chatServer.send("Client has left");
            } catch (IOException e) {
            }
        } else
            try {
                chatServer.send(getMessage());
            } catch (IOException e) {
            } //send message to synchronized print method
    }while(true); //loop until isRunning is false
}//run


public static DataOutputStream getOS(){
    return dataOut;
}

private static void setMessage(String m){
    incoming = m;
}
private static String getMessage(){
    return incoming;
}
}//public chatListener

Everything works great until the client sends a message and it calls the send method.

UPDATE I ended up using a for loop:

public static synchronized void send(String m, DataOutputStream d) throws Exception{
    for (int i=0; i<threadList.size(); i++){
        try{
            if(threadList.get(i).getOS()!=d)
                threadList.get(i).getOS().writeBytes(m+'\n');
        }catch(Exception e){}
    }//for
}//send

Solution

  • In short, it's because you modified a list without using its iterator.

    Here, you create a list and get its iterator:

    public static LinkedList<serverListener> threadList = new LinkedList<serverListener>();
    public static ListIterator<serverListener> li = threadList.listIterator();
    

    And here, you structurally modify the list without using its iterator:

    while(true){
        servSock = incoming.accept(); //accept incoming connections from clients
        serverListener c1 = new serverListener(servSock); //create a new chat listener object, passes in servSock socket
        c1.start();
        threadList.addLast(c1);
    }//while(running)
    

    So as a result, the next time you call li.next(), you'll get a ConcurrentModificationException

    From the LinkedList javadoc (emphasis mine):

    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.