End Thread that is stuck in a InputStream.read() loop

Question

I start a cmd application that outputs to System.out through this SyncPipe Runnable:

public class SyncPipe implements Runnable {

    private final InputStream is;
    private final OutputStream os;

    public SyncPipe(InputStream is, OutputStream os) {
        this.is = is;
        this.os = os;
    }

    public void run() {
        try {
            final byte[] buffer = new byte[1024];
            for ( int length = 0; ( length = is.read(buffer) ) != -1; )
                os.write(buffer, 0, length);
            System.out.print("stopped");
        } catch ( Exception ex ) { 
            ex.printStackTrace(); 
        }
    }

}

I start RunIt with cmd = "C:/bin/read.exe -f D:/test.jpg"

private class RunIt implements Runnable {

    public int p;
    public String cmd;

    public RunIt (int p, String cmd) {
        this.p = p;
        this.cmd = cmd;
    }

    public void run() {
        ProcessBuilder pb = new ProcessBuilder("cmd");
        try {
            process = pb.start();
            (new Thread(new SyncPipe(process.getErrorStream(), System.err))).start();
            (new Thread(new SyncPipe(process.getInputStream(), System.out))).start();
            OutputStream out = process.getOutputStream();
            out.write((cmd + "\r\n").getBytes());
            out.flush();
            out.close();

            try {
                process.waitFor();
            } catch ( InterruptedException e ) {
                e.printStackTrace();
            }

            println("Stopped using %d.", p);
        } catch ( IOException ex ) {
            ex.printStackTrace();
        }
    }

}

My question now: How can I make (new Thread(new SyncPipe(process.getErrorStream(), System.err))) die? Giving SyncPipe a boolean variable stop, setting it true during runtime, and checking for it via for ( int length = 0; ( length = is.read(buffer) ) != -1 && !stop; ) did not do the trick.

Thanks a lot in advance.

I ended up doing the work-around that @Gray suggested. It works now:

public void run() {
    try {
        final byte[] buffer = new byte[1024];
        do
            if ( is.available() > 0 ) {
                int length = is.read(buffer);
                if ( length != -1 )
                    os.write(buffer, 0, length);
                else
                    stop = true;
            }
        while ( !stop );
    } catch ( Exception ex ) { 
        ex.printStackTrace(); 
    }
}

score 1 · Answer 1 · edited May 23 '17 at 12:25

1

My question now: How can I make (new Thread(new SyncPipe(process.getErrorStream(), System.err))) die?

You are going to have to close the input stream out from under it I believe. I suspect that it is blocked in the read and no setting of a stop variable (even if properly volatile) will get the read thread to unblock.

You are going to need to do something like:

 InputStream is = process.getInputStream();
 InputStream es = process.getErrorStream();
 ...
 is.close();
 es.close();

Code would look approximately like the following. I'm not sure if your waitFor() call is returning or not.

 InputStream is = process.getInputStream();
 InputStream es = process.getErrorStream();
 (new Thread(new SyncPipe(es, System.err))).start();
 (new Thread(new SyncPipe(is, System.out))).start();
 try {
     OutputStream out = process.getOutputStream();
     out.write((cmd + "\r\n").getBytes());
     out.flush();
     out.close();
     try {
         process.waitFor();
     } catch ( InterruptedException e ) {
         e.printStackTrace();
     }
 } finally {
     is.close();
     es.close();
 }

Another answer might be to use the available() method on InputStream so you can loop and check your stop flag. See this answer: https://stackoverflow.com/a/1089079/179850

edited May 23 '17 at 12:25

Community

1
1

answered Oct 09 '13 at 19:15

Gray

115,027
24
293
354

