Skip to content

Commit

Permalink
Make sure we don't keep a reference to the ZipPath in ArchivePathTree
Browse files Browse the repository at this point in the history 
A Path can actually contain references to the filesystem: this is the
case of the ZipPath.
We want to make sure we don't keep ZipFileSystem around when a path tree
is closed so we also need to nullify the path.

Finally, we make sure only DirectoryPathTree is marked as Serializable
as we only serialize the workspace directories.
  • Loading branch information
@gsmet
gsmet committed Aug 12, 2024
1 parent b6cd86d commit c222639
Show file tree
Hide file tree
Showing 4 changed files with 234 additions and 171 deletions.
55 changes: 30 additions & 25 deletions independent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/ArchivePathTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ protected FileSystem openFs() throws IOException {
@Override
public OpenPathTree open() {
try {
return new OpenArchivePathTree(openFs());
return new OpenArchivePathTree(archive, openFs());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand All @@ -224,15 +224,29 @@ public boolean equals(Object obj) {
&& manifestEnabled == other.manifestEnabled;
}

protected class OpenArchivePathTree extends DirectoryPathTree {
protected class OpenArchivePathTree extends OpenContainerPathTree {

// we don't make the field final as we want to nullify it on close
private volatile FileSystem fs;
private volatile boolean open = true;
private FileSystem fs;
private Path archivePath;
private Path rootPath;
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();

protected OpenArchivePathTree(FileSystem fs) {
super(fs.getPath("/"), pathFilter, ArchivePathTree.this);
protected OpenArchivePathTree(Path archive, FileSystem fs) {
super(ArchivePathTree.this.pathFilter, ArchivePathTree.this);
this.fs = fs;
this.rootPath = fs.getPath("/");
}

@Override
protected Path getContainerPath() {
return archivePath;
}

@Override
protected Path getRootPath() {
return rootPath;
}

protected ReentrantReadWriteLock.ReadLock readLock() {
Expand Down Expand Up @@ -272,7 +286,7 @@ protected void initMultiReleaseMapping(Map<String, String> mrMapping) {
public boolean isOpen() {
lock.readLock().lock();
try {
return fs != null && fs.isOpen();
return open;
} finally {
lock.readLock().unlock();
}
Expand Down Expand Up @@ -349,7 +363,7 @@ public Path getPath(String relativePath) {
*/
private void ensureOpen() {
// let's not use isOpen() as ensureOpen() is always used inside a read lock
if (fs != null && fs.isOpen()) {
if (open) {
return;
}
throw new RuntimeException("Failed to access " + ArchivePathTree.this.getRoots()
Expand All @@ -358,28 +372,19 @@ private void ensureOpen() {

@Override
public void close() throws IOException {
Throwable t = null;
lock.writeLock().lock();
try {
super.close();
} catch (Throwable e) {
t = e;
open = false;
rootPath = null;
fs.close();
} catch (IOException e) {
throw e;
} finally {
try {
fs.close();
} catch (IOException e) {
if (t != null) {
e.addSuppressed(t);
}
throw e;
} finally {
// even when we close the fs, everything is kept as is in the fs instance
// and typically the cen, which is quite large
// let's make sure the fs is nullified for it to be garbage collected
fs = null;
lock.writeLock().unlock();
}
// even when we close the fs, everything is kept as is in the fs instance
// and typically the cen, which is quite large
// let's make sure the fs is nullified for it to be garbage collected
fs = null;
lock.writeLock().unlock();
}
}

Expand Down
155 changes: 12 additions & 143 deletions ...endent-projects/bootstrap/app-model/src/main/java/io/quarkus/paths/DirectoryPathTree.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,55 +2,14 @@

import java.io.IOException;
import java.io.Serializable;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.List;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.regex.Pattern;

public class DirectoryPathTree extends PathTreeWithManifest implements OpenPathTree, Serializable {
public class DirectoryPathTree extends OpenContainerPathTree implements Serializable {

private static final long serialVersionUID = 2255956884896445059L;

private static final boolean USE_WINDOWS_ABSOLUTE_PATH_PATTERN = !FileSystems.getDefault().getSeparator().equals("/");

private static volatile Pattern windowsAbsolutePathPattern;

private static Pattern windowsAbsolutePathPattern() {
return windowsAbsolutePathPattern == null ? windowsAbsolutePathPattern = Pattern.compile("[a-zA-Z]:\\\\.*")
: windowsAbsolutePathPattern;
}

static boolean isAbsolutePath(String path) {
return path != null && !path.isEmpty()
&& (path.charAt(0) == '/' // we want to check for '/' on every OS
|| USE_WINDOWS_ABSOLUTE_PATH_PATTERN
&& (windowsAbsolutePathPattern().matcher(path).matches())
|| path.startsWith(FileSystems.getDefault().getSeparator()));
}

static void ensureResourcePath(FileSystem fs, String path) {
if (isAbsolutePath(path)) {
throw new IllegalArgumentException("Expected a path relative to the root of the path tree but got " + path);
}
// this is to disallow reading outside the path tree root
if (path != null && path.contains("..")) {
for (Path pathElement : fs.getPath(path)) {
if (pathElement.toString().equals("..")) {
throw new IllegalArgumentException("'..' cannot be used in resource paths, but got " + path);
}
}
}
}

private Path dir;
private PathFilter pathFilter;

/**
* For deserialization
Expand All @@ -68,90 +27,37 @@ public DirectoryPathTree(Path dir, PathFilter pathFilter) {
}

public DirectoryPathTree(Path dir, PathFilter pathFilter, boolean manifestEnabled) {
super(manifestEnabled);
super(pathFilter, manifestEnabled);
this.dir = dir;
this.pathFilter = pathFilter;
}

protected DirectoryPathTree(Path dir, PathFilter pathFilter, PathTreeWithManifest pathTreeWithManifest) {
super(pathTreeWithManifest);
super(pathFilter, pathTreeWithManifest);
this.dir = dir;
this.pathFilter = pathFilter;
}

@Override
public Collection<Path> getRoots() {
return List.of(dir);
protected Path getRootPath() {
return dir;
}

@Override
public void walk(PathVisitor visitor) {
PathTreeVisit.walk(dir, dir, dir, pathFilter, getMultiReleaseMapping(), visitor);
protected Path getContainerPath() {
return dir;
}

@Override
public void walkIfContains(String relativePath, PathVisitor visitor) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return;
}
final Path walkDir = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
if (!Files.exists(walkDir)) {
return;
}
PathTreeVisit.walk(dir, dir, walkDir, pathFilter, getMultiReleaseMapping(), visitor);
}

private void ensureResourcePath(String path) {
ensureResourcePath(dir.getFileSystem(), path);
}

@Override
protected <T> T apply(String relativePath, Function<PathVisit, T> func, boolean manifestEnabled) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return func.apply(null);
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
if (!Files.exists(path)) {
return func.apply(null);
}
return PathTreeVisit.process(dir, dir, path, pathFilter, func);
}

@Override
public void accept(String relativePath, Consumer<PathVisit> consumer) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
consumer.accept(null);
return;
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
if (!Files.exists(path)) {
consumer.accept(null);
return;
}
PathTreeVisit.consume(dir, dir, path, pathFilter, consumer);
public boolean isOpen() {
return true;
}

@Override
public boolean contains(String relativePath) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return false;
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
return Files.exists(path);
public void close() throws IOException {
}

@Override
public Path getPath(String relativePath) {
ensureResourcePath(relativePath);
if (!PathFilter.isVisible(pathFilter, relativePath)) {
return null;
}
final Path path = dir.resolve(manifestEnabled ? toMultiReleaseRelativePath(relativePath) : relativePath);
return Files.exists(path) ? path : null;
public PathTree getOriginalTree() {
return this;
}

private void writeObject(java.io.ObjectOutputStream out) throws IOException {
Expand All @@ -165,41 +71,4 @@ private void readObject(java.io.ObjectInputStream in) throws IOException, ClassN
pathFilter = (PathFilter) in.readObject();
manifestEnabled = in.readBoolean();
}

@Override
public OpenPathTree open() {
return this;
}

@Override
public boolean isOpen() {
return true;
}

@Override
public void close() throws IOException {
}

@Override
public PathTree getOriginalTree() {
return this;
}

@Override
public int hashCode() {
return Objects.hash(dir, pathFilter, manifestEnabled);
}

@Override
public boolean equals(Object obj) {
if (this == obj)
return true;
if (obj == null)
return false;
if (getClass() != obj.getClass())
return false;
DirectoryPathTree other = (DirectoryPathTree) obj;
return Objects.equals(dir, other.dir) && Objects.equals(pathFilter, other.pathFilter)
&& manifestEnabled == other.manifestEnabled;
}
}
Loading

0 comments on commit c222639

Please sign in to comment.