2

I have a thread handling a socket connection:

BufferedReader socketInput = new BufferedReader(new InputStreamReader(mySocket.getInputStream()));
while (true)
{
    String line = socketInput.readLine();
    // do stuff
}

As I've read in a few answers on this site, the recommended solution is to use a flag which one thread sets and my (socket handling) thread checks and terminates itself when that flag changes state. Something like:

while (!done)
{
    String line = socketInput.readLine();
    // do stuff
}

But this can get stuck when readLine() is still waiting for input. I guess I could set a timeout:

mySocket.setSoTimeout(100);
while (!done)
{
    String line = socketInput.readLine();
    // do stuff
}

Which would probably work but I would still get a 100 ms delay before my thread "realizes" the flag's state changed.

Is there a way for the thread to "realize" right away that it should end? If not, is my solution (with timeout and flag done) correct?

Edit: I've clarified that the socketInput is of type BufferedReader (alternatively I'm considering Scanner).

NPS
  • 6,003
  • 11
  • 53
  • 90
  • See also http://stackoverflow.com/questions/12315149/interrupt-stop-thread-with-socket-i-o-blocking-operation – Matthieu Dec 31 '16 at 13:40
  • I've edited my answer to try to show that async I/O with channels are not such a big deal. I hope you'll find it interesting, and happy New Year! :) – Matthieu Jan 01 '17 at 12:07

4 Answers4

3

The most common way to handle this is to close the socket from the other Thread. This will lead the reading side to unblock and exit with the (expected) error that the socket was closed. Depending on the socket API that you have available it might also be possible to shutdown only the reading side. From a short look at the JDK shutdownInput() might work.

If you however want to continue to read from the socket later on these obvisouly won't work. Your solution should work there, but is obvisouly worse for performance and reactivity since you basically poll the socket all 100ms.

