1

I want to create server which is able to be connected with multiple clients, but I always get 'Socket Closed' exception or read NULL value from input streams, previously I thought it was caused by closing socket connection in a wrong way, so I post this topic, but now seems it's not the problem.

Server Method

//here I use thread pool to solve concurrency issue
public static void main(String[] args) {

 try{
   ExecutorService service = Executors.newFixedThreadPool(4);
   ServerSocket serverSocket = new ServerSocket();

   while (true) {
      //keep listening
      Socket socket = serverSocket.accept();
      service.execute(new HandlerThread(socket));
   }
  }catch(Exception ex){
     ex.printStackTrace();
  }
}

HandlerThread Class

public class HandlerThread extends Thread{

    private Socket socket;
    private BufferedReader in;

    public HandlerThread(Socket socket) throws IOException {
        this.socket = socket;
        this.start();
    }

    public void run(){

        try {
            //Caches is a singleton class, which used to store the received data from client
            Caches caches = Caches.getInstance();

            //32 line
            in = new BufferedReader(new InputStreamReader(socket.getInputStream(), "UTF-8"));
            String line = in.readLine();

            //caches.list is an ArrayList
            synchronized (caches.globalCaches) {
                if (null != line) {
                    caches.globalCaches.add(line);
                }
            }

            System.out.println(line);

            in.close();
            socket.close();

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


}

Client Simulator

public class Main {

    public static void main(String[] args) throws IOException, InterruptedException {

        for(int i = 0 ; i < 40; i++){
            Thread.sleep(2000);
            test test1 = new test();
            test1.start();
        }
    }

    public static class test extends Thread{

        public void run(){
            try {
                Socket socket = new Socket("localhost", 5050);

                PrintStream out = new PrintStream(socket.getOutputStream());
                out.println("Hello Server!");

                out.close();
                socket.close();
            } catch (UnknownHostException e) {
                e.printStackTrace();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }
    }
}

Exception and problems

Start server before client simulator, there isn't any exception from client simulator, however within server side, not all data were received, from the console printing information, I saw 'null', 'Hello Server', 'java.net.SocketException: Socket is closed...', like below

Hello Server
null
null
Hello Server
java.net.SocketException: Socket is closed
Hello Server
    at java.net.Socket.getInputStream(Socket.java:876)
    at com.icubeidea.core.threads.HandlerThread.run(HandlerThread.java:32)
Hello Server

I researched for a quite long time, still can't find what on earth caused those exceptions and data missing issue, and also is there any potential risks which could cause the memory leaking problem after server running for a while?

5
  • Might be because of the way you call start() on your thread. Try service.execute(new HandlerThread(socket).start()); instead and remove the this.start() from the HandlerThread constructor. Commented Jul 30, 2014 at 9:14
  • "Try service.execute(new HandlerThread(socket).start());" -- Nooooooooooo!!!! That's a double mistake! Luckily it won't even compile. Commented Jul 30, 2014 at 9:20
  • @user3811473 your suspicion is correct, but the solution is wrong, as Hanno said, it won't be even compiled. Commented Jul 30, 2014 at 9:24
  • @Z.Neeson, Yeah I realised too late, usually it would work but since the thread is passed to the executor which takes a thread object then cant call start on it there :) I didn't test it which is why I didn't write it as an answer :) Commented Jul 30, 2014 at 9:37
  • @user3811473 Agreed. Without the executor it's a valid advice not to start the thread from within its constructor. Commented Jul 30, 2014 at 9:45

1 Answer 1

3

The problem seems to be in the constructor of your HandlerThread:

You're calling this.start(); which you really shouldn't. Especially when using an ExecutorService at the same time!

(In this specific case, you're creating a new thread and start() it and at the same time tell your executor to run the same code. In the end, the same code will be run twice on a single socket, which can cause all sorts of problems, and one of the threads will close the socket while the other thread is still trying to read from it. That's where the exception actually comes from.)

Solution:

Do not extend Thread, but have your HandlerThread only implement the Runnable interface instead.

Once you hand over an instance of that class to the executor it will take care of running your run() method; no need to start any thread yourself.

Sign up to request clarification or add additional context in comments.

2 Comments

Do you find any potential risks which could cause the memory leaking problem after the server running for a while?
You'll have to somehow limit the size of your caches.globalCaches. And to make sure, I'd close the socket in a finally block: try { ... } finally { socket.close(); }. This way the socket gets closed/released even when an exception occurs during processing the data. You may also want to set a timeout to keep the server thread from being stuck indefinitely in case the client does not send any (more) data.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.