Skip to content

Commit

Permalink
Do not close GraalVM Native image FileSystem after classpath scanning
Browse files Browse the repository at this point in the history 
As can be seen in a modified version of the following example project,
attempting to access a resource discovered via classpath scanning
within a GraalVM native image -- for example via the Files.exists(...)
invocation in FileSystemResource -- currently results in a
ClosedFileSystemException since PathMatchingResourcePatternResolver
explicitly closes the native image FileSystem that backs `resource:`
resources.

Sample project: https://github.com/joshlong/spring-boot-3-aot-graphql

To address this issue, this commit removes the explicit close()
invocation for custom FileSystems after classpath scanning in
PathMatchingResourcePatternResolver.

See spring-projects/spring-graphql#495
Closes gh-29397
  • Loading branch information
@sbrannen
sbrannen committed Oct 28, 2022
1 parent 2ccfc70 commit 3c42363
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 60 deletions.
105 changes: 48 additions & 57 deletions ...rc/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.net.URL;
import java.net.URLClassLoader;
import java.net.URLConnection;
import java.nio.file.FileSystem;
import java.nio.file.FileSystemNotFoundException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
Expand Down Expand Up @@ -748,76 +747,68 @@ protected Set<Resource> doFindPathMatchingFileResources(Resource rootDirResource
return Collections.emptySet();
}

FileSystem fileSystem = null;
try {
Path rootPath = null;
if (rootDirUri.isAbsolute() && !rootDirUri.isOpaque()) {
// Prefer Path resolution from URI if possible
Path rootPath = null;
if (rootDirUri.isAbsolute() && !rootDirUri.isOpaque()) {
// Prefer Path resolution from URI if possible
try {
try {
try {
rootPath = Path.of(rootDirUri);
}
catch (FileSystemNotFoundException ex) {
// If the file system was not found, assume it's a custom file system that needs to be installed.
fileSystem = FileSystems.newFileSystem(rootDirUri, Map.of(), ClassUtils.getDefaultClassLoader());
rootPath = Path.of(rootDirUri);
}
rootPath = Path.of(rootDirUri);
}
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to resolve %s in file system: %s".formatted(rootDirUri, ex));
}
// Fallback via Resource.getFile() below
catch (FileSystemNotFoundException ex) {
// If the file system was not found, assume it's a custom file system that needs to be installed.
FileSystems.newFileSystem(rootDirUri, Map.of(), ClassUtils.getDefaultClassLoader());
rootPath = Path.of(rootDirUri);
}
}
if (rootPath == null) {
// Resource.getFile() resolution as a fallback -
// for custom URI formats and custom Resource implementations
rootPath = Path.of(rootDirResource.getFile().getAbsolutePath());
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to resolve %s in file system: %s".formatted(rootDirUri, ex));
}
// Fallback via Resource.getFile() below
}
}
if (rootPath == null) {
// Resource.getFile() resolution as a fallback -
// for custom URI formats and custom Resource implementations
rootPath = Path.of(rootDirResource.getFile().getAbsolutePath());
}

String rootDir = StringUtils.cleanPath(rootPath.toString());
if (!rootDir.endsWith("/")) {
rootDir += "/";
}
String rootDir = StringUtils.cleanPath(rootPath.toString());
if (!rootDir.endsWith("/")) {
rootDir += "/";
}

Path rootPathForPattern = rootPath;
String resourcePattern = rootDir + StringUtils.cleanPath(subPattern);
Predicate<Path> isMatchingFile = path -> (!path.equals(rootPathForPattern) &&
getPathMatcher().match(resourcePattern, StringUtils.cleanPath(path.toString())));
Path rootPathForPattern = rootPath;
String resourcePattern = rootDir + StringUtils.cleanPath(subPattern);
Predicate<Path> isMatchingFile = path -> (!path.equals(rootPathForPattern) &&
getPathMatcher().match(resourcePattern, StringUtils.cleanPath(path.toString())));

if (logger.isTraceEnabled()) {
logger.trace("Searching directory [%s] for files matching pattern [%s]"
.formatted(rootPath.toAbsolutePath(), subPattern));
}
if (logger.isTraceEnabled()) {
logger.trace("Searching directory [%s] for files matching pattern [%s]"
.formatted(rootPath.toAbsolutePath(), subPattern));
}

Set<Resource> result = new LinkedHashSet<>();
try (Stream<Path> files = Files.walk(rootPath)) {
files.filter(isMatchingFile).sorted().forEach(file -> {
try {
result.add(new FileSystemResource(file));
}
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to convert file %s to an org.springframework.core.io.Resource: %s"
.formatted(file, ex));
}
Set<Resource> result = new LinkedHashSet<>();
try (Stream<Path> files = Files.walk(rootPath)) {
files.filter(isMatchingFile).sorted().forEach(file -> {
try {
result.add(new FileSystemResource(file));
}
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to convert file %s to an org.springframework.core.io.Resource: %s"
.formatted(file, ex));
}
});
}
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to complete search in directory [%s] for files matching pattern [%s]: %s"
.formatted(rootPath.toAbsolutePath(), subPattern, ex));
}
}
return result;
});
}
finally {
if (fileSystem != null) {
fileSystem.close();
catch (Exception ex) {
if (logger.isDebugEnabled()) {
logger.debug("Failed to complete search in directory [%s] for files matching pattern [%s]: %s"
.formatted(rootPath.toAbsolutePath(), subPattern, ex));
}
}
return result;
}

/**
Expand Down
25 changes: 22 additions & 3 deletions ...st/java/org/springframework/core/io/support/PathMatchingResourcePatternResolverTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.URL;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
Expand Down Expand Up @@ -129,8 +130,16 @@ void usingClasspathStarProtocolWithWildcardInPatternAndEndingInSlash() throws Ex

List<String> actualSubPaths = getSubPathsIgnoringClassFilesEtc(pattern, pathPrefix);

// We do NOT find "support" if the pattern ENDS with a slash.
assertThat(actualSubPaths).isEmpty();
URL url = getClass().getClassLoader().getResource("org/springframework/core/io/support/EncodedResource.class");
if (!url.getProtocol().equals("jar")) {
// We do NOT find "support" if the pattern ENDS with a slash if org/springframework/core/io/support
// is in the local file system.
assertThat(actualSubPaths).isEmpty();
}
else {
// But we do find "support/" if org/springframework/core/io/support is found in a JAR on the classpath.
assertThat(actualSubPaths).containsExactly("support/");
}
}

@Test
Expand Down Expand Up @@ -258,6 +267,7 @@ private void assertFilenames(String pattern, boolean exactly, String... filename
try {
Resource[] resources = resolver.getResources(pattern);
List<String> actualNames = Arrays.stream(resources)
.peek(resource -> assertThat(resource.exists()).as(resource + " exists").isTrue())
.map(Resource::getFilename)
.sorted()
.toList();
Expand Down Expand Up @@ -304,7 +314,16 @@ private String getPath(Resource resource) {
// GraalVM native image which cannot support Path#toFile.
//
// See: https://github.com/spring-projects/spring-framework/issues/29243
return ((FileSystemResource) resource).getPath();
if (resource instanceof FileSystemResource fileSystemResource) {
return fileSystemResource.getPath();
}
try {
// Fall back to URL in case the resource came from a JAR
return resource.getURL().getPath();
}
catch (IOException ex) {
throw new UncheckedIOException(ex);
}
}

}

0 comments on commit 3c42363

Please sign in to comment.