diff --git a/bootable-jar/boot/src/main/java/org/wildfly/core/jar/boot/CleanupProcessor.java b/bootable-jar/boot/src/main/java/org/wildfly/core/jar/boot/CleanupProcessor.java index 5bdb9084311..2038fd275bf 100644 --- a/bootable-jar/boot/src/main/java/org/wildfly/core/jar/boot/CleanupProcessor.java +++ b/bootable-jar/boot/src/main/java/org/wildfly/core/jar/boot/CleanupProcessor.java @@ -15,11 +15,11 @@ import java.util.concurrent.TimeUnit; /** - * An entry point that expects a first parameter of an install directory and a second parameter of the number of retries - * used to delete the install directory. + * An entry point that expects a first parameter of an install directory, a second parameter of the number of retries + * used to delete the install directory and finally the pid of the process that started it. *
- * The retries are used for cases where a process may still have files locked and this process is executed before the - * process has fully exited. A 0.5 second sleep will happen between each retry. + * The retries are used for cases where some un-expected errors occurs during deletion. + * A 0.5 second sleep will happen between each retry. *
* * @author James R. Perkins @@ -28,12 +28,23 @@ public class CleanupProcessor { public static void main(final String[] args) throws Exception { - if (args == null || args.length != 2) { - throw new IllegalArgumentException("The path to the install directory and number of retires are required."); + if (args == null || args.length != 3) { + throw new IllegalArgumentException("The path to the install directory, number of retires and pid are required."); } final Path installDir = Paths.get(args[0]); final int retries = Integer.parseInt(args[1]); + final long pid = Long.parseLong(args[2]); final Path cleanupMarker = installDir.resolve("wildfly-cleanup-marker"); + // An invalid case, do not attempt to delete an installation that has no marker. + if (Files.notExists(cleanupMarker)) { + return; + } + // Wait until the process that started this process is terminated. + // Starting the deletion of the installation during the other process shutdown + // has no guarantee that the installation will be fully cleaned up. + while (ProcessHandle.of(pid).isPresent()) { + TimeUnit.MILLISECONDS.sleep(500L); + } int attempts = 1; while (attempts <= retries) { final boolean lastAttempt = attempts == retries; diff --git a/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/BootableJar.java b/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/BootableJar.java index 4fd278d4bc8..b3b9f53a571 100644 --- a/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/BootableJar.java +++ b/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/BootableJar.java @@ -106,6 +106,7 @@ public final class BootableJar implements ShutdownHandler { private final Arguments arguments; private final ModuleLoader loader; private final Path pidFile; + private final Path cleanupMarker; private BootableJar(BootableEnvironment environment, Arguments arguments, ModuleLoader loader, long unzipTime) throws Exception { this.environment = environment; @@ -123,6 +124,7 @@ private BootableJar(BootableEnvironment environment, Arguments arguments, Module log.advertiseInstall(environment.getJBossHome(), unzipTime + (System.currentTimeMillis() - t)); pidFile = environment.getPidFile(); + cleanupMarker = environment.getJBossHome().resolve("wildfly-cleanup-marker"); } @Override @@ -442,20 +444,26 @@ public void run() { thread.setName("installation-cleaner"); return thread; }); - final InstallationCleaner cleaner = new InstallationCleaner(environment, log); - executor.submit(cleaner); if (Files.exists(pidFile)) { waitForShutdown(); } + // We create this marker file synchronously + try { + Files.createFile(cleanupMarker); + } catch(IOException ex) { + log.cantCreate(pidFile.toString(), ex); + } + final InstallationCleaner cleaner = new InstallationCleaner(environment, log, cleanupMarker); + executor.submit(cleaner); executor.shutdown(); try { if (!executor.awaitTermination(environment.getTimeout(), TimeUnit.SECONDS)) { // For some reason we've timed out. The deletion should likely be executing. - // We can't start a new cleanup to force it. On Windows we would have the side effect to have 2 cleaner processes to - // be executed, with the risk that a new installation has been installed and the new cleaner cleaning the new installation log.cleanupTimeout(environment.getTimeout(), environment.getJBossHome()); + // This cleanup only runs if marker still exists and the directory is not deleted by an external process. + cleaner.cleanupTimeout(); } - } catch (InterruptedException e) { + } catch (IOException | InterruptedException e) { // The task has been interrupted, leaving log.cleanupTimeout(environment.getTimeout(), environment.getJBossHome()); } diff --git a/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/InstallationCleaner.java b/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/InstallationCleaner.java index f00c393a913..bce8b3cb2ad 100644 --- a/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/InstallationCleaner.java +++ b/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/InstallationCleaner.java @@ -13,15 +13,11 @@ import java.nio.file.Paths; import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; -import java.util.concurrent.TimeUnit; import org.wildfly.core.jar.runtime._private.BootableJarLogger; /** - * Allows for cleanup of a bootable JAR installation. The {@link #run()} method blocks until the - * {@linkplain BootableEnvironment#getPidFile() PID file} is deleted or a - * {@linkplain BootableEnvironment#getTimeout() timeout} is reached. Then there is an attempt to delete the install - * directory. + * Allows for cleanup of a bootable JAR installation. *
* If the {@code org.wildfly.core.jar.cleanup.newProcess} system property is set to {@code true}, the default for Windows,
* a new process will be launched to delete the install directory.
@@ -35,40 +31,25 @@ class InstallationCleaner implements Runnable {
private final BootableJarLogger logger;
private final boolean newProcess;
private final int retries;
+ private Process process;
- InstallationCleaner(final BootableEnvironment environment, final BootableJarLogger logger) {
+ InstallationCleaner(final BootableEnvironment environment, final BootableJarLogger logger, Path cleanupMarker) {
this.environment = environment;
- cleanupMarker = environment.getJBossHome().resolve("wildfly-cleanup-marker");
+ this.cleanupMarker = cleanupMarker;
this.logger = logger;
newProcess = getProperty("org.wildfly.core.jar.cleanup.newProcess", environment.isWindows());
retries = getProperty("org.wildfly.core.jar.cleanup.retries", 3);
}
@Override
- public void run() {
- // Clean up is not already in progress
+ public synchronized void run() {
if (Files.notExists(cleanupMarker)) {
- try {
- Files.createFile(cleanupMarker);
- long timeout = environment.getTimeout() * 1000;
- final Path pidFile = environment.getPidFile();
- final long wait = 500L;
- while (Files.exists(pidFile)) {
- try {
- TimeUnit.MILLISECONDS.sleep(wait);
- } catch (InterruptedException ignore) {
- break;
- }
- timeout -= wait;
- if (timeout <= 0) {
- logger.cleanupTimeout(environment.getTimeout(), pidFile);
- break;
- }
- }
- cleanup();
- } catch (IOException e) {
- logger.failedToStartCleanupProcess(e, environment.getJBossHome());
- }
+ return;
+ }
+ try {
+ cleanup();
+ } catch (InterruptedException | IOException e) {
+ logger.failedToStartCleanupProcess(e, environment.getJBossHome());
}
}
@@ -83,7 +64,10 @@ public void run() {
*
* @throws IOException if an error occurs deleting the directory
*/
- void cleanup() throws IOException {
+ private void cleanup() throws InterruptedException, IOException {
+ if (Files.notExists(cleanupMarker)) {
+ return;
+ }
if (newProcess) {
try {
newProcess();
@@ -104,6 +88,17 @@ void cleanup() throws IOException {
}
}
+ // In case of timeout, we attempt to do a cleanup only if the cleanupMarker exists
+ synchronized void cleanupTimeout() throws IOException, InterruptedException {
+ if (Files.notExists(cleanupMarker)) {
+ return;
+ }
+ // If a new process has been started, it will delete the installation when this process ends, so do nothing.
+ if (!newProcess) {
+ deleteDirectory();
+ }
+ }
+
private void deleteDirectory() throws IOException {
final Path installDir = environment.getJBossHome();
Files.walkFileTree(installDir, new SimpleFileVisitor