RFR [15] 8236825: Reading output from ... using ProcessBuilder/Process might hang

[Paul Sandoz]

My sense is it is fixing a marginal case for the less dominant platform where this is less likely arise, whereas for the dominant platform there is still an issue.

Waiting for a non-blocking native read is a reasonable approach (IIUC that is required for the desired proper fix).  It would be useful to assess any time-frame of such support to plan ahead?

—

ProcessImpl
—

 665         void processExited() {
 666             synchronized (closeLock) {
 667                 try {
 668                     InputStream in = this.in;
 669                     // this stream is closed if and only if: in == null
 670                     if (in != null) {
 671                         boolean noCompete = false;
 672                         try {
 673                             // Briefly, wait for competing read to complete
 674                             noCompete = readLock.tryAcquire(500L, TimeUnit.MILLISECONDS);
 675                             if (noCompete) {
 676                                 // no competing read, buffer any pending input
 677                                 this.in = drainInputStream(in);
 678                             }
 679                         } catch (InterruptedException ie) {
 680                             // Ignore interrupt and release and close always
 681                         } finally {
 682                             readAborted = !noCompete;
 683                             in.close();     // close the original underlying input stream
 684                             if (noCompete)
 685                                 readLock.release();
 686                         }
 687                     }
 688                 } catch (IOException ignored) {}
 689             }
 690         }


Do you need to move the code at lines 684-685 to occur before line 683? since in.close() could throw.

Paul.


> On Jan 21, 2020, at 12:36 PM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
> 
> Please review a potential way to address two issues of java.lang.Process deadlocks during process exit. [1] [2]
> (Linxu OS process expertise appreciated).
> 
> The deadlock is between some thread reading process output from a process that has exited
> and the processExited thread that is attempting to buffer any remaining output so
> the file descriptor for the pipe can be closed.  The methods involved are synchronized
> on a ProcessPipeInputStream instance.
> 
> The hard case arises infrequently since the pipe streams are closed by the OS
> normally (or within a few seconds) and the readXXX completes.
> However, the case causing trouble is when the subprocess has spawned
> another process and both processes are using the same file descriptor/stream for output.
> Though the process that exits doesn't have the fd open anymore the extra subprocess does.
> And if that subprocess does not exit, then the read and deadlock does not get resolved.
> 
> The approach proposed is to use a semaphore to guard the readXXX and
> providing some non-blocking logic in processExited to forcibly close
> the pipe if it detects that there is a conflicting read in progress.
> (And remove the synchronized on processExited).
> 
> This solution works ok on MacOSX, where one of the issues occurred frequently.
> Closing the pipe unblocks the reading thread.
> 
> On Linux, it appears that the blocking read (in native code) does not unblock
> unless a signal occurs; so the solution does not fix the problem adqurated/completely.
> 
> Having a non-blocking native read would be the next step of complexity.
> The problem has been around for a while so it may be an option to wait
> for additional work on non-blocking pipe reads, the direction that Loom is moving.
> 
> Suggestions welcome,
> 
> Thanks, Roger
> 
> Webrev: http://cr.openjdk.java.net/~rriggs/webrev-hdiutil-8236825/
> 
> Issues:
> [1] https://bugs.openjdk.java.net/browse/JDK-8236825
> [2] https://bugs.openjdk.java.net/browse/JDK-8169565