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 e7037fbc7e3..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,15 +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; @@ -54,9 +62,6 @@ 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 55604babeeb..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 @@ -445,16 +447,20 @@ public void run() { if (Files.exists(pidFile)) { waitForShutdown(); } - final InstallationCleaner cleaner = new InstallationCleaner(environment, log); + // 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 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. + // This cleanup only runs if marker still exists and the directory is not deleted by an external process. cleaner.cleanupTimeout(); } } catch (IOException | InterruptedException e) { 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 e5d0f23f6ae..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. @@ -37,9 +33,9 @@ class InstallationCleaner implements Runnable { 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); @@ -47,14 +43,13 @@ class InstallationCleaner implements Runnable { @Override public synchronized void run() { - // Clean up is not already in progress if (Files.notExists(cleanupMarker)) { - try { - Files.createFile(cleanupMarker); - cleanup(); - } catch (InterruptedException | IOException e) { - logger.failedToStartCleanupProcess(e, environment.getJBossHome()); - } + return; + } + try { + cleanup(); + } catch (InterruptedException | IOException e) { + logger.failedToStartCleanupProcess(e, environment.getJBossHome()); } } @@ -98,15 +93,9 @@ 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(); + // If a new process has been started, it will delete the installation when this process ends, so do nothing. + if (!newProcess) { + deleteDirectory(); } } @@ -153,15 +142,14 @@ private void newProcess() throws IOException, InterruptedException { 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"))); process = builder.start(); - process.waitFor(environment.getTimeout(), TimeUnit.SECONDS); - process.destroyForcibly(); } 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(); + } } }