RFR: 8331467: ImageReaderFactory can cause a ClassNotFoundException if the default FileSystemProvider is not the system-default provider [v5]

[Alan Bateman]

On Mon, 9 Dec 2024 12:31:54 GMT, liyazzi <duke at openjdk.org> wrote:

>> For two cases:
>> 
>> 1. When the ImageReaderFactory was loaded by local jdk,that means the ImageReaderFactory was loaded by boot class loader,then init the `Path BOOT_MODULES_JIMAGE` by using `sun.nio.fs.DefaultFileSystemProvider` which is obtained through reflection,due to it is in jdk internal.
>> 2. When loaded by a target jdk, such as jdk8 runtime, then use the Java 8 compatible APIs: `FileSystems.getDefault()` to init the `BOOT_MODULES_JIMAGE` field.
>> Then we can avoid the circular dependencies in class loading caused by loading the defaultSystemProvider.
>
> liyazzi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add '\s' to avoid trailing whiteSpace

The update to ImageReaderFactory is what I suggested in the other PR so that part is okay.

The proposed test is very different to what I would have expected, meaning I was surprised to see it setting BOOT_MODULES_JIMAGE and needing to run with --add-exports.  It can be massively simplified with `@build` tag to compile the test provider, using ToolProvider to run jlink with `--add-options` to set the system provider to override the default provider, e.g.

        String mlib = Path.of(System.getProperty("test.classes"), "modules").toString();
        String imageDir = "myimage";
        ToolProvider jlink = ToolProvider.findFirst("jlink").orElseThrow();
        int res = jlink.run(System.out, System.err,
                "--module-path", mlib,
                "--add-modules", "customfs",
                "--add-options", "-Djava.nio.file.spi.DefaultFileSystemProvider=customfs.CustomFileSystemProvider",
                "--output", imageDir);
        if (res != 0) {
            throw new RuntimeException("jlink failed");
        }

        String launcher = Path.of(imageDir, "bin", "java").toString();
        String[] cmd = { launcher, "--list-modules" };
        var pb = new ProcessBuilder(cmd);
        OutputAnalyzer outputAnalyzer = ProcessTools
                .executeProcess(pb)
                .outputTo(System.err)
                .errorTo(System.err);
        outputAnalyzer.shouldHaveExitValue(0);

Even better would if we updated test/jdk/java/nio/file/spi/SetDefaultProvider.java to add this as a test case. Right now, this test exercises several different configurations and this issue is really just another configuration.

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

PR Comment: https://git.openjdk.org/jdk/pull/22628#issuecomment-2528106421