Java Multiple threads using a client socket

Question

I'm creating a server based chat program that has multiple worker threads each handling a client on a socket. It has a single server socket that passes off the client to the worker thread.

    public static void main(String[] args) {
    ExecutorService executor = Executors.newFixedThreadPool(5);
    ServerSocket serverSocket = null;
    try {
        serverSocket = new ServerSocket(4001);
        System.out.println("Listening server");
        while (true) {
            Socket socket = serverSocket.accept();
            System.out.println("Connected");
            Random rand= new Random();
            int port=4001+rand.nextInt(5);
            Worker worker = new Worker(port);
            executor.execute(worker);
            System.out.println("Thread started");
            new PrintWriter(socket.getOutputStream(), true).println(port);
            socket.close();
            System.out.println("Closed");
            //  break;
        }
    } catch (IOException e) {
        e.printStackTrace();
    }

}

public class Worker implements Runnable {

private int port;
public Worker(int i) {
    port=i;
}
@Override
public void run() {
    worker();
}
private static Socket socket;
private static PrintWriter out;
private static BufferedReader in;

private void worker() {
    try {
        ServerSocket serverSocket = new ServerSocket(port);
        socket = serverSocket.accept();
        out = new PrintWriter(socket.getOutputStream(), true);
        in = new BufferedReader(new InputStreamReader(socket.getInputStream()));
        String line;
        while ((line = in.readLine()) != null) {
            System.out.println("Received: " + line);
            switch (line) {
                case ("Button Press"):
                    System.out.println("Handled button");
                    out.println("Button acknowledged");
                    break;
                case ("Give me some data"):
                    System.out.println("Handled data");
                    out.println("Have some data");
                    break;
            }
        }
    } catch (IOException e) {
        e.printStackTrace();
    } finally {
        try {
            out.close();
            in.close();
            socket.close();
        } catch (IOException e) {
            e.printStackTrace();
        }

    }
}

This works fine, however the issue is when I have an automated request by the client to check for messages and the user provides some input at the same type. This causes conflict as the actual methods take a couple of seconds to run and if more input is received then the request won't be handled because its in the middle of the method. For example:

private static BufferedReader in;
private static PrintWriter out;
public static void main(String[] args) {
    Main main=new Main();
    main.createWindow();
    try {
        Socket init = new Socket(InetAddress.getLocalHost(), 4001);
        int port
             =Integer.parseInt
                (new BufferedReader
                    (new InputStreamReader(init.getInputStream())).readLine());
        init.close();
        Socket socket=new Socket(InetAddress.getLocalHost(), port);
        out = new PrintWriter(socket.getOutputStream(), true);
        in = new BufferedReader(new InputStreamReader(socket.getInputStream()));


    while(true){
        try {
            Thread.sleep(5000);
            out.println("Give me some data");
            if(in.readLine().equals("Have some data")){
                System.out.println("Data recieved");
            }else{
                System.out.println("Data not recieved");
            }
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

    }
    } catch (IOException e) {
        e.printStackTrace();
    }
}


private void createWindow(){
    JFrame frame =new JFrame("This is a button");
    Container pane=getContentPane();
    JButton button=new JButton("This is a button");
    button.addActionListener(this);
    pane.add(button);
    frame.setTitle("Messaging");
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    frame.setContentPane(pane);
    frame.setVisible(true);
    frame.setSize(400, 350);
}

@Override
public void actionPerformed(ActionEvent e) {
    System.out.println("Button press");
    try {
        out.println("Button Press");
        if(in.readLine().equals("Button acknowledged")){
            System.out.println("Button complete");
        }else{
            System.out.println("Button fail");
        }
    } catch (IOException e1) {
        e1.printStackTrace();
    }
}

If the button is pressed whilst the server is fetching data it conflicts and the wrong response is sent. So how would I deal with this? If I create a separate socket to just handle automated checks that's double the amount of threads the server has to cope with. Is there a better solution? And please can you try to explain as this is a new area for me.

Answer 1

Your problem is that you have two threads interacting with the socket, your main thread and the Swing event handling thread. This means that the two threads can queue two different things, and the responses may be picked up by the wrong thread. The simplest way to get where you want to be is to put all the socket interaction in one thread.

There are two ways to do this. One would be for the main thread to queue the periodic automated checks to the Swing event handling thread. However, that's a bit complicated and also possibly buggy as the actual Swing threading model is different from what's documented.

The other way to do it would be for the Swing event handling thread to queue button presses to the main thread. In this case, you would queue the button press to the main thread using proper synchronization. The main loop of the main thread would check for button presses and send them through the socket and wait for the proper response.