Prevent ClasspathResourceDirectoryReader from accessing closed NativeImageResourceFileSystem

[linghengqian]

Fixes #30345.

Changes proposed in this pull request:

• Prevent ClasspathResourceDirectoryReader from accessing closed NativeImageResourceFileSystem . Call @iamhucong to help review.
• Check whether NativeImageResourceFileSystem has been created by a third-party dependency. Fixes JDBCRepositorySQLLoader native-image之后 loadFromDirectory方法,虽然进行了文件系统的创建,如果存在抛出java.nio.file.FileSystemAlreadyExistsException异常,没有考虑 #30344 .

---

Before committing this PR, I'm sure that I have checked the following options:

• My code follows the code of conduct of this project.
• I have self-reviewed the commit code.
• I have (or in comment I request) added corresponding labels for the pull request.
• I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
• I have made corresponding changes to the documentation.
• I have added corresponding unit tests for my changes.

[iamhucong]

@linghengqian Hi, I believe Stream can lazily consume resources. Are there any potential issues that I haven't considered?

[linghengqian]

@linghengqian Hi, I believe Stream can lazily consume resources. Are there any potential issues that I haven't considered?

• The Steam obtained by the ClasspathResourceDirectoryReader.read() function is not read immediately, which leads to a problem with the ClasspathResourceDirectoryReader.readDirectoryInFileSystem() function.
• com.oracle.svm.core.jdk.resources.NativeImageResourceFileSystem is opened and then closed, which causes the Stream instance returned by the ClasspathResourceDirectoryReader.read() function of the JDBCRepositorySQLLoader.load() operation to try to access a non-existent File System reads the contents of the Stream.
• Stream is still read lazily on Jar/jdk.nio.zipfs.ZipFileSystem. That won't change with this PR.

[iamhucong]

@linghengqian Hi, I believe Stream can lazily consume resources. Are there any potential issues that I haven't considered?

• The Steam obtained by the ClasspathResourceDirectoryReader.read() function is not read immediately, which leads to a problem with the ClasspathResourceDirectoryReader.readDirectoryInFileSystem() function.
• com.oracle.svm.core.jdk.resources.NativeImageResourceFileSystem is opened and then closed, which causes the Stream instance returned by the ClasspathResourceDirectoryReader.read() function of the JDBCRepositorySQLLoader.load() operation to try to access a non-existent File System reads the contents of the Stream.
• Stream is still read lazily on Jar/jdk.nio.zipfs.ZipFileSystem. That won't change with this PR.

FileSystem resourceFileSystem = FileSystems.newFileSystem(directoryUrl.toURI(), Collections.emptyMap())
return loadFromDirectory(directoryUrl).onClose(resourceFileSystem::close());

Could this code resolve those problems?

[linghengqian]

FileSystem resourceFileSystem = FileSystems.newFileSystem(directoryUrl.toURI(), Collections.emptyMap())
return loadFromDirectory(directoryUrl).onClose(resourceFileSystem::close());

Could this code resolve those problems?

