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

[jyxzwd]

On Sun, 10 Nov 2024 17:04:12 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> If we load the custom DefaultFileSystemProvider by SystemClassLoader,then we will step into the method SystemModuleFinders$SystemModuleReader#findImageLocation again, and the var imageReader will be null.But we can not define a custom classLoader to load the custom DefaultFileSystemProvider since it needs to maintain JDK 8 source compatibility.So the only way that I can think of is to use the installedProvider which has the 'file' scheme to directly get the 'modules' path value.
>> 
>> In ImageReaderFactory:
>> 
>> private static final Path BOOT_MODULES_JIMAGE = null;
>> 
>>     static {
>>         // check installed providers
>>         for (FileSystemProvider provider : FileSystemProvider.installedProviders()) {
>>             if ("file".equalsIgnoreCase(provider.getScheme())) {
>>                 try {
>>                     BOOT_MODULES_JIMAGE = provider.getFileSystem(URI.create("file:/")).getPath(JAVA_HOME, "lib", "modules");
>>                     if (BOOT_MODULES_JIMAGE != null) break;
>>                 } catch (UnsupportedOperationException uoe) {
>>                 }
>>             }
>>         }
>>     }
>> 
>> What do you think of this?I am new to openjdk source development.So I truely appreciate the time you dedecate to my question!
>
> I assume this will lead to recursive initialisation, e.g. `FileSystems.getFileSystem(URI.create("jrt:/"))` will use FileSystemProvider.installedProviders to iterate over the installed providers.
> 
> I don't have time to look into this more but I think it will have to reflectively get the FileSystem so that the code isn't compiled with references to the built-in provider, as in something like this (not tested):
> 
> 
>     private static final Path BOOT_MODULES_JIMAGE;
>     static {
>         FileSystem fs;
>         if (ImageReaderFactory.class.getClassLoader() == null) {
>             // fs = DefaultFileSystemProvider.theFileSystem()
>             try {
>                 fs = (FileSystem) Class.forName("sun.nio.fs.DefaultFileSystemProvider")
>                         .getMethod("theFileSystem")
>                         .invoke(null);
>             } catch (Exception e) {
>                 throw new ExceptionInInitializerError(e);
>             }
>         } else {
>             fs = FileSystems.getDefault();
>         }
>         BOOT_MODULES_JIMAGE = fs.getPath(JAVA_HOME, "lib", "modules");
>     }
> 
> Also just to say that we need to create a good test for this issue. I expect it will be rarely to interpose on the default file system and use jrtfs at the same time, but that is what this bug report is about so we'll need to create a good test.

#### The reason why it is difficult to solve the issue through modifying ImageReadFactory directly

1. We only can use the `java8 `api
2. We can not access any classes which are not exported by `java.base` module,even by using java reflection.Because when there is a remote/target jdk, the class in jrt-fs.jar loaded by `JrtFsLoader`  is actually in `Unnamed Module`.

The essence of this issue is that when loading the Main class, which depends on Path.of, then it needs to load CustomFileSystemProvider, resulting in a circular dependency, which will access the fields of a class that is still being initialized.

The correct situation should be that Main class should be loaded with CustomFileSystemProvided, and when it comes to CustomFileSystemProvided it should use Built-in FileSystemProvider.

But if we following the above assumptions, and when we **first load the main class**, we will run to the `SystemModuleFinders# findImageLocation`  twice then the `ImageReader imageReader = SystemImage.reader()` will be null because we should load the customFileSystemProvider during main class loading and the the both of loading will use the same  `SystemModuleFinders# findImageLocation` method.So we must load the CustomFileSystemProvided **before** loading main class.

#### The solution

1. In `System#initPhase3` which will be called before loading main class, load the CustomFileSystemProvided after `VM.initLevel(4);`.Because we will use **SystemClassLoader**.
2. And we need also to ensure the return value of `FileSystems#getDefault` is `DefaultFileSystemProvider.theFileSystem`, which depends on the value of `VM.isModuleSystemInited()`. But now the `VM.initLevel` is **4** so the value of `VM.isModuleSystemInited()` is **false**. At first I want to change the judgment condition of `if` . That means I need to create a initLevel after `SYSTEM_BOOTED = 4`, but there is already `SYSTEM_SHUTDOWN = 5` defined in VM class.I can not set a `4.5` value to a new initLevel,so I add a new  boolen field to represent the status of loading CustomFileSystemProvider.Its initValue is **false** and its value can only be set from false to true so there is no need for lock.
3. Change the judgment condition of `if` to `if (VM.isModuleSystemInited()&&VM.isCustomDefaultFileSystemProviderLoaded())`
4. After load the CustomFileSystemProvided,  regardless of whether the  `-Djava.nio.file.spi.DefaultFileSystemProvider`  was set, it needs to call `VM.setCustomDefaultFileSystemProviderLoaded();`

After loading the CustomFileSystemProvided, the initialization of `Path BOOT_MODULES_JIMAGE = ImageReaderFactory#Paths.get(JAVA_HOME, "lib", "modules");` will use the fileSystem which is provided by the loaded CustomFileSystemProvided. Even it will also call `FileSystems#getDefaultProvider` , but during where it actually only use the constructor to new instance because relevent classes were already loaded.

For detail modification see the latest commit.

I recompile the modified source and test the situation mentioned in the issue(actually the issue is submitted by me through oracle bug submission because I am not openjdk author) and the normal situation, it successed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21997#discussion_r1850371330