Closing resources when thread is finished in Java

Question

I have a thread that uses network port for communication. I added cancel() method to stop the execution and close network resources (How to properly stop the Thread in Java?)

private class ServerThread extends Thread {
        int portNumber;
        String serverAddress = null;

        public ServerThread(String serverAddress, int portNumber) {
            super();
            this.serverAddress = "localhost";
            this.portNumber = portNumber;
        }

        @Override
        public void run() {
            ServerSocket listener;
            Socket socket;
            try {
                listener = new ServerSocket(this.portNumber);
                socket = listener.accept();
                BufferedReader in = new BufferedReader(new InputStreamReader(
                        socket.getInputStream()));
                PrintWriter out = new PrintWriter(socket.getOutputStream(),
                        true);

                while (!isInterrupted()) {
                    String input = in.readLine();
                    if (input != null) {
                        out.println(input);
                        System.out.println("Hi:" + input);
                    }
                } // end of while loop
                System.out.println("OUT"); <-- ???
                socket.close(); <-- ???
                listener.close(); <-- ???
            } catch (IOException e) {
                e.printStackTrace();
            } 
        }

        public void cancel() {
            System.out.println("cancel called");
            interrupt();
        }
    }

The issue is that when I execute the ServerThread, and send cancel() message to finish the execution, it seems like that the three lines of code never executed: System.out.println("OUT"); socket.close(); listener.close();.

It also seems like that I don't need to send cancel() message to finish the thread.

ServerThread s = new ServerThread(serverAddress, serverPortNumber);
s.start();
...
s.cancel(); // ???

What's the recommended way of closing resources used by threads? Don't I have to close resources when thread is not used anymore? Or everything is just automatically processed?

ADDED

It seems like that the thread is killed automatically as this code just works.

            while(true) {
                String input = in.readLine();

                if (input != null) {
                    System.out.println("Received:" + input);
                    out.println(input);
                }
            } // end of while loop
            /* System.out.println("OUT");
            socket.close();
            listener.close(); */

Where are you calling `cancel()` from? It interrupts the _current_ `Thread`. i.e. the `Thread` the calls `cancel()` not your `ServerThread`. — Boris the Spider, Oct 15 '13 at 20:44
@Sotirios Delimanolis - I'm not sure what `a clean stop` means, but I tried s.close() to have a warning that it is a deprecated method. — prosseek, Oct 15 '13 at 20:45
@Boris the Spider - It is invoked from other thread. BTW, it should have been `interrupt()`, not `Thread.currentThread().interrupt();`. I updated my post. — prosseek, Oct 15 '13 at 20:48
You should be exiting your read loop if readLine() reruns null, and closing the socket at that point. — user207421, Oct 15 '13 at 22:52

forty-two · Accepted Answer · 2013-10-15T21:00:04.387

1

Thread#interrupt() will not interrupt the blocking I/O call on the socket. Try setting a "stop" flag and closing the socket in the cancel() method instead, and deal with the exception and check the flag in the while loop.

InterruptibleChannels reacts on the interrupt call, but not "old fashioned" socket streams.

edited Oct 15 '13 at 21:00

answered Oct 15 '13 at 20:54

forty-two

12,204
2
26
36

score 1 · Answer 2 · answered Oct 15 '13 at 20:55

1

In Java 7 you can use the try (resource) {} catch idiom like this:

try (final BufferedReader reader = new BufferedReader(new InputStreamReader(socket.getInputStream()))) {
  while ((line = reader.readLine()) != null) {
    process(line);
  }
} catch (IOException e) {
  e.printStackTrace();
}

This will guarantee that the stream is closed properly once the try block is left. No matter what happens inside or how the try/catch terminates.

answered Oct 15 '13 at 20:55

TwoThe

13,879
6
30
54

I would also add that the code should be using the concurrency framework introduced over a decade ago. There is no need to extend `Thread` and handle thread lifecycle management. Let the `Executor` do that for you. – Oct 16 '13 at 00:49
Yes, absolutely. The executor can do everything a thread can do, just more convenient. – TwoThe Oct 16 '13 at 11:26

Closing resources when thread is finished in Java

ADDED

2 Answers2