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

[Anton Kozlov]

On Mon, 17 Apr 2023 09:04:39 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:
> 
>   On exception, invoke afterRestore on throwing Context, too.

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

> 377:                     msg += JDKContext.COLLECT_FD_STACKTRACES_HINT;
> 378:                 }
> 379:                 throw new CheckpointOpenFileException(msg, resource.stackTraceHolder);

Now, the message looks like:


	Suppressed: jdk.crac.impl.CheckpointOpenFileException: FileDescriptor 323 left open.
		at java.base/java.io.FileDescriptor.beforeCheckpoint(FileDescriptor.java:379)
		at java.base/java.io.FileDescriptor$Resource.beforeCheckpoint(FileDescriptor.java:80)
		at java.base/jdk.crac.impl.AbstractContextImpl.runBeforeCheckpoint(AbstractContextImpl.java:114)
		at java.base/jdk.crac.impl.AbstractContextImpl.beforeCheckpoint(AbstractContextImpl.java:83)
		at java.base/jdk.internal.crac.JDKContext.beforeCheckpoint(JDKContext.java:85)
		at java.base/jdk.crac.impl.AbstractContextImpl.runBeforeCheckpoint(AbstractContextImpl.java:114)
		at java.base/jdk.crac.impl.AbstractContextImpl.beforeCheckpoint(AbstractContextImpl.java:83)
		at java.base/jdk.crac.Core.checkpointRestore1(Core.java:121)
		... 2 more
	Caused by: java.lang.Exception: This file descriptor was created here
		at java.base/java.io.FileDescriptor$Resource.<init>(FileDescriptor.java:71)
		at java.base/java.io.FileDescriptor.<init>(FileDescriptor.java:100)
		at java.base/sun.nio.ch.IOUtil.newFD(IOUtil.java:544)
		at java.base/sun.nio.ch.Net.socket(Net.java:524)
		at java.base/sun.nio.ch.SocketChannelImpl.<init>(SocketChannelImpl.java:146)
		at java.base/sun.nio.ch.SocketChannelImpl.<init>(SocketChannelImpl.java:129)
		at java.base/sun.nio.ch.SelectorProviderImpl.openSocketChannel(SelectorProviderImpl.java:77)
		at java.base/java.nio.channels.SocketChannel.open(SocketChannel.java:192)


While in the past the FD (which is actually a socket, not a file) was reported by the VM like:


	Suppressed: jdk.crac.impl.CheckpointOpenSocketException: tcp localAddr 0.0.0.0 localPort 8080 remoteAddr 0.0.0.0 remotePort 0
		at java.base/jdk.crac.Core.translateJVMExceptions(Core.java:92)
		at java.base/jdk.crac.Core.checkpointRestore1(Core.java:157)


We miss the fact this is a socket now, as well as details (although the stack trace is very useful!).

We can ask the VM for the FD type and details. To so we'll report report the details as we used to, by reusing the existing code providing the details, that should not be very hard.

src/java.base/share/classes/jdk/crac/LoggerContainer.java line 9:

> 7:  * Therefore, we isolate the logger into a subclass and initialize lazily.
> 8:  */
> 9: public class LoggerContainer {

This should be package private at least.

src/java.base/share/classes/jdk/crac/impl/AbstractContextImpl.java line 31:

> 29: import java.util.concurrent.locks.ReentrantLock;
> 30: 
> 31: public abstract class AbstractContextImpl<R extends Resource, P> extends Context<R> {

`P` is not used anymore.

The semantic of this class is being changed significantly, as well as implementation. It would be very nice to extract these modifications into a separate PR with a more clear desription of the reason of the change,  tests, etc.

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

PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1171241969
PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1170350155
PR Review Comment: https://git.openjdk.org/crac/pull/43#discussion_r1170359272