diff --git a/src/java.base/unix/native/libjava/ProcessImpl_md.c b/src/java.base/unix/native/libjava/ProcessImpl_md.c index f7531ad5abe..12597fbb650 100644 --- a/src/java.base/unix/native/libjava/ProcessImpl_md.c +++ b/src/java.base/unix/native/libjava/ProcessImpl_md.c @@ -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) */ diff --git a/test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java b/test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java index d3c44bc9279..2b4cdfa9bdd 100644 --- a/test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java +++ b/test/jdk/java/lang/ProcessBuilder/PipelineLeaksFD.java @@ -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 builders) throws IOException { - Set pipesBefore = myPipes(); + List lsofLines = lsofForAll(); + Set 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 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 pipesAfter = myPipes(); + lsofLines = lsofForAll(); + Set pipesAfter = pipesFromLSOF(lsofLines, MY_PID); if (!pipesBefore.equals(pipesAfter)) { Set missing = new HashSet<>(pipesBefore); missing.removeAll(pipesAfter); @@ -101,46 +135,191 @@ public class PipelineLeaksFD { Set 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 lsofLines = lsofForAll(); + final Set 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 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 myPipes() { - Path path = Path.of("/proc/" + ProcessHandle.current().pid() + "/fd"); - Set 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 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 pipesFromLSOF(List 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 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 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 + ")"; + } + } } diff --git a/test/jdk/java/lang/ProcessBuilder/TEST.properties b/test/jdk/java/lang/ProcessBuilder/TEST.properties new file mode 100644 index 00000000000..c7e8a6850ca --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/TEST.properties @@ -0,0 +1 @@ +maxOutputSize=6000000