Fix for WFCORE-7097 and Fix for WFCORE-7098

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,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;

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

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.
@@ -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<Path>() {
@@ -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() {

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