[crac] RFR: Linux file system watcher support [v3]

Radim Vansa duke at openjdk.org
Thu May 25 08:36:21 UTC 2023


On Tue, 23 May 2023 15:28:47 GMT, joeylee <duke at openjdk.org> wrote:

>> inotify monitors changes on filesystem, this support automatic restore for LinuxFileWatcher.
>> 
>> FileWatcherAfterRestoreTest verifies watcher service works after restore.
>> FileWatcherTest verifies automatic closing inotiify fd
>> 
>> The watcher keys are still managed by user, so exception will be thrown if no watcher keys are leaked, as in FileWatcherWithOpenKeysTest
>
> joeylee has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - update test
>  - update test

Alright, the test does not really show what I was asking for (an independent thread using the watch service and **reacting** to checkpoint triggered elsewhere, e.g. through `jcmd <pid> JDK.checkpoint`) but your comment explains that this change *might* be useful if the watch service is provided as static from a library beyond our control.

Therefore I think that once the technicalities are resolved this could be integrated @AntonKozlov .

src/java.base/linux/classes/sun/nio/fs/LinuxWatchService.java line 504:

> 502:                     try {
> 503:                         this.wait();
> 504:                     } catch (InterruptedException e) {

You shouldn't just ignore the interrupt; interrupt should break a loop and probably enter errored state and rethrow. Someone wanted us to stop working.

src/java.base/linux/classes/sun/nio/fs/LinuxWatchService.java line 524:

> 522:                     try {
> 523:                         this.wait();
> 524:                     } catch (InterruptedException e) {

Same here.

test/jdk/jdk/crac/fileDescriptors/FileWatcherAfterRestoreTest.java line 62:

> 60:         directory = Paths.get(System.getProperty("user.dir"), "workdir");
> 61:         directory.toFile().mkdir();
> 62:         Files.createTempFile(directory, "temp", ".txt");

You're creating the file inside `isMatchFound`, too.

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

PR Review: https://git.openjdk.org/crac/pull/72#pullrequestreview-1443332992
PR Review Comment: https://git.openjdk.org/crac/pull/72#discussion_r1205174591
PR Review Comment: https://git.openjdk.org/crac/pull/72#discussion_r1205174952
PR Review Comment: https://git.openjdk.org/crac/pull/72#discussion_r1205159796


More information about the crac-dev mailing list