Skip to content

Commit

Permalink
Centralize cleanup tasks and close created URLClassLoader instances
Browse files Browse the repository at this point in the history
This ensures that library JAR files stored in the launcher and
harness scratch directories are closed before removing the scratch
directory roots.

This was a problem on NFS, where the open files could not be unlinked
from the directory (NFS kept around '.nfs*' temporaries), preventing
the removal of the directories.
  • Loading branch information
lbulej committed Dec 4, 2024
1 parent 2b99467 commit 7d85049
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 16 deletions.
80 changes: 80 additions & 0 deletions renaissance-core/src/main/java/org/renaissance/core/Cleaner.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package org.renaissance.core;

import java.io.Closeable;
import java.io.IOException;
import java.nio.file.Path;
import java.util.LinkedHashSet;
import java.util.Objects;
import java.util.Set;

final class Cleaner {
private static Set<Closeable> registeredCloseables = new LinkedHashSet<>();
private static Set<Path> registeredPaths = new LinkedHashSet<>();

private Cleaner() {
// Not to be instantiated.
}

static synchronized Path deleteOnExit(Path path) {
if (registeredPaths == null) {
throw new IllegalStateException("Shutdown in progress");
} else {
registeredPaths.add(Objects.requireNonNull(path));
}

return path;
}

static synchronized <T extends Closeable> T closeOnExit(T closeable) {
if (registeredCloseables == null) {
throw new IllegalStateException("Shutdown in progress");
} else {
registeredCloseables.add(Objects.requireNonNull(closeable));
}

return closeable;
}

private static void run() {
Set<Path> paths;
Set<Closeable> closeables;

synchronized (Cleaner.class) {
closeables = registeredCloseables;
registeredCloseables = null;

paths = registeredPaths;
registeredPaths = null;
}

// Close closeables BEFORE deleting files/directories.
// In particular URLClassLoader instances which keep files open.

closeables.forEach(closeable -> {
try {
closeable.close();
} catch (IOException e) {
// Just a notification. This should be rare and is not critical.
System.err.format("warning: failed to close %s on shutdown: %s\n", closeable, e);
}
});

closeables.clear();

// Delete files/directories AFTER closing closeables.

paths.forEach(path -> {
try {
DirUtils.deleteRecursively(path);
} catch (IOException e) {
// Just a notification. This should be rare (not so much on NFS) and is not critical.
System.err.format("warning: failed to delete %s on shutdown: %s\n", path, e);
}
});
}

static {
Runtime.getRuntime().addShutdownHook(new Thread(Cleaner::run));
}

}
22 changes: 7 additions & 15 deletions renaissance-core/src/main/java/org/renaissance/core/DirUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
@Override
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
if (deleteRoot || !rootDir.equals(dir)) {
if (exc != null) {
// There was an earlier failure with one of the children, which means
// that we will not be able do delete this directory anyway.
throw exc;
}

Files.delete(dir);
}

Expand All @@ -52,24 +58,10 @@ public static Path createScratchDirectory(Path base, String prefix, boolean keep

Path scratchDir = createTempDirectory(base, tsPrefix).normalize();
if (!keepOnExit) {
Runtime.getRuntime().addShutdownHook(new Thread (() -> {
logger.fine(() -> "Deleting scratch directory: " + printable(scratchDir));
try {
deleteRecursively(scratchDir);
} catch (IOException e) {
// Just a notification. This should be rare and is not critical.
logger.warning(String.format(
"Error deleting scratch directory %s: %s", printable(scratchDir), e.getMessage()
));
}
}));
Cleaner.deleteOnExit(scratchDir);
}

return scratchDir;
}

private static Path printable(Path path) {
return path.toAbsolutePath().normalize();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ ClassLoader createClassLoaderForModule(String name) throws ModuleLoadingExceptio
name, filePaths.size(), makeClassPath(filePaths)
));

return new URLClassLoader(urls, thisClass.getClassLoader());
return Cleaner.closeOnExit(new URLClassLoader(urls, thisClass.getClassLoader()));

} catch (IOException e) {
// Just wrap the underlying IOException.
Expand Down

0 comments on commit 7d85049

Please sign in to comment.