Messages are only being sent to the client that sent them in my chat program

Question

In my chat client/server program I'm having trouble broadcasting the messages to all connected clients. The messages sent only seem to be sent back to the original server even though I'm iterating through an ArrayList. I have an ArrayList of sockets and a method that takes in the read message as a parameter and creates a PrintWriter for each socket. I think the problem has to do with my Client class (excuse me if I'm wrong) but I haven't been able to figure out what.

The Client Class:

public class H7Client extends Thread implements ActionListener{

private JTextField jtfPortName = new JTextField(20);
private JTextField jtfHostName = new JTextField(20);
private String hostName;
private int portNumber;

private JTextArea jtaChat = new JTextArea("Send a message to the client", 15,40);
private JTextArea jtaRecive = new JTextArea("WELCOME TO THE CHAT!", 15,40);
private JTextField jtfUName = new JTextField("user");



public H7Client(){
    JFrame defaultFrame = new JFrame();

    JLabel jlPortName = new JLabel("Enter The Port number");
    JLabel jlHostName = new JLabel("Enter the Host name");
    JLabel jlUName = new JLabel("Enter a username");

    JButton jbSetSocketInfo = new JButton("Confirm Port and Host Info");
    JButton jbExit = new JButton("Exit");
    JButton jbSendText = new JButton("Send");

    jbSetSocketInfo.addActionListener(this);
    jbExit.addActionListener(this);
    jbSendText.addActionListener(this);



    JPanel jpNorth = new JPanel();
    JPanel jpCenter = new JPanel();
    JPanel jpLabels = new JPanel();

    defaultFrame.add(jpNorth,BorderLayout.NORTH);
    jpNorth.add(jbSetSocketInfo,BorderLayout.EAST);
    jpNorth.add(jbSendText, BorderLayout.CENTER);
    jpNorth.add(jbExit,BorderLayout.WEST);


    defaultFrame.add(jpCenter,BorderLayout.CENTER);
    jpCenter.add(jtaChat,BorderLayout.SOUTH);
    jpCenter.add(jpLabels,BorderLayout.NORTH);

    jpLabels.setLayout(new GridLayout(2,3));
    jpLabels.add(jlHostName);
    jpLabels.add(jlPortName);
    jpLabels.add(jlUName);
    jpLabels.add(jtfHostName);
    jpLabels.add(jtfPortName);
    jpLabels.add(jtfUName);

    defaultFrame.add(jtaRecive,BorderLayout.SOUTH);

    defaultFrame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);


    defaultFrame.setLocationRelativeTo(null);
    defaultFrame.setSize(800,800);
    defaultFrame.setVisible(true);

}

public void setClientComms(String message){
    try{
        // open communications to the server
        Socket s = new Socket(hostName, portNumber);
        ClientThread ct = new ClientThread(s, message);
    }
    catch(IOException e){
        e.printStackTrace();
    }
}

class ClientThread extends Thread{

    public ClientThread(Socket sock, String msg) {

        try {

            // open input stream
            InputStream in = sock.getInputStream();
            BufferedReader br = new BufferedReader(
                    new InputStreamReader(in));

            // open output stream
            OutputStream out = sock.getOutputStream();
            PrintWriter pout = new PrintWriter(
                    new OutputStreamWriter(out));


            // write something to the server
            pout.println(msg);

            // make sure it went
            pout.flush();

            // read something back from server
            String incomingMessage = br.readLine();

            // print the something to the user
            System.out.println("Message: " + msg);
            //jtaChat.setText("");
            jtaRecive.append("\n" + msg);


            // Send the terminating string to the server
            pout.println("quit");
            pout.flush();

            // close everything
            pout.close();
            br.close();
            sock.close();
        } catch (UnknownHostException uhe) {
            System.out.println("What host you speak of?");
        } catch (IOException ioe) {
            System.out.println("Bad IO?");
            ioe.printStackTrace();
        }
    }
}

public void actionPerformed(ActionEvent event) {
    if (event.getActionCommand().equals("Exit")) {
        System.exit(0);
    } else if (event.getActionCommand().equals("Send")) {
        String chatMessage = jtaChat.getText();
        jtaChat.setText("");
        setClientComms(chatMessage);


    } else if (event.getActionCommand().equals("Confirm Port and Host Info")) {
        hostName = jtfHostName.getText();
        //NEED TO ADD IN WAY TO HANDLE IP ADDRESSES
        portNumber = Integer.parseInt(jtfPortName.getText());
    }
}


public static void main(String [] args) {

    new H7Client();


}

}

