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.gerasi...@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
>
