[crac] RFR: Handle open file descriptors with configurable policies [v6]

[Anton Kozlov]

On Thu, 15 Jun 2023 11:27:43 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> When the application does not close some file descriptors through Resources we can use `jdk.crac.file-policy.checkpoint`, `jdk.crac.file-policy.restore`, `jdk.crac.socket-policy.checkpoint` and `jdk.crac.socket-policy.checkpoint` to configure the behaviour.
>> 
>> These properties can specify a list of semicolon-separated key=value pairs, where the key can be one of:
>> 
>> * numeric file descriptor
>> * for `file-policy`, path using 'glob' pattern matching (see FileSystem.getPathMatcher() for details) - this matches named pipes, too
>> * for `file-policy` keyword `FIFO` matching all pipes (including anonymous)
>> * for `socket-policy` a `<local>,<remote>` pair, with the `<remote>` part being optional. Both `<local>` and `<remote>` can be unix socket path, IPv4/IPv6 address with optional colon and port number or wildcard `*` replacing any of those parts.
>> 
>> The possible values are in OpenFilePolicies.BeforeCheckpoint, OpenFilePolicies.AfterRestore, OpenSocketPolicies.BeforeCheckpoint and OpenSocketPolicies.AfterRestore enums.
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   cleanup

Some random notes from first reading, I'll do another pass. This is not a request to change anything.

src/java.base/share/classes/java/io/FileInputStream.java line 539:

> 537:         }
> 538:     };
> 539: 

Providing full range of policies for FileInputStreams seems inherently unsafe, e.g. *_AT_END. I also think we need to distinguish FileInputStreams created with a path or with fd.

src/java.base/share/classes/java/io/FileOutputStream.java line 470:

> 468: 
> 469:         @Override
> 470:         protected Supplier<Exception> claimException(int fdNum, String path) {

Here and everywhere else, please use platform independent `FileDescriptor`.

src/java.base/share/classes/java/io/RandomAccessFile.java line 89:

> 87:             return fd;
> 88:         }
> 89:     };

The path known to RandomAccessFile and one read from the OS may differ, if the former is symlink. So the RandomAccessFile may need to override getPath, getOffset,.. Apparently, RandomAccessFile don't need operating system intraspection at all.

src/java.base/share/classes/java/lang/ProcessBuilder.java line 715:

> 713:         final FileDescriptor fd;
> 714:         @SuppressWarnings("unused")
> 715:         final JDKFileResource resource = new JDKFileResource(this) {

Should really a pipe be handled as a File?

src/java.base/share/classes/java/net/DatagramSocketImpl.java line 79:

> 77:     // We don't know the protocol family when this socket is created and FD allocated, but it's not UNIX
> 78:     @SuppressWarnings("unused")
> 79:     private final JDKSocketResource resource = new JDKSocketResource(this, StandardProtocolFamily.INET, () -> fd);

Is there family notion on java level? Having this comment, it seems we may stop considering family.

src/java.base/share/classes/jdk/crac/impl/OpenFilePolicies.java line 220:

> 218:          * and opens the file at the end.
> 219:          */
> 220:         OPEN_OTHER_AT_END,

Semantically, these modes are the same, the second is for FD opened in APPEND mode. But having both modes allows changing the FD mode for the application. Such change will break app assumptions about the file descriptor.

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

PR Review: https://git.openjdk.org/crac/pull/69#pullrequestreview-1474975958
PR Review Comment: https://git.openjdk.org/crac/pull/69#discussion_r1232257310
PR Review Comment: https://git.openjdk.org/crac/pull/69#discussion_r1232240489
PR Review Comment: https://git.openjdk.org/crac/pull/69#discussion_r1232210712
PR Review Comment: https://git.openjdk.org/crac/pull/69#discussion_r1232211126
PR Review Comment: https://git.openjdk.org/crac/pull/69#discussion_r1232212645
PR Review Comment: https://git.openjdk.org/crac/pull/69#discussion_r1232237388