Close extraneous file descriptors

Reviewed-by: akozlov

src/hotspot/os/linux/os_linux.cpp

@@ -5750,6 +5750,7 @@ void os::Linux::vm_create_start() {
   if (!CRaCCheckpointTo) {
     return;
   }
+  close_extra_descriptors();
   _vm_inited_fds.initialize();
 }
 
@@ -6202,7 +6203,7 @@ void VM_Crac::doit() {
       continue;
     }
 
-    char detailsbuf[128];
+    char detailsbuf[PATH_MAX];
     int linkret = readfdlink(i, detailsbuf, sizeof(detailsbuf));
     const char* details = 0 < linkret ? detailsbuf : "";
     print_resources("JVM: FD fd=%d type=%s: details1=\"%s\" ",
@@ -6456,6 +6457,69 @@ void os::Linux::restore() {
   }
 }
 
+static char modules_path[JVM_MAXPATHLEN] = { '\0' };
+
+static bool is_fd_ignored(int fd, const char *path) {
+  if (!strcmp(modules_path, path)) {
+    // Path to the modules directory is opened early when JVM is booted up and won't be closed.
+    // We can ignore this for purposes of CRaC.
+    return true;
+  }
+
+  const char *list = CRaCIgnoredFileDescriptors;
+  while (list && *list) {
+    const char *end = strchr(list, ',');
+    if (!end) {
+      end = list + strlen(list);
+    }
+    char *invalid;
+    int ignored_fd = strtol(list, &invalid, 10);
+    if (invalid == end) { // entry was integer -> file descriptor
+      if (fd == ignored_fd) {
+        log_trace(os)("CRaC not closing file descriptor %d (%s) as it is marked as ignored.", fd, path);
+        return true;
+      }
+    } else { // interpret entry as path
+      int path_len = path ? strlen(path) : -1;
+      if (path_len != -1 && path_len == end - list && !strncmp(path, list, end - list)) {
+        log_trace(os)("CRaC not closing file descriptor %d (%s) as it is marked as ignored.", fd, path);
+        return true;
+      }
+    }
+    if (*end) {
+      list = end + 1;
+    } else {
+      break;
+    }
+  }
+  return false;
+}
+
+void os::Linux::close_extra_descriptors() {
+  // Path to the modules directory is opened early when JVM is booted up and won't be closed.
+  // We can ignore this for purposes of CRaC.
+  if (modules_path[0] == '\0') {
+    const char* fileSep = os::file_separator();
+    jio_snprintf(modules_path, JVM_MAXPATHLEN, "%s%slib%smodules", Arguments::get_java_home(), fileSep, fileSep);
+  }
+
+  char path[PATH_MAX];
+  struct dirent *dp;
+
+  DIR *dir = opendir("/proc/self/fd");
+  while (dp = readdir(dir)) {
+    int fd = atoi(dp->d_name);
+    if (fd > 2 && fd != dirfd(dir)) {
+      int r = readfdlink(fd, path, sizeof(path));
+      if (!is_fd_ignored(fd, r != -1 ? path : nullptr)) {
+        log_warning(os)("CRaC closing file descriptor %d: %s\n", fd, path);
+        close(fd);
+      }
+    }
+  }
+  closedir(dir);
+}
+
 bool CracRestoreParameters::read_from(int fd) {
   struct stat st;
   if (fstat(fd, &st)) {

src/hotspot/os/linux/os_linux.hpp

@@ -175,6 +175,7 @@ class Linux {
   static bool prepare_checkpoint();
   static Handle checkpoint(bool dry_run, jlong jcmd_stream, TRAPS);
   static void restore();
+  static void close_extra_descriptors();
   static void register_persistent_fd(int fd, int st_dev, int st_ino);
   static void deregister_persistent_fd(int fd, int st_dev, int st_ino);
 

src/hotspot/share/runtime/globals.hpp

@@ -2100,6 +2100,11 @@ const intx ObjectAlignmentInBytes = 8;
       "-XX:CRaCRestoreFrom and continue initialization if restore is "      \
       "unavailable")                                                        \
                                                                             \
+  product(ccstr, CRaCIgnoredFileDescriptors, NULL, "Comma-separated list "  \
+      "of file descriptor numbers or paths. All file descriptors greater "  \
+      "than 2 (stdin, stdout and stderr are excluded automatically) not "   \
+      "in this list are closed when the VM is started.")                    \
+                                                                            \
   product(bool, CRAllowToSkipCheckpoint, false, DIAGNOSTIC,                 \
           "Allow implementation to not call Checkpoint if helper not found")\
                                                                             \

test/jdk/jdk/crac/CheckpointRestore.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright (c) 2023, Azul Systems, Inc. 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+import jdk.crac.CheckpointException;
+import jdk.crac.Core;
+import jdk.crac.RestoreException;
+
+class CheckpointRestore {
+    static final String RESTORED_MESSAGE = "Restored";
+
+    public static void main(String[] args) throws CheckpointException, RestoreException {
+        Core.checkpointRestore();
+        System.out.println(RESTORED_MESSAGE);
+    }
+}

test/jdk/jdk/crac/FileDescriptorsCloseTest.java

@@ -0,0 +1,117 @@
+/*
+ * Copyright (c) 2023, Azul Systems, Inc. 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.
+ */
+
+import jdk.crac.CheckpointException;
+import jdk.crac.RestoreException;
+import jdk.test.lib.JDKToolFinder;
+import jdk.test.lib.Utils;
+import jdk.test.lib.process.ProcessTools;
+
+import java.io.IOException;
+import java.lang.reflect.Method;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.*;
+import java.util.stream.Collectors;
+
+/**
+ * @test
+ * @library /test/lib
+ * @build CheckpointRestore
+ * @run main FileDescriptorsCloseTest testCheckpointWithOpenFds
+ * @run main FileDescriptorsCloseTest testIgnoredFileDescriptors
+ */
+public class FileDescriptorsCloseTest {
+    private static final String EXTRA_FD_WRAPPER = Path.of(Utils.TEST_SRC, "extra_fd_wrapper.sh").toString();
+
+    public static void main(String[] args) throws Throwable {
+        if (args.length < 1) {
+            throw new IllegalArgumentException();
+        }
+        FileDescriptorsCloseTest.class.getMethod(args[0]).invoke(null);
+    }
+
+    public static void testCheckpointWithOpenFds() throws Throwable {
+        List<String> cmd = new ArrayList<>();
+        cmd.add(EXTRA_FD_WRAPPER);
+        cmd.add(JDKToolFinder.getJDKTool("java"));
+        cmd.add("-cp");
+        cmd.add(System.getProperty("java.class.path"));
+        cmd.add("-XX:CRaCCheckpointTo=./cr");
+        cmd.add(CheckpointRestore.class.getSimpleName());
+        // Note that the process is killed after checkpoint
+        ProcessTools.executeProcess(cmd.toArray(new String[0]))
+                .shouldHaveExitValue(137);
+
+        ProcessTools.executeTestJvm("-XX:CRaCRestoreFrom=./cr")
+                .shouldHaveExitValue(0)
+                .shouldContain(CheckpointRestore.RESTORED_MESSAGE);
+    }
+
+    public static void testIgnoredFileDescriptors() throws Throwable {
+        List<String> cmd = new ArrayList<>();
+        cmd.add(EXTRA_FD_WRAPPER);
+        cmd.addAll(Arrays.asList("-o", "43", "/dev/stdout"));
+        cmd.addAll(Arrays.asList("-o", "45", "/dev/urandom"));
+        cmd.add(JDKToolFinder.getJDKTool("java"));
+        cmd.add("-cp");
+        cmd.add(System.getProperty("java.class.path"));
+        cmd.add("-XX:CRaCCheckpointTo=./cr");
+        cmd.add("-XX:CRaCIgnoredFileDescriptors=43,/dev/null,44,/dev/urandom");
+        cmd.add("FileDescriptorsCloseTest$TestIgnoredDescriptors");
+        // Note that the process is killed after checkpoint
+        ProcessTools.executeProcess(cmd.toArray(new String[0]))
+                .shouldHaveExitValue(137);
+
+        ProcessTools.executeTestJvm("-XX:CRaCRestoreFrom=./cr")
+                .shouldHaveExitValue(0)
+                .shouldContain(CheckpointRestore.RESTORED_MESSAGE);
+    }
+
+    public static class TestIgnoredDescriptors {
+        public static void main(String[] args) throws IOException, RestoreException, CheckpointException {
+            try (var stream = Files.list(Path.of("/proc/self/fd"))) {
+                Map<Integer, String> fds = stream.filter(Files::isSymbolicLink)
+                        .collect(Collectors.toMap(
+                                f -> Integer.parseInt(f.toFile().getName()),
+                                f -> {
+                                    try {
+                                        return Files.readSymbolicLink(f).toFile().getAbsoluteFile().toString();
+                                    } catch (IOException e) {
+                                        throw new RuntimeException(e);
+                                    }
+                                }));
+                if (fds.containsKey(42)) {
+                    throw new IllegalStateException("Oh no, 42 was not supposed to be ignored");
+                } else if (!fds.containsKey(0) || !fds.containsKey(1) || !fds.containsKey(2)) {
+                    throw new IllegalStateException("Missing standard I/O? Available: " + fds);
+                } else if (!fds.containsKey(43)) {
+                    throw new IllegalStateException("Missing FD 43");
+                } else if (!fds.containsValue("/dev/urandom")) {
+                    throw new IllegalStateException("Missing /dev/urandom");
+                }
+            }
+            CheckpointRestore.main(args);
+        }
+    }
+}

test/jdk/jdk/crac/LeaveRunning.java

@@ -23,40 +23,29 @@
  * questions.
  */
 
-import jdk.crac.*;
-
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.process.ProcessTools;
 
 /**
  * @test
  * @library /test/lib
+ * @build CheckpointRestore
  * @run main LeaveRunning
  */
 public class LeaveRunning {
-    private static final String RESTORED_MESSAGE = "Restored";
-
-    static class Test {
-        public static void main(String[] args) throws CheckpointException, RestoreException {
-            Core.checkpointRestore();
-            System.out.println(RESTORED_MESSAGE);
-        }
-    }
-
     public static void main(String[] args) {
         OutputAnalyzer output;
         try {
             ProcessBuilder pb = ProcessTools.createTestJvm(
-                "-XX:CRaCCheckpointTo=./cr",
-                "LeaveRunning$Test");
+                "-XX:CRaCCheckpointTo=./cr", CheckpointRestore.class.getSimpleName());
             pb.environment().put("CRAC_CRIU_LEAVE_RUNNING", "");
             output = ProcessTools.executeProcess(pb);
         } catch (Exception e) {
             throw new RuntimeException(e);
         }
 
         output.shouldHaveExitValue(0);
-        output.shouldContain(RESTORED_MESSAGE);
+        output.shouldContain(CheckpointRestore.RESTORED_MESSAGE);
 
         try {
             output = ProcessTools.executeTestJvm(
@@ -66,6 +55,6 @@ public static void main(String[] args) {
         }
 
         output.shouldHaveExitValue(0);
-        output.shouldContain(RESTORED_MESSAGE);
+        output.shouldContain(CheckpointRestore.RESTORED_MESSAGE);
     }
 }

test/jdk/jdk/crac/extra_fd_wrapper.sh

@@ -0,0 +1,14 @@
+#!/bin/bash
+# Java opens all files with O_CLOEXEC (or calls fcntl(FD_CLOEXEC)) so we cannot trigger this behaviour from Java code;
+# this opens a file descriptor and executes subprocess based on its arguments.
+FILE=$(mktemp -p /dev/shm)
+exec 42<>$FILE
+# criu uses DEFAULT_GHOST_LIMIT 1M - let's create a file bigger than that
+dd if=/dev/urandom bs=4096 count=257 >&42 2>/dev/null
+rm $FILE
+# Open some extra files
+while [ $1 = "-o" ]; do
+  eval "exec $2<>$3"
+  shift 3
+done
+exec "$@"