The Server Class:

public class H7Server{

public ArrayList<Socket> clients = new ArrayList<Socket>();

public static void main(String[] args){new H7Server();}
public H7Server()
{



    ServerSocket ss = null;

    try {
        System.out.println("getLocalHost: "+ InetAddress.getLocalHost() );
        System.out.println("getByName:    "+InetAddress.getByName("localhost") );

        ss = new ServerSocket(16789);
        Socket cs = null;
        while(true){        // run forever once up
            //try{
            cs = ss.accept();               // wait for connection
            clients.add(cs);
            ThreadServer ths = new ThreadServer( cs );
            ths.start();
       } // end while
    }
    catch( BindException be ) {
        System.out.println("Server already running on this computer, stopping.");
    }
    catch( IOException ioe ) {
        System.out.println("IO Error");
        ioe.printStackTrace();
    }

} // end main

class ThreadServer extends Thread {
    Socket cs;

    public ThreadServer( Socket cs ) {
        this.cs = cs;
    }

    public void run() {

           BufferedReader br;
           String clientMsg;
           try {
               br = new BufferedReader(
                       new InputStreamReader(
                               cs.getInputStream()));


               clientMsg = br.readLine();                // from client
               System.out.println("Server read: " + clientMsg);
               while(clientMsg != null) {
                   sendMessage(clientMsg);
               }

           } catch (IOException e) {
               System.out.println("Inside catch");
               e.printStackTrace();
           }


    }

    public synchronized void sendMessage(String s){
        try{
            for(Socket sock: clients) {
                PrintWriter pw = new PrintWriter(new OutputStreamWriter(sock.getOutputStream()));

                pw.println(s);
                pw.flush();
            }
        }
        catch(IOException e){
            e.printStackTrace();
        }


    }

} // end class ThreadServer

}

Answer 1

There are problems with both your client class, as you suspected, and also the server class. There are two things you need to fix.

First, you receive the message from the server but aren't doing anything with it. The client receives incomingMessage in the try block of ClientThread's constructor but is unused. A possible fix is to append it to jtaReceive and to remove the other append, but don't do this.

For real-time chatting, the client must have a minimum of two threads: one to send messages, and one to constantly check if there are any messages to receive. Perhaps, this is what you were trying to achieve with ClientThread but it unfortunately does not work. In addition to the wrong code, your use of ClientThread creates a Thread but never actually runs it. You need to have a thread that, in the run method, contains a forever while loop that constantly receives messages from the server. One implementation is this: (you can rename to anything you like)

class ReceiverThread extends Thread {

    private BufferedReader s;

    public ReceiverThread(Socket sock) throws IOException {
        s = new BufferedReader(new InputStreamReader(sock.getInputStream()));
    }

    @Override
    public void run() {
        while (true) 
            try {
                String message = s.readLine();
                // ...code to do whatever with message...
            } catch (IOException e) {
                e.printStackTrace();
            }
    }

}

Furthermore, you also need to create a socket that stays open (maybe create and start this thread with the socket when you confirm host and port info).

The problem with the server class is a problem that gets us all: the notorious ConcurrentModificationException. You can read about it here, but basically this exception is thrown when you modify (add, remove, etc.) a data structure when you aren't supposed to. The data structure in question is the ArrayList of Socket's, and the when is during the for-each loop when you are sending the message. Basically, while that loop is running, the while loop creating Socket's is adding them to the ArrayList and throwing the exception. To fix, you need to stop modifying the ArrayList when you are in the for loop, possibly by removing the while loop, holding the while loop until the for loop is finished, or even having a Queue of sockets to be added when the for loop is active, but there are many ways to do this.

I am assuming that your server will only receive one message, just to test, but if you decide that you want to be able to chat more than one message, you must use the extra thread you use for client class.

Best of luck :).

Note: I advise against use of AWT thread (the thread that calls actionPerformed) for networking stuff/anything that may take some time, or your application GUI will actually freeze.