[crac] RFR: Close extraneous file descriptors [v2]

[Anton Kozlov]

On Thu, 16 Feb 2023 09:27:07 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> Java code cannot use inherited non-standard file descriptors; these can cause trouble when trying to checkpoint the process via CRIU, and hence we will proactively close them on boot.
>
> Radim Vansa has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains three commits:
> 
>  - Remove unneeded check
>  - Applied review suggestions
>  - Close non-standard file descriptors on boot
>    
>    If the process is configured for Checkpoint and is started with open file descriptors (e.g. inherited from parent process), under some circumstances CRIU cannot properly execute the checkpoint. Prevent this be proactively closing file descriptors > 2 on boot.
>    This behaviour can be overriden by settings -XX:CRIgnoreFileDescriptors to a comma-separated list of FD numbers or paths these file descriptors map to.

Few minor comments

src/hotspot/os/linux/os_linux.cpp line 6501:

> 6499:     const char* fileSep = os::file_separator();
> 6500:     jio_snprintf(modules_path, JVM_MAXPATHLEN, "%s%slib%smodules", Arguments::get_java_home(), fileSep, fileSep);
> 6501:   }

A comment would be useful here. I cannot suggest a way to avoid this particular check, as modules are opened very early during argument parsing, to get possible additional arguments from the module, so let's document this for future readers of the code.

src/hotspot/os/linux/os_linux.cpp line 6512:

> 6510:       int r = readfdlink(fd, path, sizeof(path));
> 6511:       if (!is_fd_ignored(fd, r != -1 ? path : nullptr)) {
> 6512:         tty->print("CRaC closing file descriptor %d: %s\n", fd, path);

`log_trace(os)` to be consistent with line 6484?

src/hotspot/share/runtime/globals.hpp line 2103:

> 2101:       "unavailable")                                                        \
> 2102:                                                                             \
> 2103:   product(ccstr, CRaCIgnoredFileDescriptors, NULL, "Comma-separated list "  \

Should not `0,1,2` be the default option then? So it will be also possible to overwrite this default.

src/hotspot/share/runtime/globals.hpp line 2126:

> 2124:   product(bool, CRTrace, true, "Minimal C/R tracing")                       \
> 2125: 
> 2126: 

Unnecessary whitespace change

-------------

Changes requested by akozlov (Lead).

PR: https://git.openjdk.org/crac/pull/45