Multithreaded Client/Server application java issue

Question

This is my first post, so correct me if I am doing something wrong.

I am trying to make a multithreaded client/server application. I have no problem making a basic 1 to 1 client/server application without threading, but I want every client to talk individually to the server and each client only to recieve the messages that belongs to the particular client in the console. This is where I can't make it work.

I have looked at some tutorials online and I tried to debug in Eclipse without any luck. What I want the application to do:

If I run the server and 2 clients I want to be able to write a message on a client and make the server return the message and the message number without skipping a number.

Client 1 :

Message no: 0. You said: Hello!

Message no: 1. You said: Whats up!

Client 2 :

Message no: 0. You said: Hey Server!

Message no: 1. You said: What are you doing now?

It works with the first Client that I run and perfectly increment the messagenumber, but when I create the second one it freezes, so I guess my problem is either with the socket declaration or with the way I am threading. (Maybe a socket.close() is missing somewhere?)

I know there is some other flaws with the application, but keep in mind that it is just a school assignment for working with threads.

Thanks in advance if someone is willing to help. :)

Server Code:

public class Server extends Thread 
{   
  DataInputStream in;
  DataOutputStream out;
  ServerSocket listener;

public Server() throws IOException
{      
    try 
    {
        while (true) 
        {
            listener = new ServerSocket(9090);
            Socket socket = listener.accept(); 
            System.out.println("Test");
            ThreadServer ts = new ThreadServer(socket);
            Thread t1 = new Thread(ts);
            t1.start();   
        }
    }
    finally 
    {
        listener.close();
    }   
}

public static void main(String[] args) throws IOException 
{
    new Server();
}
}

Client code:

public class Client extends JFrame 
 {      
   DataOutputStream dos;
   BufferedReader input;
   String answer = null;
   String textString;
   Socket s = new Socket("localhost", 9090);

public Client() throws IOException
{
    JButton btn = new JButton("run");
    final JTextField txf = new JTextField(10);
    btn.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent arg0) {             
            try 
            {   
                dos = new DataOutputStream(s.getOutputStream());
                textString = txf.getText();
                dos.writeUTF(textString);
                dos.flush();        
                input = new BufferedReader(new InputStreamReader(s.getInputStream()));
                answer = input.readLine();                      
            } 
            catch (Exception e1) 
            {
                e1.printStackTrace();
            }           
                System.out.println(answer); 
            }       
    });
    setLayout(new GridLayout(2,2));
    add(btn);
    add(txf);
    setVisible(true);
    pack();
    setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

}

public static void main(String[] args) throws IOException 
{
    new Client();
}
}

ThreadServer code:

public class ThreadServer implements Runnable
{
  DataInputStream in;
  DataOutputStream out;

public ThreadServer (Socket socket) throws IOException
{       
    int i = 0;  

    try 
    {
        while (true) 
        {            
            in = new DataInputStream(socket.getInputStream());
            out = new DataOutputStream(socket.getOutputStream());

            try 
            {
                String message = in.readUTF();          
                PrintWriter out = new PrintWriter(socket.getOutputStream(), true);
                out.println("Message no: " + i + ". You said: " + message);                  
                out.println("Message no: " + i);
            }
            catch(Exception e) 
            {
                e.printStackTrace();
            }
            i++;
        }
    }
    finally 
    {
        socket.close();
    }
}

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

}
  }

Answer 1

Your ThreadServer is looping infinitely in the constructor, instead of the run method. This is very bad. The run method is where you should put that code.

You should also follow the advice in the comment and move the socket creation.

-edit-

same is true for your ThreadServer, don't do the socket init in the loop. just the reading.

You might also discover later that you need an extra thread for the client. As it is now it seems each client can only receive messages when the action performed button is clicked, and that is probably not what you want. This will expose some synchronization problems that you will have to deal with, but most you gui library might actually help you there. It was a long time since I dealt with swing.

-edit again- In addition, read up on exception handling. You should decide where you want to catch the IO exceptions, rather than randomly nesting try/catch and declaring methods as throwing them.

Answer 2

Let me try to solve it.

• Create only one ServerSocket. Move below line from while loop. Put it before while loop.

  listener = new ServerSocket(9090);
  

• Move the code of ThreadServer's constructor to run() method because it implements Runnable but doesn't behave like a Runnable class.

  public ThreadServer(Socket socket) throws IOException {
      this.socket=socket;
  }
  
  @Override
  public void run() {
     ...
     //code from constructor
  }
  

• In Server class you are writing two lines to Client but client is reading just one. Remove extra line as shown below

  out.println("Message no: " + i + ". You said: " + message);
  //out.println("Message no: " + i);
  

---

One last suggestion:

Always check for inputs before calling readUTF() as shown below:

if (in.available() > 0) {
    try {
        String message = in.readUTF();
        PrintWriter out = new PrintWriter(socket.getOutputStream(), true);
        out.println("Message no: " + i + ". You said: " + message);
        //out.println("Message no: " + i);
    } catch (Exception e) {
        e.printStackTrace();
    }
    i++;
}