I tried that already and was hoping for an Exception that I could handle, but nothing happened. – dotwin Oct 09 '13 at 19:25
Huh. That works for me. Is your process stopping or are you trying to stop the `SyncPipe` from another thread? – Gray Oct 09 '13 at 19:30
If you are trying to wait for the process, you might have to do something like `synchronized (process) { process.wait(1000); }` and then kill the streams @dotwin. – Gray Oct 09 '13 at 19:31
I suspect that you are hanging in `process.waitFor()`, right? If you use the `process.wait(...)` hack, it may work. – Gray Oct 09 '13 at 19:35
Yes, looks like it. The change with the finally's doesn't help. – dotwin Oct 09 '13 at 19:36
how exactly would i implement the `synchronized (process) { process.wait(1000); }`? – dotwin Oct 09 '13 at 19:37
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/38912/discussion-between-gray-and-dotwin) – Gray Oct 09 '13 at 19:37
So EJP learns from our work and he gets the checkmark by stating the [now] obvious? Huh. @dotwin – Gray Oct 09 '13 at 22:44
I don't know how to make this fair ... I can already see the comment a la "but EJP had the correct answer, he should be given the check ..." I would love to give both of you a check. In any case: A BIG THANK YOU TO YOU!!! – dotwin Oct 09 '13 at 22:51
Can I transfer reputation score to you directly? @Gray – dotwin Oct 09 '13 at 22:54
No worries dude. @dotwin – Gray Oct 09 '13 at 22:55

score 1 · Answer 2 · answered Oct 09 '13 at 19:15

1

InputStream#read() states

This method blocks until input data is available, the end of the stream is detected, or an exception is thrown.

So when you go into your for loop

for ( int length = 0; ( length = is.read(buffer) ) != -1; )
    os.write(buffer, 0, length);

it won't be able to exit until the end of the stream is reached, ie. the process stops or you close the streams yourself.

If the whole point of the SyncPipe is to pass the content to your standard output/error streams, why would you want to stop the Thread running it?

answered Oct 09 '13 at 19:15

Sotirios Delimanolis

274,122
60
696
724

I need the entire thing to shut down at some point. The `C:/bin/read.exe` does not close nicely (I always close it with ctrl+c when executing it in the command line). Closing down `process` with `.destroy()` does not do the job. I would like everything to close when I need it to. `out.write(((char)3 + "\r\n").getBytes());` does also not help. I suspect the SyncPipe Threads keep everything alive. – dotwin Oct 09 '13 at 19:23
@dotwin You will need to close the streams yourself, calling `close()`. – Sotirios Delimanolis Oct 09 '13 at 19:24
Yeah, printing a control-c won't work. The shell/OS takes that keystroke and sends a signal to the process -- it's not the process that handles it @dotwin. – Gray Oct 09 '13 at 19:35

user207421 · Accepted Answer · 2013-10-09T21:53:16.423

1

The thread will read EOS and exit when the underlying process exits. You don't have to do anything special about it yourself.

EDIT It seems to me from reading your comments to other answers that your real problem is ending the process. These threads will come unstuck as soon as that happens. You're attacking the wrong end of the problem.

edited Oct 09 '13 at 21:53

answered Oct 09 '13 at 21:18

user207421

305,947
44
307
483

You are correct but he still needs to close the streams -- at least the background threads did not terminate on Unix without a specific close in my test program. – Gray Oct 09 '13 at 22:38
Both of you are correct ... I killed the underlying process via `Runtime.getRuntime().exec("taskkill /f /pid " + pid);` and my old implementation closes nicely as well. But I definitely need to close the streams. In any case: Thanks a lot. – dotwin Oct 09 '13 at 22:44
@Gray Of course he needs to close the streams, when he reads EOS on them. I haven't said otherwise. – user207421 Oct 09 '13 at 22:47
You said "you don't have to do anything special". If he uses process.destroy() to kill the process (under Unix that is), the threads did not seem to see EOS. Closing the streams was the only way to get the threads to exit for me. Sounds like the windows `taskkill` is doing something different. – Gray Oct 09 '13 at 22:51
@Gray I have production code that does this. I don't have to do anything special, and the OP has marked the answer as correct. I can't explain your observations. Maybe you should be asking your own question. – user207421 Oct 10 '13 at 00:43

End Thread that is stuck in a InputStream.read() loop

3 Answers3