• I don't quite understand where your logic should go. Stream is returned to org.apache.shardingsphere.mode.repository.standalone.jdbc.sql.JDBCRepositorySQLLoader normally. Once JDBCRepositorySQLLoader starts traversing Iterable<String> resourceNameIterable, it will start to access the closed File System. This has moved away from the logic of org.apache.shardingsphere.infra.util.directory.ClasspathResourceDirectoryReader.
• See https://github.com/apache/shardingsphere/blob/f356f3228fde7e37024188185b991310880940f4/mode/type/standalone/repository/provider/jdbc/src/main/java/org/apache/shardingsphere/mode/repository/standalone/jdbc/sql/JDBCRepositorySQLLoader.java .

  try (Stream<String> resourceNameStream = ClasspathResourceDirectoryReader.read(JDBCRepositorySQLLoader.class.getClassLoader(), ROOT_DIRECTORY)) {
             Iterable<String> resourceNameIterable = resourceNameStream::iterator;
             for (String each : resourceNameIterable) {

[iamhucong]

FileSystem resourceFileSystem = FileSystems.newFileSystem(directoryUrl.toURI(), Collections.emptyMap())
return loadFromDirectory(directoryUrl).onClose(resourceFileSystem::close());

Could this code resolve those problems?

• I don't quite understand where your logic should go. Stream is returned to org.apache.shardingsphere.mode.repository.standalone.jdbc.sql.JDBCRepositorySQLLoader normally. Once JDBCRepositorySQLLoader starts traversing Iterable<String> resourceNameIterable, it will start to access the closed File System. This has moved away from the logic of org.apache.shardingsphere.infra.util.directory.ClasspathResourceDirectoryReader.
• See https://github.com/apache/shardingsphere/blob/f356f3228fde7e37024188185b991310880940f4/mode/type/standalone/repository/provider/jdbc/src/main/java/org/apache/shardingsphere/mode/repository/standalone/jdbc/sql/JDBCRepositorySQLLoader.java .

  try (Stream<String> resourceNameStream = ClasspathResourceDirectoryReader.read(JDBCRepositorySQLLoader.class.getClassLoader(), ROOT_DIRECTORY)) {
             Iterable<String> resourceNameIterable = resourceNameStream::iterator;
             for (String each : resourceNameIterable) {

The resourceFileSystem.close() method will be called only when the resourceNameStream is closed, therefore the resourceFileSystem will be closed after the resourceNameStream has been used.

[iamhucong]

https://github.com/oracle/graal/blob/486451e0af9772d24e604668fb3c6058233bdce3/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/resources/NativeImageResourceFileSystem.java#L178-L204

    public void close() throws IOException {
        beginWrite();
        try {
            if (!isOpen) {
                return;
            }
            isOpen = false;
        } finally {
            endWrite();
        }


        if (!inputStreams.isEmpty()) {    // Unlock and close all remaining input streams.
            Set<InputStream> copy = new HashSet<>(inputStreams);
            for (InputStream is : copy) {
                is.close();
            }
        }


        if (!outputStreams.isEmpty()) {    // Unlock and close all remaining output streams.
            Set<OutputStream> copy = new HashSet<>(outputStreams);
            for (OutputStream os : copy) {
                os.close();
            }
        }


        provider.removeFileSystem();
    }

Alternatively, is it possible to consider not closing the resource file system? It seems that the close() method only closes some input/output streams, but I'm not very familiar with GraalVM.

[linghengqian]

The resourceFileSystem.close() method will be called only when the resourceNameStream is closed, therefore the resourceFileSystem will be closed after the resourceNameStream has been used.

• I chose this approach and made a new commit.

Alternatively, is it possible to consider not closing the resource file system? It seems that the close() method only closes some input/output streams, but I'm not very familiar with GraalVM.

• According to Cannot open native image resource using Path, NativeImageResourceFileSystemProvider not initialised oracle/graal#7682 , the behavior of NativeImageResourceFileSystem is as consistent as possible with jdk.nio.zipfs.ZipFileSystem/Jar. Therefore, affected by https://bugs.openjdk.org/browse/JDK-8175243, developers are always required to manually switch File System on and off.
• In the Context instance of the Spring Framework, NativeImageResourceFileSystem will be opened for a long time during the life cycle of the Spring Context, so the PR needs to discuss two situations.
• Therefore I don't think PR can open NativeImageResourceFileSystem for a long time, which may conflict with the behavior of third-party dependencies.

[iamhucong]

Should a file system that has already been created not be closed?

[linghengqian]

• According to my understanding of JDBCRepositorySQLLoader native-image之后 loadFromDirectory方法,虽然进行了文件系统的创建,如果存在抛出java.nio.file.FileSystemAlreadyExistsException异常,没有考虑 #30344, if FileSystems.getFileSystem() does not throw java.nio.file.FileSystemNotFoundException, it means that com.oracle.svm.core.jdk.resources.NativeImageResourceFileSystem is already in a third-party dependency life cycle, and additional judgments here are meaningless.
• So I made my third commit. Call @huyu-tom .What do you think of this? This still doesn't break NativeTest.