8259570: (macos) tools/jpackage tests fails with 'hdiutil: couldn't eject "disk2" - Resource busy'

Reviewed-by: herrick, asemenyuk

src/jdk.jpackage/macosx/classes/jdk/jpackage/internal/MacDmgBundler.java

@@ -437,7 +437,6 @@ public class MacDmgBundler extends MacBaseInstallerBundler {
             pb = new ProcessBuilder(
                     hdiutil,
                     "detach",
-                    "-force",
                     hdiUtilVerbosityFlag,
                     mountedRoot.toAbsolutePath().toString());
             // "hdiutil detach" might not work right away due to resource busy error, so
@@ -451,12 +450,21 @@ public class MacDmgBundler extends MacBaseInstallerBundler {
                 }
             });
             try {
-                // 10 times with 3 second delays.
+                // 10 times with 6 second delays.
-                retryExecutor.setMaxAttemptsCount(10).setAttemptTimeoutMillis(3000)
+                retryExecutor.setMaxAttemptsCount(10).setAttemptTimeoutMillis(6000)
                         .execute(pb);
             } catch (IOException ex) {
                 if (!retryExecutor.isAborted()) {
-                    throw ex;
+                    // Now force to detach if it still attached
+                    if (Files.exists(mountedRoot)) {
+                        pb = new ProcessBuilder(
+                                hdiutil,
+                                "detach",
+                                "-force",
+                                hdiUtilVerbosityFlag,
+                                mountedRoot.toAbsolutePath().toString());
+                        IOUtils.exec(pb);
+                    }
                 }
             }
         }
@@ -469,10 +477,29 @@ public class MacDmgBundler extends MacBaseInstallerBundler {
                 hdiUtilVerbosityFlag,
                 "-format", "UDZO",
                 "-o", finalDMG.toAbsolutePath().toString());
-        new RetryExecutor()
+        try {
+            new RetryExecutor()
                 .setMaxAttemptsCount(10)
                 .setAttemptTimeoutMillis(3000)
                 .execute(pb);
+        } catch (Exception ex) {
+            // Convert might failed if something holds file. Try to convert copy.
+            Path protoDMG2 = imagesRoot
+                    .resolve(APP_NAME.fetchFrom(params) + "-tmp2.dmg");
+            Files.copy(protoDMG, protoDMG2);
+            try {
+                pb = new ProcessBuilder(
+                        hdiutil,
+                        "convert",
+                        protoDMG2.toAbsolutePath().toString(),
+                        hdiUtilVerbosityFlag,
+                        "-format", "UDZO",
+                        "-o", finalDMG.toAbsolutePath().toString());
+                IOUtils.exec(pb);
+            } finally {
+                Files.deleteIfExists(protoDMG2);
+            }
+        }
 
         //add license if needed
         if (Files.exists(getConfig_LicenseFile(params))) {

src/jdk.jpackage/share/classes/jdk/jpackage/internal/RetryExecutor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -29,31 +29,31 @@ import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 public final class RetryExecutor {
-    RetryExecutor() {
+    public RetryExecutor() {
         setMaxAttemptsCount(5);
         setAttemptTimeoutMillis(2 * 1000);
     }
 
-    RetryExecutor setMaxAttemptsCount(int v) {
+    public RetryExecutor setMaxAttemptsCount(int v) {
         attempts = v;
         return this;
     }
 
-    RetryExecutor setAttemptTimeoutMillis(int v) {
+    public RetryExecutor setAttemptTimeoutMillis(int v) {
         timeoutMillis = v;
         return this;
     }
 
-    RetryExecutor setExecutorInitializer(Consumer<Executor> v) {
+    public RetryExecutor setExecutorInitializer(Consumer<Executor> v) {
         executorInitializer = v;
         return this;
     }
 
-    void abort() {
+    public void abort() {
         aborted = true;
     }
 
-    boolean isAborted() {
+    public boolean isAborted() {
         return aborted;
     }
 
@@ -68,11 +68,11 @@ public final class RetryExecutor {
         });
     }
 
-    void execute(String cmdline[]) throws IOException {
+    public void execute(String cmdline[]) throws IOException {
         executeLoop(() -> Executor.of(cmdline));
     }
 
-    void execute(ProcessBuilder pb) throws IOException {
+    public void execute(ProcessBuilder pb) throws IOException {
         executeLoop(() -> Executor.of(pb));
     }
 

test/jdk/tools/jpackage/helpers/jdk/jpackage/test/MacHelper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -42,6 +42,7 @@ import javax.xml.xpath.XPathFactory;
 import jdk.jpackage.test.Functional.ThrowingConsumer;
 import jdk.jpackage.test.Functional.ThrowingSupplier;
 import jdk.jpackage.test.PackageTest.PackageHandlers;
+import jdk.jpackage.internal.RetryExecutor;
 import org.xml.sax.SAXException;
 import org.w3c.dom.NodeList;
 
@@ -66,11 +67,36 @@ public class MacHelper {
                     cmd.outputBundle(), dmgImage));
             ThrowingConsumer.toConsumer(consumer).accept(dmgImage);
         } finally {
-            // detach might not work right away due to resource busy error, so
+            String cmdline[] = {
-            // repeat detach several times or fail. Try 10 times with 3 seconds
+                "/usr/bin/hdiutil",
-            // delay.
+                "detach",
-            Executor.of("/usr/bin/hdiutil", "detach").addArgument(mountPoint).
+                "-verbose",
-                    executeAndRepeatUntilExitCode(0, 10, 3);
+                mountPoint.toAbsolutePath().toString()};
+            // "hdiutil detach" might not work right away due to resource busy error, so
+            // repeat detach several times.
+            RetryExecutor retryExecutor = new RetryExecutor();
+            // Image can get detach even if we got resource busy error, so stop
+            // trying to detach it if it is no longer attached.
+            retryExecutor.setExecutorInitializer(exec -> {
+                if (!Files.exists(mountPoint)) {
+                    retryExecutor.abort();
+                }
+            });
+            try {
+                // 10 times with 6 second delays.
+                retryExecutor.setMaxAttemptsCount(10)
+                        .setAttemptTimeoutMillis(6000)
+                        .execute(cmdline);
+            } catch (IOException ex) {
+                if (!retryExecutor.isAborted()) {
+                    // Now force to detach if it still attached
+                    if (Files.exists(mountPoint)) {
+                        Executor.of("/usr/bin/hdiutil", "detach",
+                                    "-force", "-verbose")
+                                 .addArgument(mountPoint).execute();
+                    }
+                }
+            }
         }
     }