Simple Chat Server / Client - Server does not return messages

Question

I have gone through a lot of threads and blogs/examples outside SO but I just can't get my code to work. The code is very similar to what others have posted but this thing just won't work .. Here is a brief outline of the code.

Server Class implements Runnable

Creates new ServerSocket(PORT, Numberof Users)

Uses serverSocket.Accept() to get the socket and stores it in ArrayList. 

Sends a welcome message to socket  The code uses print writer on OUTPUTSTREAM on Socket. 

(this part actually works and I get the message back)

Client Class socket = new Socket(localhost, PORT);

In a separate thread start reader which reads from socket.inputstream and puts it on chat window. 

In a separate thread write to socket.output stream.

This is mostly same as server side code

So when I test the code. The server starts. The client opens up swing window. It connects to server. It gets the server welcome message back. But when I write something it does not return back the text that I just sent. it just stays at readLine and never come back. I had a similar issue in echoserver code which I fixed (thanks to SO !) using threads. Now both reader and writer are in separate thread, so I am not sure whats wrong.

Thanks in advance! Apologies if you think its a repeated thread but I just can't fix my code on my own !

Here is the full code:

Server Class:

import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.ServerSocket;
import java.net.Socket;
import java.util.ArrayList;
import java.util.Date;
import java.util.Iterator;

public class ChatServer implements Runnable {

    private static int port; 
    private static String ip_add;
    private Socket socket;
    private ArrayList<Socket> clients;
    private ServerSocket ss;

    public ChatServer(String ip_add_in, int port_in) {
        // TODO Auto-generated constructor stub
        port = port_in;
        ip_add = ip_add_in;

        clients = new ArrayList<Socket>();
    }



    @Override
    public void run() {
        // TODO Auto-generated method stub

        System.out.println("ChatServer Start!!" + new Date() );

        try {
            ss = new ServerSocket(port, 10);

            while(true){
                socket = ss.accept();
                System.out.println("ChatServer Accepts!!" + new Date() );
                clients.add(socket);
                sendWelcomeMessage();       
            }


        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }


    }

    public void sendWelcomeMessage() {
        try {
            PrintWriter writer = new PrintWriter(socket.getOutputStream());
            writer.println("Welcome To Chat Server");
            writer.flush();
            writer.close();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

    public class transmitMessages implements Runnable {

        @Override
        public void run() {
            // TODO Auto-generated method stub

            while(true){
                if (ss.isBound()) {
                    for (Socket sock : clients) {


                    }
                }


            }

        }

    }

}

Client Class:

import java.awt.*; 
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.UnknownHostException;
import java.util.*;

import javax.swing.*;


public class Chat1 {

    JFrame frame;
    JPanel msg_pan, chat_pan;
    JButton send;
    JTextArea type_area, chat_area;

    private static String user;
    private SendMsg send_action = new SendMsg();
    private Socket writerSocket; 
    //  private Socket readerSocket;
    static String ip_address;
    static int port;
    private PrintWriter writer;
    private ReaderThread readerRunner;
    private SenderThread senderRunner;
    Thread senderThread;


    Thread  readerThread;

    public Chat1(String name){
        user = name;
    }


    public void create_window() {

        frame = new JFrame(user);

        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        chat_pan = new JPanel();

        //      Adding Main Chat Area 
        chat_area = new JTextArea(30,50);
        chat_area.setEditable(false);
        chat_area.setLineWrap(true);


        JScrollPane chat_scroll = new JScrollPane(chat_area);
        chat_scroll.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        chat_scroll.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_NEVER);
        chat_pan.add(chat_scroll);



        msg_pan = new JPanel();


        send = new JButton("Send");
        send.addActionListener(send_action);
        type_area = new JTextArea(5,50);
        type_area.setLineWrap(true);

        JScrollPane type_scroll = new JScrollPane(type_area);
        type_scroll.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_ALWAYS);
        msg_pan.add(type_scroll);
        msg_pan.add(send);
        msg_pan.setLayout(new BoxLayout(msg_pan, BoxLayout.X_AXIS));


        frame.getContentPane().add(BorderLayout.SOUTH, msg_pan);
        frame.getContentPane().add(BorderLayout.CENTER, chat_pan);
        frame.setSize(600, 600);
        frame.setVisible(true);
        frame.pack();


    }


    public static void main(String ip_add_in, int port_in) {
        // TODO Auto-generated method stub
        ip_address = ip_add_in;
        port = port_in;
        Chat1 user1 = new Chat1("User1");

        user1.startchat();

    }

