[crac] RFR: Improved open file descriptors tracking [v9]

[Anton Kozlov]

On Mon, 24 Apr 2023 13:27:30 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> Tracks `java.io.FileDescriptor` instances as CRaC resource; before checkpoint these are reported and if not allow-listed (e.g. as opened as standard descriptors) an exception is thrown. Further investigation can use system property `jdk.crac.collect-fd-stacktraces=true` to record origin of those file descriptors.
>> File descriptors claimed in Java code are passed to native; native code checks all open file descriptors and reports error if there's an unexpected FD that is not included in the list passed previously.
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Provide more information for file descriptors

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

> 6090:     int ilen = snprintf(msg, maxinfo, "FD fd=%d type=%s path=", i, type);
> 6091:     ilen = ilen > maxinfo ? maxinfo : ilen;
> 6092:     strncpy(msg + ilen, detailsbuf, buflen - ilen);

`ilen >= maxinfo` if the output was truncated [1]

`strncpy` may leave the string unterminated if `buflen-ilen` smaller than details.

So `snprintf(msg, maxinfo, "FD fd=%d type=%s path=%s", i, type, details)` will be a safer option. 

[1] RETURN in https://linux.die.net/man/3/snprintf

src/java.base/share/classes/java/io/FileDescriptor.java line 400:

> 398:                 } else {
> 399:                     info = (path != null ? path : "unknown path") + " (" + (type != null ? type : "unknown") + ")";
> 400:                 }

This have too many socket-related details, also a number of java/native transitions that will be unavoidable if we adopt the proposed interface.

src/java.base/share/classes/java/net/Socket.java line 1939:

> 1937:      * @return Textual representation of the type.
> 1938:      */
> 1939:     public static native String getType(int fd);

`int fd` is a Unix platfrom detail.

I propose a single function `public static String getDescription(FileDescriptor socket)`. That e.g. returns `null` if the FileDescriptor is not a socket. The method can be a native or not, depends on the implementation.

src/java.base/unix/native/libnet/SocketImpl.c line 115:

> 113:            return NULL;
> 114:         }
> 115:     }

No need for `} else {` here and everywhere else since the previous block has anyway terminated the function.

This will make the code more streamlined.

Suggestion:

}

localAddr = create_isa(env, isa_class, isa_ctor, &local);
if (localAddr == NULL) {
    JNU_ThrowOutOfMemoryError(env, "java.net.InetSocketAddres");
    return NULL;
}

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

PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1182827339
PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1182802505
PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1182794395
PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1182815623