Concurrency issue between waiting for a process and reading the stream?

Question

I use a ProcessBuilder to run processes. I handle Input/Output streams by submitting the corresponding runnables handling them in a thread pool (Executors.newCachedThreadPool()).
I get the result but every now and then I don't get anything.
For instance if I do: cmd \C dir to the process builder I get the results of dir back but sometimes I don't get anything (despite that the result seems to comeback from the runnable that handles the process.getInputStream).
How can I debug this? It shows up intermitently. With the same code I did not have any problem when I did new Thread(runnable).start(). It started to happen after I switched to a thread pool.

Update:
I think I found something:
I do the following in the Runnable:

 try {  
    while ( (line = br.readLine()) != null) {  
            pw.println(line);  
                sb.append(line);   
    }  
    System.out.println("Finished reading "+sb.length());  
} catch (IOException e) {             
    e.printStackTrace();  
}  
finally{  
   pw.flush();     
  try{
    isr.close();  
  }catch(Exception e){}  
}

In the cases that does not work it prints Finished reading 521. But I try to get the result via the pw and not sb.
pw is PrintWriter pw = PrintWriter(outputStream);` which I pass in the runnable

Update 2:
It seems that:status = process.waitFor(); returns earlier before the runnable that handled the inputstream finishes. How can this happen?
I read in the javadoc:
the calling thread will be blocked until the subprocess exits. So does that mean that I could return before consuming the I/O streams?

Update 3:
Seems to be the same issue here in Ruby
I.e. there is some race condition between the process ending and consuming the output

Cram

If a process calls another process the first process might return prematurely - normal behaviour. – Miscall

This not about premature returning.It is about returning before the output streams has been consumed – Cram

Answer 1

Yes. stdio between processes is buffered (usually 4KB buffers). Process A write into the buffer and exist. Process B has two threads; one waits for the end of A and the other reads the output from A. There is no way to be sure which thread executes first.

So it's possible (even likely when there is a lot of output) that process.waitFor(); returns before all buffered output has been read.

Note that flushing doesn't help here because it just makes sure that A has written everything. There is no way to "force" B to read the data in a similar way.

Therefore, you should remember the exit status and consider the process as "fully terminated" only when you read EOF from the input stream.

EDIT One solution would be to move the waitFor() into the stream gobbler and convert the gobbler into a Callable which you could then submit to the executor and use the Future API (example) to get the result(s).

Supermarket

So how should I modify my code to handle that?I just do waitFor now and the code in my stream reader is posted – Cram

Call waitFor and then join() on the thread which contains the stream handler. Or have the stream handler call waitFor() outside of the while() loop to have both in the same place. – Supermarket

1)Stream handler is submitted to an executor.How can I join?2)How can the stream handler call waitFor?The result is supposed to be passed to the thread waiting for the process to finish – Cram

You will have to use a different strategy. For example, give the stream handler a handle to an instance which can process the result instead of waiting. Or push the results into a queue and have a thread wait for work in this queue. – Supermarket

What about converting Runnable to Callable and do get for each stream gobler? – Cram