On Fri, 8 Jul 2022 16:24:16 GMT, Roger Riggs <rri...@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.
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Cleanup of PipelineLeaksFD test improving error messages and source cleanup.
test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java line 130:
> 128: })
> 129: .filter(p1 -> p1.link().toString().startsWith("pipe:"))
> 130: .collect(Collectors.toSet());
Is this intentionally leaking the returned `DirectoryStream` from `Files.walk`
to trigger the previous failure mode or should this be auto-closed (i.e.
https://errorprone.info/bugpattern/StreamResourceLeak )?
Suggestion:
try (Stream<Path> s = Files.walk(path)) {
return s.filter(Files::isSymbolicLink)
.map(p -> {
try {
return new PipeRecord(p, Files.readSymbolicLink(p));
} catch (IOException ioe) {
}
return new PipeRecord(p, null);
})
.filter(p1 -> p1.link().toString().startsWith("pipe:"))
.collect(Collectors.toSet());
}
-------------
PR: https://git.openjdk.org/jdk/pull/9414
