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..e7037fbc7e3 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 @@ -34,6 +34,9 @@ public static void main(final String[] args) throws Exception { final Path installDir = Paths.get(args[0]); final int retries = Integer.parseInt(args[1]); final Path cleanupMarker = installDir.resolve("wildfly-cleanup-marker"); + if (Files.notExists(cleanupMarker)) { + return; + } int attempts = 1; while (attempts <= retries) { final boolean lastAttempt = attempts == retries; @@ -51,6 +54,9 @@ public static void main(final String[] args) throws Exception { } private static void cleanup(final Path installDir, final Path cleanupMarker, final boolean log) throws IOException { + if (Files.notExists(cleanupMarker)) { + return; + } Files.walkFileTree(installDir, new SimpleFileVisitor() { @Override public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException { 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..55604babeeb 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 @@ -442,20 +442,22 @@ public void run() { thread.setName("installation-cleaner"); return thread; }); - final InstallationCleaner cleaner = new InstallationCleaner(environment, log); - executor.submit(cleaner); if (Files.exists(pidFile)) { waitForShutdown(); } + final InstallationCleaner cleaner = new InstallationCleaner(environment, log); + 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 + // We can't start a new full 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. + 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..e5d0f23f6ae 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 @@ -35,6 +35,7 @@ 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) { this.environment = environment; @@ -45,28 +46,13 @@ class InstallationCleaner implements Runnable { } @Override - public void run() { + public synchronized void run() { // Clean up is not already in progress 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) { + } catch (InterruptedException | IOException e) { logger.failedToStartCleanupProcess(e, environment.getJBossHome()); } } @@ -83,7 +69,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 +93,23 @@ 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; + } + // In case we have a running process, kill it to avoid it to continue any cleanup. + if (newProcess && process != null) { + process.destroyForcibly(); + process.waitFor(environment.getTimeout(), TimeUnit.SECONDS); + process = null; + } + // Do a last cleanup, in case the cleanupMarker still exists (could have been deleted by running process). + if (Files.exists(cleanupMarker)) { + cleanup(); + } + } + private void deleteDirectory() throws IOException { final Path installDir = environment.getJBossHome(); Files.walkFileTree(installDir, new SimpleFileVisitor() { @@ -136,7 +142,7 @@ public FileVisitResult postVisitDirectory(final Path dir, final IOException exc) }); } - private void newProcess() throws IOException { + private void newProcess() throws IOException, InterruptedException { // Start a new process which will clean up the install directory. This is done in a new process in cases where // this process may hold locks on to resources that need to be cleaned up. final String[] cmd = { @@ -153,7 +159,9 @@ private void newProcess() throws IOException { .redirectError(ProcessBuilder.Redirect.INHERIT) .redirectOutput(ProcessBuilder.Redirect.INHERIT) .directory(new File(System.getProperty("user.dir"))); - builder.start(); + process = builder.start(); + process.waitFor(environment.getTimeout(), TimeUnit.SECONDS); + process.destroyForcibly(); } private String getJavaCommand() {