From f95af744b07a9ec87e2507b3d584cbcddc827bbd Mon Sep 17 00:00:00 2001 From: Guanqiang Han Date: Wed, 6 Aug 2025 15:37:31 +0000 Subject: [PATCH] 8364312: debug agent should set FD_CLOEXEC flag rather than explicitly closing every open file Reviewed-by: cjplummer, kevinw --- .../unix/native/libjdwp/exec_md.c | 87 ++++++++++--------- 1 file changed, 44 insertions(+), 43 deletions(-) diff --git a/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c b/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c index 096a70fe524..3a66e451519 100644 --- a/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c +++ b/src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c @@ -30,6 +30,7 @@ #include #include #include +#include #include "sys.h" #include "util.h" #include "error_messages.h" @@ -48,6 +49,21 @@ static char *skipNonWhitespace(char *p) { return p; } +static 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; +} + #if defined(_AIX) /* AIX does not understand '/proc/self' - it requires the real process ID */ #define FD_DIR aix_fd_dir @@ -57,73 +73,60 @@ static char *skipNonWhitespace(char *p) { #define FD_DIR "/proc/self/fd" #endif -// Closes every file descriptor that is listed as a directory -// entry in "/proc/self/fd" (or its equivalent). Standard -// input/output/error file descriptors will not be closed -// by this function. This function returns 0 on failure -// and 1 on success. +// Marks all file descriptors found in /proc/self/fd with the +// FD_CLOEXEC flag to ensure they are automatically closed +// upon execution of a new program via exec(). This function +// returns -1 on failure and 0 on success. static int -closeDescriptors(void) +markDescriptorsCloseOnExec(void) { DIR *dp; struct dirent *dirp; - /* leave out standard input/output/error descriptors */ - int from_fd = STDERR_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 - * before calling opendir(), we close a couple explicitly, so that - * opendir() can then use these lowest numbered closed file - * descriptors afresh. - * - * WARNING: We are not allowed to return with a failure until after - * these two closes are done. forkedChildProcess() relies on this. */ - - close(from_fd); /* for possible use by opendir() */ - close(from_fd + 1); /* another one for good luck */ - from_fd += 2; /* leave out the 2 we just closed, which the opendir() may use */ + const int from_fd = STDERR_FILENO; #if defined(_AIX) - /* set FD_DIR for AIX which does not understand '/proc/self' - it - * requires the real process ID */ + /* 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) { ERROR_MESSAGE(("failed to open dir %s while determining" - " file descriptors to close for process %d", + " file descriptors to mark or close for process %d", FD_DIR, getpid())); - return 0; // failure + return -1; // failure } + int dir_fd = dirfd(dp); + while ((dirp = readdir(dp)) != NULL) { if (!isdigit(dirp->d_name[0])) { continue; } - const long fd = strtol(dirp->d_name, NULL, 10); - if (fd <= INT_MAX && fd >= from_fd) { - (void)close((int)fd); + int fd = strtol(dirp->d_name, NULL, 10); + if (fd <= INT_MAX && fd > from_fd && fd != dir_fd) { + if (markCloseOnExec(fd) == -1) { + (void)close((int)fd); + } } } (void)closedir(dp); - return 1; // success + return 0; // success } -// Does necessary housekeeping of a forked child process -// (like closing copied file descriptors) before -// execing the child process. This function never returns. +// Performs necessary housekeeping in the forked child process, +// such as marking copied file descriptors (except standard input/output/error) +// with FD_CLOEXEC to ensure they are closed during exec(). +// This function never returns. static void forkedChildProcess(const char *file, char *const argv[]) { - /* Close all file descriptors that have been copied over - * from the parent process due to fork(). */ - if (closeDescriptors() == 0) { /* failed, close the old way */ + /* Mark all file descriptors (except standard input/output/error) + * copied from the parent process with FD_CLOEXEC, so they are + * closed automatically upon exec(). */ + if (markDescriptorsCloseOnExec() < 0) { /* failed, close the old way */ /* Find max allowed file descriptors for a process * and assume all were opened for the parent process and * copied over to this child process. We close them all. */ @@ -131,13 +134,11 @@ forkedChildProcess(const char *file, char *const argv[]) JDI_ASSERT(max_fd != (rlim_t)-1); // -1 represents error /* close(), that we subsequently call, takes only int values */ JDI_ASSERT(max_fd <= INT_MAX); - /* Leave out standard input/output/error file descriptors. Also, - * leave out STDERR_FILENO +1 and +2 since closeDescriptors() - * already closed them, even when returning an error. */ - rlim_t i = STDERR_FILENO + 3; + /* leave out standard input/output/error file descriptors */ + rlim_t i = STDERR_FILENO + 1; ERROR_MESSAGE(("failed to close file descriptors of" " child process optimally, falling back to closing" - " %d file descriptors sequentially", (max_fd - i + 1))); + " %d file descriptors sequentially", (max_fd - i))); for (; i < max_fd; i++) { (void)close(i); }