Chat: Sending message to all clients on a server

Question

I have a server-client application set. (HOMEWORK)

So far I have figured out how to have multiple clients connect to the server and have the server aggregate the messages the clients send, as well as having the server send the client's message back to the client and displaying it in the chat pane.

My issue is trying to send a message to multiple clients. I am only allowed to use ServerSocket and Socket libraries.

Say I have 2 clients connected to the server. One client sends a message, the message is displayed in the client's chat. The second client sends a message, the first client does NOT receive it, and the second client's chat window displays the first client's message.

Essentially the server is sending the most recent message that the respective client has not displayed in the chatbox, and I have no idea why or how.

Code for server-to-client communication :

Class CommunicationThread extends Thread {

  //Vector containing all client sockets currently connected
  //Held offsite, here for clarity
  public Vector<Socket> socketVector;

  public CommunicationThread (Socket clientSoc, Server ec3, Vector<Socket>socketVectorg)
  {
    //add new socket to vector, start thread
    clientSocket = clientSoc;
    socketVectorg.add(clientSocket);
    this.socketVector = socketVectorg;
    gui = ec3;
  }

  public void run()
  {
    System.out.println ("New Communication Thread Started");

    try {
        //Client's chat box (output)
        PrintWriter out = new PrintWriter(clientSocket.getOutputStream(),
                true);

        //Input line from client
        BufferedReader in = new BufferedReader(
                new InputStreamReader(clientSocket.getInputStream()));

        String inputLine;

        while ((inputLine = in.readLine()) != null) {
            System.out.println("Server: " + inputLine);
            gui.history.insert(inputLine + "\n", 0);

            //*************HERE IS MY ISSUE*******************
            for(Socket s : socketVector){
                out = new PrintWriter(s.getOutputStream(),
                        true);
                out.println(inputLine);
            }

            if (inputLine.equals("Bye."))
                break;

            if (inputLine.equals("End Server."))
                gui.serverContinue = false;

        }

        out.close();
        in.close();
        clientSocket.close();
    } 
    catch (IOException e) 
    {
     System.err.println("Problem with Communication Server");
     //System.exit(1); 
    } 
  }
}

I know I am overwriting "out" but I don't think that is my issue so it is in my code while I am testing.

My issue is marked in the code above. The vector accurately stores the socket ID, and since I am creating a new PrinterWriter based on the vector, I would only assume that it would get the output field of the respective client, but it does not.

My intuition is that it is a problem with threading or closing the output, but I honestly don't know.

Any recommendations are appreciated.

Oh, lord, please don't call start() on a Thread in the constructor. The object isn't fully constructed yet. Call start() after the constructor returns; use a static method (a factory method) if you need to enforce this behavior. — markspace
Well, part of the problem may be that you create a new PrintWriter for each line of input. That seems "off". Also, I see GUI code in this class. Normally GUI runs on the client, not the server. So I just want to confirm that your code above is the server code. Correct? — markspace
Yes, this is the server code. Every time the send button is pressed by the client, it seems that the while ((inputLine = in.readLine()) != null) encapsulated code is executed. The PrintWriter is for each line of OUTput, so it takes the message (inputLine) from some client and outputs it to the client's chat pane. The socket info of the client is stored in the vector, so "out" is supposed to be the output stream of each client in the vector — mrybak3
@markspace you're right, but next week, someone wil post 'You must never use 2-stage construction - it's a code smell and anti-pattern!' — Martin James

Hovercraft Full Of Eels Hovercraft Full Of Eels · Accepted Answer · 2016-04-03T22:42:03

Your problem it seems to me is that you want to do the input and output work on the client socket all in the same place, and there's no need for that. The client socket's output stream can be written to in the GUI thread, and all in one place. You can keep a collection of the output streams if need be and when you want to reply to all, iterate through the collection (probably a HashMap<String, OutpuStream> where the String is some client identifier) and send the messages.

Chat: Sending message to all clients on a server

1 Answers