infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-01-28 12:09:14 +00:00
Code Issues Packages Projects Releases Wiki Activity

8291986: ProcessBuilder.redirectErrorStream(true) leaves error stream available

Browse Source 
Reviewed-by: jpai
This commit is contained in:
Roger Riggs 2026-01-15 15:54:11 +00:00
parent 78a106ffbb
commit 203eb70110
3 changed files with 230 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/java.base/unix/native/libjava/ProcessImpl_md.c
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 1995, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1995, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -729,12 +729,13 @@ Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
if ((fds[0] == -1 && pipe(in) < 0) ||
(fds[1] == -1 && pipe(out) < 0) ||
(fds[2] == -1 && pipe(err) < 0) ||
(fds[2] == -1 && !redirectErrorStream && pipe(err) < 0) || // if not redirecting create the pipe
(pipe(childenv) < 0) ||
(pipe(fail) < 0)) {
throwInternalIOException(env, errno, "Bad file descriptor", mode);
goto Catch;
}
c->fds[0] = fds[0];
c->fds[1] = fds[1];
c->fds[2] = fds[2];
@ -764,17 +765,19 @@ Java_java_lang_ProcessImpl_forkAndExec(JNIEnv *env,
assert(resultPid != 0);
if (resultPid < 0) {
char * failMessage = "unknown";
switch (c->mode) {
case MODE_VFORK:
throwInternalIOException(env, errno, "vfork failed", c->mode);
failMessage = "vfork failed";
break;
case MODE_FORK:
throwInternalIOException(env, errno, "fork failed", c->mode);
failMessage = "fork failed";
break;
case MODE_POSIX_SPAWN:
throwInternalIOException(env, errno, "posix_spawn failed", c->mode);
failMessage = "posix_spawn failed";
break;
}
throwInternalIOException(env, errno, failMessage, c->mode);
goto Catch;
}
close(fail[1]); fail[1] = -1; /* See: WhyCantJohnnyExec (childproc.c) */

263
test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2022, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -21,32 +21,42 @@
* questions.
*/
import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import org.junit.jupiter.api.condition.EnabledIf;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import static org.junit.jupiter.api.Assertions.*;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.Writer;
import java.lang.ProcessHandle;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
/*
* @test
* @bug 8289643 8291760
* @requires (os.family == "linux" & !vm.musl)
* @bug 8289643 8291760 8291986
* @requires os.family == "mac" | (os.family == "linux" & !vm.musl)
* @summary File descriptor leak detection with ProcessBuilder.startPipeline
* @run testng/othervm PipelineLeaksFD
* @run junit/othervm PipelineLeaksFD
*/
@Test
public class PipelineLeaksFD {
@DataProvider
public Object[][] builders() {
private static final String OS_NAME = System.getProperty("os.name", "Unknown");
private static final long MY_PID = ProcessHandle.current().pid();
private static final boolean LSOF_AVAILABLE = checkForLSOF();
// Test cases for pipelines with a number of pipeline sequences
public static Object[][] builders() {
return new Object[][]{
{List.of(new ProcessBuilder("cat"))},
{List.of(new ProcessBuilder("cat"),
@ -59,20 +69,43 @@ public class PipelineLeaksFD {
};
}
@Test(dataProvider = "builders")
// True if lsof is runnable and collect pipe usage.
private static boolean lsofAvailable() {
return LSOF_AVAILABLE;
}
// Check if lsof is available
private static boolean checkForLSOF() {
try {
lsofForAll();
return true;
} catch (IOException ioe) {
System.err.println("Skipping: " + ioe);
return false;
}
}
@EnabledIf("lsofAvailable")
@ParameterizedTest
@MethodSource("builders")
void checkForLeaks(List<ProcessBuilder> builders) throws IOException {
Set<PipeRecord> pipesBefore = myPipes();
List<String> lsofLines = lsofForAll();
Set<PipeRecord> pipesBefore = pipesFromLSOF(lsofLines, MY_PID);
if (pipesBefore.size() < 3) {
System.out.println(pipesBefore);
Assert.fail("There should be at least 3 pipes before, (0, 1, 2)");
// Dump all lsof output to aid debugging
System.out.println("Output from lsof");
lsofLines.forEach(System.err::println);
System.err.println(pipesBefore);
fail("There should be at least 3 pipes before, (0, 1, 2)");
}
printPipes(pipesBefore, "Before start");
List<Process> processes = ProcessBuilder.startPipeline(builders);
// Write something through the pipeline
final String text = "xyz";
try (Writer out = processes.get(0).outputWriter()) {
try (Writer out = processes.getFirst().outputWriter()) {
out.write(text);
}
@ -84,16 +117,17 @@ public class PipelineLeaksFD {
try (BufferedReader inputStream = p.inputReader();
BufferedReader errorStream = p.errorReader()) {
String outActual = inputStream.readLine();
Assert.assertEquals(outActual, expectedOut, "stdout, process[ " + i + "]: " + p);
assertEquals(expectedOut, outActual, "stdout, process[ " + i + "]: " + p);
String errActual = errorStream.readLine();
Assert.assertEquals(errActual, expectedErr, "stderr, process[ " + i + "]: " + p);
assertEquals(expectedErr, errActual, "stderr, process[ " + i + "]: " + p);
}
}
processes.forEach(p -> waitForQuiet(p));
processes.forEach(PipelineLeaksFD::waitForQuiet);
Set<PipeRecord> pipesAfter = myPipes();
lsofLines = lsofForAll();
Set<PipeRecord> pipesAfter = pipesFromLSOF(lsofLines, MY_PID);
if (!pipesBefore.equals(pipesAfter)) {
Set<PipeRecord> missing = new HashSet<>(pipesBefore);
missing.removeAll(pipesAfter);
@ -101,46 +135,191 @@ public class PipelineLeaksFD {
Set<PipeRecord> extra = new HashSet<>(pipesAfter);
extra.removeAll(pipesBefore);
printPipes(extra, "Extra pipes in pipesAfter");
Assert.fail("More or fewer pipes than expected");
// Dump all lsof output to aid debugging
System.out.println("\nOutput from lsof");
lsofLines.forEach(System.err::println);
fail("More or fewer pipes than expected");
}
}
// Test redirectErrorStream, both true and false
public static Object[][] redirectCases() {
return new Object[][] {
{true},
{false},
};
}
// Test redirectErrorStream (true/false) has the right number of pipes in use
@EnabledIf("lsofAvailable")
@ParameterizedTest()
@MethodSource("redirectCases")
void checkRedirectErrorStream(boolean redirectError) throws IOException {
try (Process p = new ProcessBuilder("cat")
.redirectErrorStream(redirectError)
.start()) {
System.err.printf("Parent PID; %d, Child Pid: %d\n", MY_PID, p.pid());
List<String> lsofLines = lsofForAll();
final Set<PipeRecord> pipes = pipesFromLSOF(lsofLines, p.pid());
printPipes(pipes, "Parent and waiting child pipes");
int uniquePipes = redirectError ? 8 : 9;
if (uniquePipes != pipes.size()) {
// Dump all lsof output to aid debugging
System.out.println("\nOutput from lsof");
lsofLines.forEach(System.err::println);
}
assertEquals(uniquePipes, pipes.size(),
"wrong number of pipes for redirect: " + redirectError);
String expectedTypeName = redirectError
? "java.lang.ProcessBuilder$NullInputStream"
: "java.lang.ProcessImpl$ProcessPipeInputStream";
assertEquals(expectedTypeName, p.getErrorStream().getClass().getName(),
"errorStream type is incorrect");
} catch (IOException ioe) {
fail("Process start", ioe);
}
}
static void printPipes(Set<PipeRecord> pipes, String label) {
System.out.printf("%s: [%d]%n", label, pipes.size());
pipes.forEach(r -> System.out.printf("%-20s: %s%n", r.fd(), r.link()));
System.err.printf("%s: [%d]%n", label, pipes.size());
pipes.forEach(System.err::println);
}
static void waitForQuiet(Process p) {
try {
int st = p.waitFor();
if (st != 0) {
System.out.println("non-zero exit status: " + p);
System.err.println("non-zero exit status: " + p);
}
} catch (InterruptedException ie) {
}
}
/**
* Collect a Set of pairs of /proc fd paths and the symbol links that are pipes.
* Collect a Set of file descriptors and identifying information.
* To identify the pipes in use the `lsof` command is invoked and output scrapped for
* fd's, pids, unique identities of the pipes (to match with parent).
* @return A set of PipeRecords, possibly empty
*/
static Set<PipeRecord> myPipes() {
Path path = Path.of("/proc/" + ProcessHandle.current().pid() + "/fd");
Set<PipeRecord> pipes = new HashSet<>();
File[] files = path.toFile().listFiles(f -> Files.isSymbolicLink(f.toPath()));
if (files != null) {
for (File file : files) {
try {
Path link = Files.readSymbolicLink(file.toPath());
if (link.toString().startsWith("pipe:")) {
pipes.add(new PipeRecord(file.toPath(), link));
}
} catch (IOException ioe) {
}
}
}
return pipes;
private static Set<PipeRecord> pipesForPid(long pid) throws IOException {
var lines = lsofForAll();
return pipesFromLSOF(lines, pid);
}
record PipeRecord(Path fd, Path link) { };
/**
* Extract the PipeRecords from the `lsof` output for this process and a designated child process.
*
* @param lines lines of lsof output
* @param pid pid of child process of interest
* @return a Set of PipeRecords for parent and child
*/
private static LinkedHashSet<PipeRecord> pipesFromLSOF(List<String> lines, long pid) {
return lines.stream()
.map(PipelineLeaksFD::pipeFromLSOF)
.filter(pr -> pr != null &&
(pr.pid() == pid || pr.pid() == MY_PID))
.collect(Collectors.toCollection(LinkedHashSet::new));
}
/**
* Collect the output of `lsof` for all files.
* Files are used for `lsof` input and output to avoid creating pipes.
* @return a List of lines output from `lsof`.
*/
private static List<String> lsofForAll() throws IOException {
Path tmpDir = Path.of(".");
String tmpPrefix = "lsof-";
Path lsofEmptyInput = Files.createTempFile(tmpDir, tmpPrefix, ".empty");
Path lsofOutput = Files.createTempFile(tmpDir, tmpPrefix, ".tmp");
try (Process p = new ProcessBuilder("lsof")
.redirectOutput(lsofOutput.toFile())
.redirectInput(lsofEmptyInput.toFile()) // empty input
.redirectError(ProcessBuilder.Redirect.DISCARD) // ignored output
.start()) {
int status = p.waitFor();
assertEquals(0, status, "Process 'lsof' failed");
return Files.readAllLines(lsofOutput);
} catch (InterruptedException ie) {
throw new IOException("Waiting for lsof exit interrupted", ie);
}
}
// Return pipe records by parsing the appropriate platform specific `lsof` output.
static PipeRecord pipeFromLSOF(String s) {
return switch (OS_NAME) {
case "Linux" -> pipeFromLinuxLSOF(s);
case "Mac OS X" -> pipeFromMacLSOF(s);
default -> throw new RuntimeException("lsof not supported on platform: " + OS_NAME);
};
}
// Return Pipe from lsof output put, or null (on Mac OS X)
// lsof 55221 rriggs 0 PIPE 0xc76402237956a5cb 16384 ->0xfcb0c07ae447908c
// lsof 55221 rriggs 1 PIPE 0xb486e02f86da463e 16384 ->0xf94eacc85896b4e6
static PipeRecord pipeFromMacLSOF(String s) {
String[] fields = s.split("\\s+");
if ("PIPE".equals(fields[4])) {
final int pid = Integer.parseInt(fields[1]);
final String myKey = (fields.length > 5) ? fields[5] : "";
final String otherKey = (fields.length > 7) ? fields[7].substring(2) : "";
return PipeRecord.lookup(Integer.parseInt(fields[3]), myKey, otherKey, pid);
}
return null;
}
// Return Pipe from lsof output put, or null (on Linux)
// java 7612 rriggs 14w FIFO 0,12 0t0 117662267 pipe
// java 7612 rriggs 15r FIFO 0,12 0t0 117662268 pipe
static PipeRecord pipeFromLinuxLSOF(String s) {
String[] fields = s.split("\\s+");
if ("FIFO".equals(fields[4])) {
final int pid = Integer.parseInt(fields[1]);
final String key = (fields.length > 7) ? fields[7] : "";
final int fdNum = Integer.parseInt(fields[3].substring(0, fields[3].length() - 1));
return PipeRecord.lookup(fdNum, key, null, pid);
}
return null;
}
// Identify a pipe by pid, fd, and a key (unique across processes)
// Mac OS X has separate keys for read and write sides, both are matched to the same "name"
record PipeRecord(long pid, int fd, KeyedString myKey) {
static PipeRecord lookup(int fd, String myKey, String otherKey, int pid) {
return new PipeRecord(pid, fd, KeyedString.getKey(myKey, otherKey));
}
}
// A unique name for a string with a count of uses
// Used to associate pipe between parent and child.
static class KeyedString {
private static final HashMap<String, KeyedString> map = new HashMap<>();
private static int nextInt = 1;
private final String key;
private final String name;
private int count;
KeyedString(String key, String name) {
this.key = key;
this.name = name;
this.count = 0;
}
KeyedString(String s) {
String k = "p" + nextInt++;
this(s, k);
}
static KeyedString getKey(String key, String otherKey) {
var k = map.computeIfAbsent(key, KeyedString::new);
k.count++;
if (otherKey != null) {
map.putIfAbsent(otherKey, k);
}
return k;
}
public String toString() {
return name + "(" + count + ")";
}
}
}

1
test/jdk/java/lang/ProcessBuilder/TEST.properties Normal file
View File

@ -0,0 +1 @@
maxOutputSize=6000000