diff --git a/renaissance-core/src/main/java/org/renaissance/core/BenchmarkSuite.java b/renaissance-core/src/main/java/org/renaissance/core/BenchmarkSuite.java index 462a4d64..69588ffc 100644 --- a/renaissance-core/src/main/java/org/renaissance/core/BenchmarkSuite.java +++ b/renaissance-core/src/main/java/org/renaissance/core/BenchmarkSuite.java @@ -283,6 +283,10 @@ private static ClassLoader createClassLoaderFromPaths( throw new ExtensionException("malformed URL(s) in classpath specification"); } + // + // No need to explicitly close this URLClassLoader on exit, because it does not + // operate on files created by the harness that need to be deleted on exit. + // ClassLoader parent = ModuleLoader.class.getClassLoader(); return new URLClassLoader(classPathUrls, parent); } diff --git a/renaissance-core/src/main/java/org/renaissance/core/Cleaner.java b/renaissance-core/src/main/java/org/renaissance/core/Cleaner.java new file mode 100644 index 00000000..fcd10f93 --- /dev/null +++ b/renaissance-core/src/main/java/org/renaissance/core/Cleaner.java @@ -0,0 +1,76 @@ +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 registeredCloseables = new LinkedHashSet<>(); + private static Set registeredPaths = new LinkedHashSet<>(); + + private Cleaner() {} + + static Path deleteOnExit(Path path) { + return register(registeredPaths, path); + } + + static T closeOnExit(T closeable) { + return register(registeredCloseables, closeable); + } + + private synchronized static T register(Set registry, T item) { + if (registry == null) { + throw new IllegalStateException("Shutdown in progress"); + } else { + registry.add(Objects.requireNonNull(item)); + } + + return item; + } + + private static void run() { + Set closeables; + Set paths; + + 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. + // Delete files/directories AFTER closing closeables. + + cleanupEach(closeables, "close", Closeable::close).clear(); + cleanupEach(paths, "delete", DirUtils::deleteRecursively).clear(); + } + + @FunctionalInterface + private interface Action { + void accept(T object) throws IOException; + } + + private static > C cleanupEach(C items, String name, Action action) { + for (T item : items) { + try { + action.accept(item); + } catch (IOException e) { + // Just a notification. This should be rare and is not critical. + System.err.format("warning: failed to %s %s on shutdown: %s\n", name, item, e); + } + } + + return items; + } + + static { + Runtime.getRuntime().addShutdownHook(new Thread(Cleaner::run)); + } + +} diff --git a/renaissance-core/src/main/java/org/renaissance/core/DirUtils.java b/renaissance-core/src/main/java/org/renaissance/core/DirUtils.java index fcb53c35..9f845a76 100644 --- a/renaissance-core/src/main/java/org/renaissance/core/DirUtils.java +++ b/renaissance-core/src/main/java/org/renaissance/core/DirUtils.java @@ -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 to delete this directory anyway. + throw exc; + } + Files.delete(dir); } @@ -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(); - } - } diff --git a/renaissance-core/src/main/java/org/renaissance/core/Logging.java b/renaissance-core/src/main/java/org/renaissance/core/Logging.java index 18346e2a..6310e117 100644 --- a/renaissance-core/src/main/java/org/renaissance/core/Logging.java +++ b/renaissance-core/src/main/java/org/renaissance/core/Logging.java @@ -1,8 +1,8 @@ package org.renaissance.core; +import java.util.logging.ConsoleHandler; import java.util.logging.Level; import java.util.logging.Logger; -import java.util.logging.SimpleFormatter; import java.util.logging.StreamHandler; public final class Logging { @@ -30,8 +30,8 @@ private static StreamHandler createHandler() { System.setProperty(FORMAT_PROPERTY, DEFAULT_FORMAT); } - // Create SimpleFormatter AFTER setting the system property. - return new StreamHandler(System.err, new SimpleFormatter()); + // Creates SimpleFormatter AFTER setting the system property. + return new ConsoleHandler(); } private Logging() {} diff --git a/renaissance-core/src/main/java/org/renaissance/core/ModuleLoader.java b/renaissance-core/src/main/java/org/renaissance/core/ModuleLoader.java index 49f294eb..b352a7af 100644 --- a/renaissance-core/src/main/java/org/renaissance/core/ModuleLoader.java +++ b/renaissance-core/src/main/java/org/renaissance/core/ModuleLoader.java @@ -158,7 +158,15 @@ ClassLoader createClassLoaderForModule(String name) throws ModuleLoadingExceptio name, filePaths.size(), makeClassPath(filePaths) )); - return new URLClassLoader(urls, thisClass.getClassLoader()); + // + // Make sure to explicitly close this URLClassLoader on exit, because it operates + // on files created by the harness in the scratch directory hierarchy that need to + // be deleted on exit. Leaving the class loader open keeps the library JAR files + // open, preventing removal of the scratch directories. This is because on NFS, + // deleting an open file produces a NFS temporary file in the same directory, and + // on Windows, an open file cannot be deleted at all. + // + return Cleaner.closeOnExit(new URLClassLoader(urls, thisClass.getClassLoader())); } catch (IOException e) { // Just wrap the underlying IOException.