(Java, Socket) BufferedReader blocks thread, unable to .close()

Question

So I wrote a Client-Server-application for my raspberry pis - to handle multiple clients on the server I always open a new "ClientMessageListener"-thread for every client-socket.

I tried to create a destroy-chain which is called when I want the ServerSocket to shutdown. It iterates through every thread and calls the destroy-method of the ClientMessageListener which should close the connection-resources and then the socket itself.

My client-handlers look like this:

package com.imnos.piserver.server.serversocket.client;

import com.imnos.piserver.server.serversocket.ServerRequestController;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.Socket;

/**
* @author simon
*/
public class ClientMessageListener implements Runnable {

    private static final ServerRequestController SRC
            = ServerRequestController.getInstance();

    private Socket clientSocket = null;

    private PrintWriter out = null;
    private InputStreamReader inputStream = null;
    private BufferedReader in = null;

    ClientMessageListener(final Socket clientSocket) {
        this.clientSocket = clientSocket;
        this.initDataStream();
    }

    /**
     * @return
     */
    synchronized boolean destroyClientMessageListener() {

        if (this.clientSocket == null) {
            return false;
        }

        if (this.in != null) {

            try {

                this.in.close(); // Method stucks here

            } catch (IOException ignore) {
                ignore.printStackTrace();
            } finally {
                this.in = null;
            }

        }

        if (this.inputStream != null) {

            try {

                this.inputStream.close();

            } catch (IOException ignore) {
                ignore.printStackTrace();
            } finally {
                this.inputStream = null;
            }

        }

        if (this.out != null) {

            this.out.close();
            this.out = null;

        }

        return true;
    }

    /**
     * @return
     */
    private synchronized boolean initDataStream() {

        if (this.clientSocket == null) {
            return false;
        }

        if (this.clientSocket.isClosed()) {
            return false;
        }

        try {

            this.out = new PrintWriter(
                    this.clientSocket.getOutputStream(), true);

            this.inputStream = new InputStreamReader(
                    this.clientSocket.getInputStream());

            this.in = new BufferedReader(this.inputStream);

            return true;

        } catch (IOException ex) {

            this.destroyClientMessageListener();
            ex.printStackTrace();

        }

        return false;
    }

    /**
     *
     */
    @Override
    public void run() {

        if (in != null) {

            String strInput;

            try {

                while ((strInput = this.in.readLine()) != null) {

                    final boolean success
                        = SRC.handleIncoming(
                            this.clientSocket, strInput);

                }

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

        }

    }

}

Everything works fine and I expected the this.in.readLine()-call to throw an IOException when I close the resource in the destroy()-method so the thread just ends. But instead the destroy-method blocks when calling this.in.close() and absolutely no Exception is thrown.

Even Thread.getCurrentThread.interrupt() does not work and I have no idea why. Is there a clean solution to close the resources and end the run()-method?

Hard to say. You didn't provide your real code, only "something like this", so who knows. However closing the Socket will definitely destroy your listener. — Kayaman
You have a lot of unnecessary code there. Making constant null checks, like you don't know what state your object is in. Also outer streams/readers close their inner ones, so you only need to close the outermost stream. You could replace your entire destroy() method with if(socket != null) { socket.close(); socket = null; }. — Kayaman
No, they're not relevant. You've just written a lot of nonsense safety code, because you don't understand what can go wrong. At least you're protected from conditions that can never happen, but it just results in a lot of unnecessary code. Besides, if the resources aren't initialized properly, you shouldn't even be calling destroy(). — Kayaman
You should also avoid using booleans to indicate method success. Let exceptions bubble up and handle them where the initialization is done. — Kayaman

user207421 user207421 · Accepted Answer · 2017-09-15T12:02:38

Shutdown the socket for input. That will cause readLine() to return null, which will cause the thread that is reading the socket to close it and exit.

(Java, Socket) BufferedReader blocks thread, unable to .close()

1 Answers