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() { @@ -136,7 +131,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 = { @@ -147,13 +142,14 @@ private void newProcess() throws IOException { System.getProperty("java.class.path"), "org.wildfly.core.jar.boot.CleanupProcessor", environment.getJBossHome().toString(), - Integer.toString(retries) + Integer.toString(retries), + Long.toString(ProcessHandle.current().pid()) }; final ProcessBuilder builder = new ProcessBuilder(cmd) .redirectError(ProcessBuilder.Redirect.INHERIT) .redirectOutput(ProcessBuilder.Redirect.INHERIT) .directory(new File(System.getProperty("user.dir"))); - builder.start(); + process = builder.start(); } private String getJavaCommand() { diff --git a/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/_private/BootableJarLogger.java b/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/_private/BootableJarLogger.java index 86880e36fc7..e35e769ec3e 100644 --- a/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/_private/BootableJarLogger.java +++ b/bootable-jar/runtime/src/main/java/org/wildfly/core/jar/runtime/_private/BootableJarLogger.java @@ -164,4 +164,8 @@ public interface BootableJarLogger extends BasicLogger { @LogMessage(level = DEBUG) @Message(id = 25, value = "Failed to initialize a security provider. Reason: %s") void securityProviderFailed(Throwable ex); + + @LogMessage(level = WARN) + @Message(id = 26, value = "Can't create %s. Exception %s") + void cantCreate(String path, IOException ioex); } diff --git a/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/provisioning/InstallationManagerBootTestCase.java b/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/provisioning/InstallationManagerBootTestCase.java index ca0da6232fd..fd2f57c3739 100644 --- a/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/provisioning/InstallationManagerBootTestCase.java +++ b/testsuite/manualmode/src/test/java/org/jboss/as/test/manualmode/provisioning/InstallationManagerBootTestCase.java @@ -16,6 +16,7 @@ import org.jboss.as.controller.client.ModelControllerClient; import org.jboss.as.test.integration.management.util.ServerReload; import org.jboss.as.test.module.util.TestModule; +import org.jboss.as.test.shared.AssumeTestGroupUtil; import org.jboss.as.test.shared.TestSuiteEnvironment; import org.jboss.as.test.shared.logging.TestLogHandlerSetupTask; import org.jboss.shrinkwrap.api.asset.StringAsset; @@ -65,7 +66,10 @@ public static void createTestModule() throws IOException { public static void removeTestModule() { if (testModule != null) { controller.stop(); - testModule.remove(); + // Bootable JAR fully delete the installation + if (!AssumeTestGroupUtil.isBootableJar()) { + testModule.remove(); + } } }