From 75607e760e4b74aa1f02ba171dee7eabfca49209 Mon Sep 17 00:00:00 2001 From: Thomas Stuefe Date: Tue, 17 Jun 2025 20:23:20 +0000 Subject: [PATCH 1/4] JDK-8210549 --- make/test/JtregNativeJdk.gmk | 2 +- src/java.base/unix/native/libjava/childproc.c | 54 ++++++++------ .../ProcessBuilder/FDLeakTest/FDLeakTest.java | 72 +++++++++++++++++++ .../FDLeakTest/exeFDLeakTester.c | 56 +++++++++++++++ .../ProcessBuilder/FDLeakTest/libFDLeaker.c | 36 ++++++++++ 5 files changed, 197 insertions(+), 23 deletions(-) create mode 100644 test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java create mode 100644 test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c create mode 100644 test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c diff --git a/make/test/JtregNativeJdk.gmk b/make/test/JtregNativeJdk.gmk index 00ef4bece58..a204467a77b 100644 --- a/make/test/JtregNativeJdk.gmk +++ b/make/test/JtregNativeJdk.gmk @@ -62,7 +62,7 @@ BUILD_JDK_JTREG_LIBRARIES_JDK_LIBS_libGetXSpace := java.base:libjava ifeq ($(call isTargetOs, windows), true) BUILD_JDK_JTREG_EXCLUDE += libDirectIO.c libInheritedChannel.c \ libExplicitAttach.c libImplicitAttach.c \ - exelauncher.c \ + exelauncher.c libFDLeaker.c exeFDLeakTester.c \ libChangeSignalDisposition.c exePrintSignalDisposition.c BUILD_JDK_JTREG_EXECUTABLES_LIBS_exeNullCallerTest := $(LIBCXX) diff --git a/src/java.base/unix/native/libjava/childproc.c b/src/java.base/unix/native/libjava/childproc.c index de66fa32571..afbe1eea762 100644 --- a/src/java.base/unix/native/libjava/childproc.c +++ b/src/java.base/unix/native/libjava/childproc.c @@ -52,6 +52,21 @@ closeSafely(int fd) return (fd == -1) ? 0 : close(fd); } +int +markCloseOnExec(int fd) +{ + const int flags = fcntl(fd, F_GETFD); + if (flags < 0) { + return -1; + } + if ((flags & FD_CLOEXEC) == 0) { + if (fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0) { + return -1; + } + } + return 0; +} + static int isAsciiDigit(char c) { @@ -68,21 +83,15 @@ isAsciiDigit(char c) #endif static int -closeDescriptors(void) +markDescriptorsCloseOnExec(void) { DIR *dp; struct dirent *dirp; - int from_fd = FAIL_FILENO + 1; - - /* We're trying to close all file descriptors, but opendir() might - * itself be implemented using a file descriptor, and we certainly - * don't want to close that while it's in use. We assume that if - * opendir() is implemented using a file descriptor, then it uses - * the lowest numbered file descriptor, just like open(). So we - * close a couple explicitly. */ - - close(from_fd); /* for possible use by opendir() */ - close(from_fd + 1); /* another one for good luck */ + /* This function marks all file descriptors beyond stderr as CLOEXEC. + * That includes the file descriptor used for the fail pipe: we want that + * one to stay open up until the execve, but it should be closed with the + * execve. */ + const int fd_from = STDERR_FILENO + 1; #if defined(_AIX) /* AIX does not understand '/proc/self' - it requires the real process ID */ @@ -91,18 +100,22 @@ closeDescriptors(void) #endif if ((dp = opendir(FD_DIR)) == NULL) - return 0; + return -1; while ((dirp = readdir(dp)) != NULL) { int fd; if (isAsciiDigit(dirp->d_name[0]) && - (fd = strtol(dirp->d_name, NULL, 10)) >= from_fd + 2) - close(fd); + (fd = strtol(dirp->d_name, NULL, 10)) >= fd_from) { + if (markCloseOnExec(fd) == -1) { + closedir(dp); + return -1; + } + } } closedir(dp); - return 1; + return 0; } static int @@ -394,11 +407,11 @@ childProcess(void *arg) fail_pipe_fd = FAIL_FILENO; /* close everything */ - if (closeDescriptors() == 0) { /* failed, close the old way */ + if (markDescriptorsCloseOnExec() == -1) { /* failed, close the old way */ int max_fd = (int)sysconf(_SC_OPEN_MAX); int fd; - for (fd = FAIL_FILENO + 1; fd < max_fd; fd++) - if (close(fd) == -1 && errno != EBADF) + for (fd = STDERR_FILENO + 1; fd < max_fd; fd++) + if (markCloseOnExec(fd) == -1 && errno != EBADF) goto WhyCantJohnnyExec; } @@ -413,9 +426,6 @@ childProcess(void *arg) sigprocmask(SIG_SETMASK, &unblock_signals, NULL); } - if (fcntl(FAIL_FILENO, F_SETFD, FD_CLOEXEC) == -1) - goto WhyCantJohnnyExec; - // Children should be started with default signal disposition for SIGPIPE if (signal(SIGPIPE, SIG_DFL) == SIG_ERR) { goto WhyCantJohnnyExec; diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java new file mode 100644 index 00000000000..146c2be563f --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/FDLeakTest.java @@ -0,0 +1,72 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + + +/** + * @test id=posix_spawn + * @summary Check that we don't leak FDs + * @requires os.family != "windows" + * @library /test/lib + * @run main/othervm/native -Djdk.lang.Process.launchMechanism=posix_spawn -agentlib:FDLeaker FDLeakTest + */ + +/** + * @test id=fork + * @summary Check that we don't leak FDs + * @requires os.family != "windows" + * @library /test/lib + * @run main/othervm/native -Djdk.lang.Process.launchMechanism=fork -agentlib:FDLeaker FDLeakTest + */ + +/** + * @test id=vfork + * @summary Check that we don't leak FDs + * @requires os.family == "linux" + * @library /test/lib + * @run main/othervm/native -Djdk.lang.Process.launchMechanism=vfork -agentlib:FDLeaker FDLeakTest + */ + +import jdk.test.lib.process.ProcessTools; +public class FDLeakTest { + // This test has two native parts: + // - a library invoked with -agentlib that ensures that, in the parent JVM, we open + // a native fd without setting FD_CLOEXEC (libFDLeaker.c). This is necessary because + // there is no way to do this from Java: if Java functions correctly, all files the + // user could open via its APIs should be marked with FD_CLOEXEC. + // - a small native executable that tests - without using /proc - whether any file + // descriptors other than stdin/out/err are open. + // + // What should happen: In the child process, between the initial fork and the exec of + // the target binary, we should close all filedescriptors that are not stdin/out/err. + // If that works, the child process should not see any other file descriptors save + // those three. + public static void main(String[] args) throws Exception { + ProcessBuilder pb = ProcessTools.createNativeTestProcessBuilder("FDLeakTester"); + pb.inheritIO(); + Process p = pb.start(); + p.waitFor(); + if (p.exitValue() != 0) { + throw new RuntimeException("Failed"); + } + } +} diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c new file mode 100644 index 00000000000..b47228ee5de --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/exeFDLeakTester.c @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2021, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#include +#include +#include +#include +#include + +/* Check if any fd past stderr is valid; if true, print warning on stderr and return -1 + * + * Note: check without accessing /proc since: + * - non-portable + * - may cause creation of temporary file descriptors + */ +int main(int argc, char** argv) { + int errors = 0; + int rc = 0; + char buf[128]; + int max_fd = (int)sysconf(_SC_OPEN_MAX); + if (max_fd == -1) { + snprintf(buf, sizeof(buf), "*** sysconf(_SC_OPEN_MAX) failed? (%d) ***\n", errno); + rc = write(2, buf, strlen(buf)); + max_fd = 10000; + } + // We start after stderr fd + for (int fd = 3; fd < max_fd; fd++) { + if (fcntl(fd, F_GETFD, 0) >= 0) { + // Error: found valid file descriptor + errors++; + snprintf(buf, sizeof(buf), "*** Parent leaked file descriptor %d ***\n", fd); + rc = write(2, buf, strlen(buf)); + } + } + return errors > 0 ? -1 : 0; +} diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c new file mode 100644 index 00000000000..b3fa296cae2 --- /dev/null +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2025, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +#include +#include "jvmti.h" + +JNIEXPORT jint JNICALL +Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) { + const char* filename = "./testfile_FDLeaker.txt"; + FILE* f = fopen(filename, "w"); + if (f == NULL) { + return JNI_ERR; + } + printf("Opened and leaked %s (%d)", filename, fileno(f)); + return JNI_OK; +} From 24b779bc35c9515ffaa0ecb1acbbb6bf252f084d Mon Sep 17 00:00:00 2001 From: Guanqiang Han Date: Thu, 7 Aug 2025 14:11:46 +0000 Subject: [PATCH 2/4] bugtrail fix: 8364822 --- src/hotspot/os/aix/os_aix.cpp | 3 +-- src/hotspot/os/bsd/os_bsd.cpp | 3 +-- src/hotspot/os/linux/os_linux.cpp | 5 ++--- src/java.base/unix/native/libjava/childproc.c | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/hotspot/os/aix/os_aix.cpp b/src/hotspot/os/aix/os_aix.cpp index e6d8791367a..c9b8a4498f0 100644 --- a/src/hotspot/os/aix/os_aix.cpp +++ b/src/hotspot/os/aix/os_aix.cpp @@ -2420,8 +2420,7 @@ int os::open(const char *path, int oflag, int mode) { // specifically destined for a subprocess should have the // close-on-exec flag set. If we don't set it, then careless 3rd // party native code might fork and exec without closing all - // appropriate file descriptors (e.g. as we do in closeDescriptors in - // UNIXProcess.c), and this in turn might: + // appropriate file descriptors, and this in turn might: // // - cause end-of-file to fail to be detected on some file // descriptors, resulting in mysterious hangs, or diff --git a/src/hotspot/os/bsd/os_bsd.cpp b/src/hotspot/os/bsd/os_bsd.cpp index bfbea849768..db5f83ef3d6 100644 --- a/src/hotspot/os/bsd/os_bsd.cpp +++ b/src/hotspot/os/bsd/os_bsd.cpp @@ -2336,8 +2336,7 @@ int os::open(const char *path, int oflag, int mode) { // specifically destined for a subprocess should have the // close-on-exec flag set. If we don't set it, then careless 3rd // party native code might fork and exec without closing all - // appropriate file descriptors (e.g. as we do in closeDescriptors in - // UNIXProcess.c), and this in turn might: + // appropriate file descriptors, and this in turn might: // // - cause end-of-file to fail to be detected on some file // descriptors, resulting in mysterious hangs, or diff --git a/src/hotspot/os/linux/os_linux.cpp b/src/hotspot/os/linux/os_linux.cpp index 760c44a500b..d1d78db1381 100644 --- a/src/hotspot/os/linux/os_linux.cpp +++ b/src/hotspot/os/linux/os_linux.cpp @@ -5050,9 +5050,8 @@ int os::open(const char *path, int oflag, int mode) { // All file descriptors that are opened in the Java process and not // specifically destined for a subprocess should have the close-on-exec // flag set. If we don't set it, then careless 3rd party native code - // might fork and exec without closing all appropriate file descriptors - // (e.g. as we do in closeDescriptors in UNIXProcess.c), and this in - // turn might: + // might fork and exec without closing all appropriate file descriptors, + // and this in turn might: // // - cause end-of-file to fail to be detected on some file // descriptors, resulting in mysterious hangs, or diff --git a/src/java.base/unix/native/libjava/childproc.c b/src/java.base/unix/native/libjava/childproc.c index afbe1eea762..c4b5a2d7b29 100644 --- a/src/java.base/unix/native/libjava/childproc.c +++ b/src/java.base/unix/native/libjava/childproc.c @@ -372,7 +372,7 @@ childProcess(void *arg) jtregSimulateCrash(0, 6); #endif /* Close the parent sides of the pipes. - Closing pipe fds here is redundant, since closeDescriptors() + Closing pipe fds here is redundant, since markDescriptorsCloseOnExec() would do it anyways, but a little paranoia is a good thing. */ if ((closeSafely(p->in[1]) == -1) || (closeSafely(p->out[0]) == -1) || From 48d90f7bd5a387fee191518e73fc575a24627f84 Mon Sep 17 00:00:00 2001 From: Joachim Kern Date: Wed, 24 Sep 2025 07:38:23 +0000 Subject: [PATCH 3/4] bugtrail fix: JDK-8360401 --- src/java.base/unix/native/libjava/childproc.c | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/src/java.base/unix/native/libjava/childproc.c b/src/java.base/unix/native/libjava/childproc.c index c4b5a2d7b29..9c6334e52d2 100644 --- a/src/java.base/unix/native/libjava/childproc.c +++ b/src/java.base/unix/native/libjava/childproc.c @@ -67,24 +67,39 @@ markCloseOnExec(int fd) return 0; } +#if !defined(_AIX) + /* The /proc file system on AIX does not contain open system files + * like /dev/random. Therefore we use a different approach and do + * not need isAsciiDigit() or FD_DIR */ static int isAsciiDigit(char c) { return c >= '0' && c <= '9'; } -#if defined(_AIX) - /* AIX does not understand '/proc/self' - it requires the real process ID */ - #define FD_DIR aix_fd_dir -#elif defined(_ALLBSD_SOURCE) - #define FD_DIR "/dev/fd" -#else - #define FD_DIR "/proc/self/fd" + #if defined(_ALLBSD_SOURCE) + #define FD_DIR "/dev/fd" + #else + #define FD_DIR "/proc/self/fd" + #endif #endif static int markDescriptorsCloseOnExec(void) { +#if defined(_AIX) + /* On AIX, we cannot rely on proc file system iteration to find all open files. Since + * iteration over all possible file descriptors, and subsequently closing them, can + * take a very long time, we use a bulk close via `ioctl` that is available on AIX. + * Since we hard-close, we need to make sure to keep the fail pipe file descriptor + * alive until the exec call. Therefore we mark the fail pipe fd with close on exec + * like the other OSes do, but then proceed to hard-close file descriptors beyond that. + */ + if (fcntl(FAIL_FILENO + 1, F_CLOSEM, 0) == -1 || + (markCloseOnExec(FAIL_FILENO) == -1 && errno != EBADF)) { + return -1; + } +#else DIR *dp; struct dirent *dirp; /* This function marks all file descriptors beyond stderr as CLOEXEC. @@ -93,12 +108,6 @@ markDescriptorsCloseOnExec(void) * execve. */ const int fd_from = STDERR_FILENO + 1; -#if defined(_AIX) - /* AIX does not understand '/proc/self' - it requires the real process ID */ - char aix_fd_dir[32]; /* the pid has at most 19 digits */ - snprintf(aix_fd_dir, 32, "/proc/%d/fd", getpid()); -#endif - if ((dp = opendir(FD_DIR)) == NULL) return -1; @@ -114,6 +123,7 @@ markDescriptorsCloseOnExec(void) } closedir(dp); +#endif return 0; } @@ -406,6 +416,10 @@ childProcess(void *arg) /* We moved the fail pipe fd */ fail_pipe_fd = FAIL_FILENO; + /* For AIX: The code in markDescriptorsCloseOnExec() relies on the current + * semantic of this function. When this point here is reached only the + * FDs 0,1,2 and 3 are further used until the exec() or the exit(-1). */ + /* close everything */ if (markDescriptorsCloseOnExec() == -1) { /* failed, close the old way */ int max_fd = (int)sysconf(_SC_OPEN_MAX); From 656194e9addef70ff174a72b37145c4864e2e1e9 Mon Sep 17 00:00:00 2001 From: Stefan Karlsson Date: Wed, 3 Sep 2025 13:51:17 +0000 Subject: [PATCH 4/4] bugtrail fix: JDK-8366298 --- .../ProcessBuilder/FDLeakTest/libFDLeaker.c | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c index b3fa296cae2..afec94f65c7 100644 --- a/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c +++ b/test/jdk/java/lang/ProcessBuilder/FDLeakTest/libFDLeaker.c @@ -21,16 +21,55 @@ * questions. */ +#include #include +#include +#include +#include #include "jvmti.h" +static jint limit_num_fds(); + JNIEXPORT jint JNICALL Agent_OnLoad(JavaVM *jvm, char *options, void *reserved) { + // Lower the number of possible open files to make the test go faster + jint ret = limit_num_fds(); + if (ret != 0) { + fprintf(stderr, "Failed to limit number of fds: %s", strerror(errno)); + return ret; + } + const char* filename = "./testfile_FDLeaker.txt"; FILE* f = fopen(filename, "w"); if (f == NULL) { + fprintf(stderr, "Failed to open file: %s", strerror(errno)); return JNI_ERR; } + printf("Opened and leaked %s (%d)", filename, fileno(f)); return JNI_OK; } + +static jint limit_num_fds() { + struct rlimit rl; + + // Fetch the current limit + int ret = getrlimit(RLIMIT_NOFILE, &rl); + if (ret != 0) { + return JNI_ERR; + } + + // Use a lower value unless it is already low + rlim_t limit = 100; + if (limit < rl.rlim_cur) { + rl.rlim_cur = limit; + } + + // Lower the value + int ret2 = setrlimit(RLIMIT_NOFILE, &rl); + if (ret2 != 0) { + return JNI_ERR; + } + + return JNI_OK; +}