RFR [8024521] (process) Async close issues with Process InputStream

[Paul Sandoz]

Hi Ivan,

Why do you require closeLock? is it not sufficient leave processExited as is and just add:

public synchronized void close() throws IOException {
    super.close();
}

Plus there was already some logic checking if close was asynchronously called (see BufferedInputStream.close):

 372                         if (buf == null) // asynchronous close()?
 373                             this.in = null;

and this would no longer be required

I am somewhat struggling to understand the test (not too familiar with this area). Requires a more expert eye in this area than I.

IIUC you are trying to induce the the output process fd of ExecLoop to "flip" to another fd opened by OpenLoop. The test concerns me since it is gonna use up loads of resources and proceed for 10 minutes until deciding beyond reasonable doubt that there is no issue.

Paul.

On Oct 11, 2013, at 9:45 PM, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:

> Hello all!
> 
> Would you please help review a fix?
> http://bugs.sun.com/view_bug.do?bug_id=8024521
> 
> As of JDK 1.7, ProcessPipeInputStream tries to drain the InputStream when the process exits.
> However, it is racing with application code that could be closing the inputstream at the same time.
> This can lead to processExited() operating on the wrong underlying file.
> 
> The test included into the webrev demonstrates exactly that: Due to the race, the draining thread can suddenly start to read a huge file which will cause OutOfMemoryException.
> 
> Here's the webrev with a simple fix, which synchronizes close() and processExited():
> http://cr.openjdk.java.net/~igerasim/8024521/0/webrev/
> 
> It's important to note that this simple fix isn't complete.
> It only protects from the race when the file descriptor is closed by a call to Process#getInputStream().close().
> The file descriptor can still be asynchronously closed in some other way, for example by calling to the native close() function.
> 
> Sincerely yours,
> Ivan Gerasimov
>