8364312: debug agent should set FD_CLOEXEC flag rather than explicitly closing every open file

Reviewed-by: cjplummer, kevinw

src/jdk.jdwp.agent/unix/native/libjdwp/exec_md.c

@@ -30,6 +30,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <ctype.h>
+#include <fcntl.h>
 #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);
         }