Java exec method, how to handle streams correctly

Question

What is the proper way to produce and consume the streams (IO) of external process from Java? As far as I know, java end input streams (process output) should be consumed in threads parallel to producing the process input due the possibly limited buffer size.

But I'm not sure if I eventually need to synchronize with those consumer threads, or is it enough just to wait for process to exit with waitFor method, to be certain that all the process output is actually consumed? I.E is it possible, even if the process exits (closes it's output stream), there is still unread data in the java end of the stream? How does the waitFor actually even know when the process is done? For the process in question, EOF (closing the java end of it's input stream) signals it to exit.

My current solution to handle the streams is following

public class Application {

    private static final StringBuffer output = new StringBuffer();
    private static final StringBuffer errOutput = new StringBuffer();
    private static final CountDownLatch latch = new CountDownLatch(2);

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

        Process exec = Runtime.getRuntime().exec("/bin/cat");

        OutputStream procIn = exec.getOutputStream();
        InputStream procOut = exec.getInputStream();
        InputStream procErrOut = exec.getErrorStream();

        new Thread(new StreamConsumer(procOut, output)).start();
        new Thread(new StreamConsumer(procErrOut, errOutput)).start();

        PrintWriter printWriter = new PrintWriter(procIn);

        printWriter.print("hello world");
        printWriter.flush();
        printWriter.close();

        int ret = exec.waitFor();
        latch.await();

        System.out.println(output.toString());
        System.out.println(errOutput.toString());
    }

    public static class StreamConsumer implements Runnable {

        private InputStream input;
        private StringBuffer output;

        public StreamConsumer(InputStream input, StringBuffer output) {
            this.input = input;
            this.output = output;
        }

        @Override
        public void run() {
            BufferedReader reader = new BufferedReader(new InputStreamReader(input));
            String line;
            try {
                while ((line = reader.readLine()) != null) {
                    output.append(line + System.lineSeparator());
                }
            } catch (IOException e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            } finally {
                try {
                    reader.close();
                } catch (IOException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                } finally {
                    latch.countDown();
                }
            }
        }

    }
}

Is it necessary to use the latch here, or does the waitFor implicate all the output is already consumed? Also, if the output doesn't end/contain new line, will the readLine miss the output, or still read all that is left? Does reading null mean process has closed it's end of the stream - is there any other scenario where null could be read?

What is the correct way to handle streams, could I do something better than in my example?

Answer 1

waitFor signals that the process ended, but you cannot be sure the threads which collect strings from its stdout and stderr finished also, so using a latch is a step in the right direction, but not an optimal one. Instead of waiting for the latch, you can wait for the threads directly:

Thread stdoutThread = new Thread(new StreamConsumer(procOut, output)).start();
Thread stderrThread = ...
...
int ret = exec.waitFor();
stdoutThread.join();
stderrThread.join();

BTW, storing lines in StringBuffers is useless work. Use ArrayList<String> instead, put lines there without any conversion, and finally retrieve them in a loop.

Answer 2

Your appapproach is right, but is't better to remove CountDownLatch and use ThreadPool, and not create new Thread directly. From ThreadPool you will get two futures, which you can wait after to completion.

But I'm not sure if I eventually need to synchronize with those consumer threads, or is it enough just to wait for process to exit with waitFor method, to be certain that all the process output is actually consumed? I.E is it possible, even if the process exits (closes it's output stream), there is still unread data in the java end of the stream?

Yes, this situation may occurs. Termination and reading IO streams is unrelated processes.