Matthias247
  • 9,836
  • 1
  • 20
  • 29
  • Of course, I can try to adjust 100 ms to something shorter. But is there a better way to do it? Assuming I want to keep using the socket later on. And also assuming I can't close the connection from the other side (because it's out of my control or for whatever other reason). – NPS Dec 31 '16 at 13:32
  • If you go lower than 100ms you will get even more unnecessary interruptions for the thread (and you will get an exception thrown every time), so you trade of fast stopping versus overhead. Normally there is no use case for stopping to read and resuming to read later on, so close is sufficient. If someone wants to stop reading temporarily he would not further call read[line] on the socket until he is interested in data again. Do you have any special scenario in mind? – Matthias247 Dec 31 '16 at 13:56
  • I had in mind a case where one thread both reads from and writes to a socket. Normally, it waits on `read` for some data but when it gets data to write to the socket (from outside) it should stop listening, write the data to the socket and then continue waiting on `read`. – NPS Dec 31 '16 at 14:10
  • 1
    If you want to do that in a reasonable way you are in async/NIO land. But I won't recommend that unless you need the scalability. For normal applications a common pattern is to have either a dedicated writer thread that cares about writing data the socket (write requests can be transferred through queues to this thread). Or to write directly from the outside (application) to the socket, which means you would need some synchronization around writes. – Matthias247 Dec 31 '16 at 14:16
  • I'd actually tried that approach (dedicated reader and writer threads) once upon a time. But I'd though that was an overkill (assuming I didn't want to wait 100 ms). Guess not. – NPS Dec 31 '16 at 14:22
  • 1
    @NPS unless you have a massive amount of data to send, writes can safely be performed in your "from outside" thread. Remember to tune the Socket [send buffer size](http://docs.oracle.com/javase/7/docs/api/java/net/Socket.html#setSendBufferSize(int)) to an appropriate value. Otherwise, async NIO is the clean way to go (just look at it as an I/O signaling mechanism, you can still use regular `java.io` stream :)) – Matthieu Dec 31 '16 at 14:25
  • @Matthias247 Is closing the thread from the other side considered a "clean" way to end the connection (and, as a result, unblock the reading thread)? Or should the other side instead send some kind of "finish" message so that this side can read it and close the socket on its own? – NPS Dec 31 '16 at 14:55
  • 1
    That depends on your protocol. For most things it's sufficient when one side simply closes the connection. Some advanced things will want to have some kind of closing handshake, e.g. in order to ensure that all sent data was also received and processed. However you always have to handle the fact that the remote could close the connection at arbitrary points of time (or the network fails). And even if you receive an end of the stream from the other side you have to close your local socket (otherwise the socket handle is leaked). – Matthias247 Dec 31 '16 at 15:21
2

The solution is correct, it will exit when done is set to true. And yes, the readLine will always wait for 100ms, if you don't want to wait you may interrupt the thread by calling thread.interrupt() it but it's not very clean way.

Graham
  • 7,431
  • 18
  • 59
  • 84
Pascal Heraud
  • 365
  • 2
  • 8
  • I'm learning socket and multithreaded programming so the clean solution (and efficient one) is precisely what I'm aiming for. So what you're saying is that ending the thread right away is impossible with `java.io` and I have to use `java.nio` instead? And the link you provided: `sel.select(100)` - doesn't this do the same as my code? Wait 100 ms before doing anything? – NPS Dec 31 '16 at 12:46
  • Javadoc https://docs.oracle.com/javase/7/docs/api/java/nio/channels/Selector.html#select(long) states that the wait is done using Object.wait(long) so I think it could be possible to notify the Thread using selector.notify(). Here is the documentation of wait/notify : http://stackoverflow.com/documentation/java/145/object-class-methods-and-constructor/619/wait-and-notify-methods#t=201612311301369525337 – Pascal Heraud Dec 31 '16 at 13:01
  • 1) But who would notify the thread? And when? I think it's impossible. 2) Now I just started wondering - why is `Thread.interrupt()` "not very clean way"? Is it deprecated? Is it discouraged? Why? – NPS Dec 31 '16 at 13:16
  • @NPS interrupting a thread blocked in a socket I/O [closes the socket](http://docs.oracle.com/javase/6/docs/api/java/nio/channels/InterruptibleChannel.html). – Matthieu Dec 31 '16 at 13:39
  • @Matthieu Why link to `java.nio`? Is the same true for `java.io`? – NPS Dec 31 '16 at 13:45
  • @NPS that's if you want to go the NIO way. For regular I/O, `read()` would [just return -1](http://stackoverflow.com/questions/6165919/java-interrupt-thread-when-reading-socket#comment60981444_6165940). – Matthieu Dec 31 '16 at 13:50
  • @Matthieu I've just wrote a simple program using `myThread.interrupt()`. The `Thread.interrupted()` returned `true` but the `read()` operation returned value `!= -1`. Also, I repeat my question - are there any downsides to using `interrupt()` with `java.io`? – NPS Dec 31 '16 at 14:01
  • @NPS if you wrote a test program to assess the behaviour with `interrupt()` and found out there are no problem then you just showed there are no downside in that case :) Did you try with `readLine()` as well (does it return `null` or throw an `InterruptedException`)? – Matthieu Dec 31 '16 at 14:11
  • I said "Not a clean way" because I never saw that in any documentation, I dont know if it's "state of the art". It's better to wait for the end of the timeout I think. If 100 ms is too long, reduce the timeout. As said @Matthieu, it seems to be documented in java.nio (and maybe notify() works). – Pascal Heraud Dec 31 '16 at 14:12
  • @PascalHeraud use [`Selector.wakeup()`](http://docs.oracle.com/javase/7/docs/api/java/nio/channels/Selector.html#wakeup()) instead. – Matthieu Dec 31 '16 at 14:21
2
  1. Create a Selector
  2. Configure your socket.getChannel() to non-blocking and register it to the Selector with SelectionKey.OP_READ
  3. Call your Selector select() method that will return when there are some data to read so you can call readLine() (i.e. select() returns > 0)

Whenever you want to end your socket processing, set your done flag and call your Selector wakeup() method. That will make the select() return immediately (potentially 0, or 1 if there was activity). You can then check your done flag and end your thread gracefully.

Here is a quick implementation. Notice I pass the BufferedReader as an argument as if you're opening it in the thread you should also close it there, which would close the socket too, so it has to be done outside. There are two methods to signal the thread to gracefully stop processing input and one to send data:

public class SocketHandler extends Thread {

    private Socket sok;
    private BufferedReader socketInput;

    private Selector sel;
    private SocketChannel chan;
    private boolean done;

    public SocketHandler(Socket sok, BufferedReader socketInput) throws IOException {
        this.sok = sok;
        chan = sok.getChannel();
        chan.configureBlocking(false);
        sel = Selector.open();
        chan.register(sel, SelectionKey.OP_READ);
        this.socketInput = socketInput;
        done = false;
    }

    @Override
    public void run() {
        while (!done) {
            try {
                if (sel.select() == 0)
                    continue;
            } catch (IOException e) {
                e.printStackTrace();
            }

            // Only one channel is registered on only one operation so we know exactly what happened.
            sel.selectedKeys().clear();
            doRead();
            // Otherwise: loop through sel.selectedKeys(), check for readability and clear the set
        }
        try {
            sel.close();
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    private void doRead() {
        try {
            String line = socketInput.readLine();
            // TODO: process 'line'
        } catch (IOException e) {
            e.printStackTrace();
        }
    }

    public void signalStop() {
        done = true;
        if (sel != null)
            sel.wakeup(); // Get out of sel.select()
    }

    public void doWrite(byte[] buffer) throws IOException { // Or "String message"
        sok.getOutputStream().write(buffer); // Or anything else
    }

}
Graham
  • 7,431
  • 18
  • 59
  • 84
Matthieu
  • 2,736
  • 4
  • 57
  • 87
0

The best way to know when finish a socket connection is to try to read something. If read method return -1 you can end threadling socket connection

byte[] data = new byte[2048];
while (!done) {
    int count = input.read(data);
    if (count <= 0) {
        if (count < 0)
            done = true;
        continue;
    }
    String request = new String(data, 0, count);
    //do stuff
}

We try to read something in input if count == -1, the socket client is disconnected now we can end the loop, by changing the value of done.

thepaulo
  • 370
  • 4
  • 10
  • I'm not sure I follow - does your solution require the other side of the connection to end in order for my thread to end as well? What if I want to end the connection from my side? – NPS Dec 31 '16 at 12:36