    class SendMsg implements ActionListener{

        @Override
        public void actionPerformed(ActionEvent e) {
            // TODO Auto-generated method stub
            String msg = type_area.getText();

            if (msg != null) {

                senderRunner = new SenderThread(msg);
                senderThread = new Thread(senderRunner);
                senderThread.start();
            }
            type_area.setText(null);


        }


    }

    public void setup_connection() {




    }

    public void startchat(){

        connect_socket();

        create_window();

        System.out.println("Window Done");

        readerRunner = new ReaderThread();
        readerThread = new Thread(readerRunner);
        readerThread.start();

        System.out.println("Thread Started");


    }

    public void connect_socket(){

        try {
            System.out.println("Start Chat" + new Date());
            writerSocket = new Socket(ip_address,port);

        } catch (UnknownHostException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } catch (IOException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }


    }

    public class ReaderThread implements Runnable {

        InputStreamReader streamReader; 
        BufferedReader reader;
        String msg;

        public void run(){

            try {

                System.out.println("Entered ReaderThread Run");
                streamReader = new InputStreamReader(writerSocket.getInputStream());
                reader = new BufferedReader(streamReader);

                while (true) {
                    msg = reader.readLine();
                    if (msg != null) {
                        chat_area.append(msg);  
                    }


                }
            } catch (IOException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }

            System.out.println("Leaving ReaderThread Run");

        }

    }

    public class SenderThread implements Runnable{

        String msg;
        public SenderThread(String msg_in){
            msg = msg_in;
        }

        public void run(){

            System.out.println("Entered SenderThread Run" + msg);


            try {

                if (writer == null) {
                    writer = new PrintWriter(writerSocket.getOutputStream());   
                }

                System.out.println("Writer has Error-->" + writer.checkError());

                writer.println(msg);
                writer.flush();
                writer.close();

            } catch (UnknownHostException e) {
                // TODO Auto-generated catch block
                System.out.println("Sender Thread Run Exception 1");
                e.printStackTrace();
            } catch (IOException e) {
                // TODO Auto-generated catch block
                System.out.println("Sender Thread Run Exception 2");
                e.printStackTrace();
            }

            System.out.println("Leaving SenderThread Run");

        }
    }

}

Tester Class:

import java.util.*;

public class ChatTester {

    public static void main(String[] args) {
        // TODO Auto-generated method stub
        ChatServer serverRunner = new ChatServer("127.0.0.1", 5000);

        Thread serverThread = new Thread(serverRunner);

        serverThread.start();

        System.out.println("Server Started" + new Date());

        try {
            Thread.sleep(2000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }



        Thread clientThread = new Thread() {
            public void run() {
                Chat1.main("127.0.0.1", 5000);

            }
        };


        clientThread.start();
        System.out.println("Client Started" + new Date());
    }

}

Answer 1

You are closing the writer after every send, which also closes the underlying socket output stream. You can see this in your test output:

Writer has Error-->false (before first send)
Writer has Error-->false (before second send, output is already closed!)
Writer has Error-->true (before third send, second send failed because the stream is closed)

The server on the other hand does not listen for any incoming messages. It only sends the welcome message and then closes the output stream!

The server needs to keeps reading the input streams of the client sockets. When it receives a message, it should write it to the output streams of the clients. Do not close the streams as long as you are communication with the clients.

Below is a simple example of how to receive messages and send a reply.

---

First remove writer.close() in both SenderThread and sendWelcomeMessage().

Start a new handler thread for each connection (note: starting a new thread for each connection might not be the best practice for "real" applications).

while(true){
    socket = ss.accept();
    System.out.println("ChatServer Accepts!!" + new Date() );
    clients.add(socket);
    sendWelcomeMessage();
    (new Thread(new ClientHandler(socket))).start();
}

In the thread, listen for incoming messages and send them back.

public class ClientHandler implements Runnable {

    private Socket socket;
    private BufferedReader reader;
    private BufferedWriter writer;

    public ClientHandler(Socket socket)
    {
        this.socket = socket;
    }

    @Override
    public void run() {
        try
        {
            reader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
            writer = new BufferedWriter(new OutputStreamWriter(socket.getOutputStream()));

            String message = reader.readLine();
            while (message != null)
            {
                writer.write(message + System.lineSeparator());
                writer.flush();
                message = reader.readLine();
            }
        }
        catch (IOException e)
        {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

Note that the ClientHandler thread only echoes the received message back to the sender. In a real chat, these messages would be sent to all connected clients, for example in your transmitMessages class, that does not do anything at the moment.