Fix for WFCORE-7098, InstallationManagerBootTestCase shouldn't delete…

… module when ts.bootable

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.
  * <p>
- * 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.
  * </p>
  *
  * @author <a href="mailto:[email protected]">James R. Perkins</a>
@@ -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<Path>() {
             @Override
             public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {

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) {

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.
  * <p>
  * 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,24 +33,23 @@ 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);
     }
 
     @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() {

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);
 }

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();
+            }
         }
     }