Reputation: 5552
I have a server (thread) that receives data and has an array of clients (Sockets). Whenever a client connects a new thread is created and a new client is added to the clients array.
Here is the clients:
List<Socket> clients = Collections.synchronizedList(new ArrayList<Socket>());
So then the server send the incoming data to the clients as such:
//for each message:
for(Socket client : clients){ //throws java.util.ConcurrentModificationException
new PrintWriter(client.getOutputStream(), true).println(line);
}
Now there are two issues here:
First of all I occasionally get a java.util.ConcurrentModificationException when thread 2 adds a client.
Also I am creating a new PrintWriter
every time I am sending a message. The other option will be to store a PrintWriter
for each client but it is a client so it doesn't make sense to store a PrintWriter
.
Is this a good approach ?
Upvotes: 0
Views: 46
Reputation: 472
You are trying to access to a object (client.getOutputStream()
) while iterating over the list thus the java.util.ConcurrentModificationException
. Change your List<>
for a ConcurrentLinkedQueue
. Also for PrintWriter
here you just wrap an existing stream.
Upvotes: 0
Reputation: 18276
That's correct if all clients are waiting a println with a getString() on their side, else you may want to change to the UDP protocol.
Creating a new PrintWriter everyloop is not necessary, since the connection will be kept open so just one (for each client) is best, also in the client side you will have one only Reader and that one will wait for messages trough its get methods.
For the concurrent problem you must create a copy of the current list:
for (Socket client : new ArrayList<>(clients)) {
new PrintWriter(client.getOutputStream(), true).println(line);
}
Upvotes: 1