Fix for WFCORE-7097, Intermittent bootable jar test failures of PermissionsDeploymentTestCase.testWithConfiguredMaxBootThreads

Author: jfdenise

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

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

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