Trt Trt
Trt Trt

Reputation: 5552

Is this a correct approach for sending to multiple clients?

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

Answers (2)

Mladen Savić
Mladen Savić

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

Marcos Vasconcelos
Marcos Vasconcelos

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

Related Questions