RFR: 8289643: File descriptor leak with ProcessBuilder.startPipeline

[Alan Bateman]

On Thu, 7 Jul 2022 19:08:35 GMT, Roger Riggs <rriggs at openjdk.org> wrote:

> The `ProcessBuilder.pipelineStart()` implementation does not close all of the file descriptors it uses to create the pipeline of processes.
> 
> The process calling `pipelineStart()` is creating the pipes between the stages.
> As each process is launched, the file descriptor is inherited by the child process and
> the child process `dup`s them to the respective stdin/stdout/stderr fd.  
> These copies of inherited file descriptors are handled correctly.
> 
> Between the launching of each Process, the file descriptor for the read-side of the pipe for the output of the previous process is kept open (in the parent process invoking `pipelineStart`).  The file descriptor is correctly passed to the child and is dup'd to the stdin of the next process.
> 
> However, the open file descriptor in the parent is not closed after it has been used as the input for the next Process. 
> The fix is to close the fd after it has been used as the input of the next process.
> 
> A new test verifies that after `pipelineStart` is complete, the same file descriptors are open for Unix Pipes as before the test.
> The test only runs on Linux using the /proc/<pid>/fd filesystem to identify open file descriptors.
> 
> The bug fix is in `ProcessBuilder.pipelineStart` and is applicable to all platforms.

src/java.base/share/classes/java/lang/ProcessBuilder.java line 1301:

> 1299:                 if (prevOutput instanceof RedirectPipeImpl redir) {
> 1300:                     // Wrap the fd so it can be closed
> 1301:                     new Process.PipeInputStream(redir.getFd()).close();

It looks unusual to need to create a new input stream to invoke close but I think this is okay here.

test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 59:

> 57:  * @requires (os.family == "linux" & !vm.musl)
> 58:  * @summary file descriptor leak with ProcessBuilder.startPipeline
> 59:  * @run testng PipelineLeaksFD

I wonder if this test should be /othervm, just in case there are pipes left over from previous tests that could interfere with the list found in /proc.

-------------

PR: https://git.openjdk.org/jdk/pull/9414