8383821: IllegalStateException when command executed by jpackage "Executor" timeouts

Reviewed-by: asemenyuk

Author: Alexander Matveev

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

@@ -34,19 +34,19 @@
 import java.util.Optional;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
-import jdk.jpackage.internal.util.CommandOutputControl.UnexpectedExitCodeException;
+import jdk.jpackage.internal.util.CommandOutputControl.UnexpectedResultException;
 
 public final class Codesign {
 
     public static final class CodesignException extends Exception {
 
-        CodesignException(UnexpectedExitCodeException cause) {
+        CodesignException(UnexpectedResultException cause) {
             super(Objects.requireNonNull(cause));
         }
 
         @Override
-        public UnexpectedExitCodeException getCause() {
-            return (UnexpectedExitCodeException)super.getCause();
+        public UnexpectedResultException getCause() {
+            return (UnexpectedResultException)super.getCause();
         }
 
         private static final long serialVersionUID = 1L;
@@ -97,7 +97,7 @@ public void applyTo(Path path) throws IOException, CodesignException {
 
         try {
             exec.execute().expectExitCode(0);
-        } catch (UnexpectedExitCodeException ex) {
+        } catch (UnexpectedResultException ex) {
             throw new CodesignException(ex);
         }
     }

src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/CommandOutputControl.java

@@ -913,18 +913,20 @@ public int getExitCode() {
             });
         }
 
-        public Result expectExitCode(int main, int... other) throws UnexpectedExitCodeException {
+        public Result expectExitCode(int main, int... other) throws UnexpectedResultException {
             return expectExitCode(v -> {
                 return IntStream.concat(IntStream.of(main), IntStream.of(other)).boxed().anyMatch(Predicate.isEqual(v));
             });
         }
 
-        public Result expectExitCode(Collection<Integer> expected) throws UnexpectedExitCodeException {
+        public Result expectExitCode(Collection<Integer> expected) throws UnexpectedResultException {
             return expectExitCode(expected::contains);
         }
 
-        public Result expectExitCode(IntPredicate expected) throws UnexpectedExitCodeException {
-            if (!expected.test(getExitCode())) {
+        public Result expectExitCode(IntPredicate expected) throws UnexpectedResultException {
+            if (!expected.test(exitCode.orElseThrow(() -> {
+                return new UnavailableExitCodeException(this);
+            }))) {
                 throw new UnexpectedExitCodeException(this);
             }
             return this;
@@ -1089,6 +1091,23 @@ private static Result requireExitCode(Result v) {
         private static final long serialVersionUID = 1L;
     }
 
+    public static final class UnavailableExitCodeException extends UnexpectedResultException {
+
+        public UnavailableExitCodeException(Result value, String message) {
+            super(value, message);
+            if (value.exitCode.isPresent()) {
+                throw new IllegalArgumentException();
+            }
+        }
+
+        public UnavailableExitCodeException(Result value) {
+            this(value, String.format("Exit code unavailable from executing the command %s",
+                    value.execAttrs().printableCommandLine()));
+        }
+
+        private static final long serialVersionUID = 1L;
+    }
+
     public String description() {
         var tokens = outputStreamsControl.descriptionTokens();
         if (isBinaryOutput()) {

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

@@ -42,6 +42,7 @@
 import java.util.stream.Stream;
 import jdk.jpackage.internal.util.CommandLineFormat;
 import jdk.jpackage.internal.util.CommandOutputControl;
+import jdk.jpackage.internal.util.CommandOutputControl.UnexpectedResultException;
 import jdk.jpackage.internal.util.CommandOutputControl.UnexpectedExitCodeException;
 import jdk.jpackage.internal.util.RetryExecutor;
 import jdk.jpackage.internal.util.function.ExceptionBox;
@@ -371,7 +372,15 @@ public RetryExecutor<Result, UnexpectedExitCodeException> retryUntilExitCodeIs(
             int mainExpectedExitCode, int... otherExpectedExitCodes) {
         return new RetryExecutor<Result, UnexpectedExitCodeException>(UnexpectedExitCodeException.class).setExecutable(() -> {
             var result = executeWithoutExitCodeCheck();
-            result.base().expectExitCode(mainExpectedExitCode, otherExpectedExitCodes);
+            try {
+                result.base().expectExitCode(mainExpectedExitCode, otherExpectedExitCodes);
+            } catch (UnexpectedResultException ex) {
+                if (ex instanceof UnexpectedExitCodeException uecex) {
+                    throw uecex; // Pass to exception mapper
+                }
+                // Unreachable, because the result must always have the exit code, as the executor never runs commands with a timeout.
+                throw ExceptionBox.reachedUnreachable();
+            }
             return result;
         }).setExceptionMapper((UnexpectedExitCodeException ex) -> {
             createResult(ex.getResult()).assertExitCodeIs(mainExpectedExitCode, otherExpectedExitCodes);

test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util/CommandOutputControlTest.java

@@ -261,6 +261,20 @@ public void test_Result_expectExitCode_negative(boolean collection) {
         assertEquals("Unexpected exit code 3 from executing the command <unknown>", ex.getMessage());
     }
 
+    @Test
+    public void test_Result_expectExitCode_unavailable() {
+        var result = CommandOutputControl.Result.build().noExitCode().create();
+
+        var ex = assertThrowsExactly(CommandOutputControl.UnavailableExitCodeException.class, () -> {
+            result.expectExitCode(0);
+        });
+
+        assertNull(ex.getCause());
+        assertSame(result, ex.getResult());
+        assertEquals(String.format("Exit code unavailable from executing the command %s",
+                result.execAttrs().printableCommandLine()), ex.getMessage());
+    }
+
     @ParameterizedTest
     @MethodSource
     public void test_Result_toCharacterResult(ToCharacterResultTestSpec spec) throws IOException, InterruptedException {
@@ -352,6 +366,13 @@ public void test_timeout_expires(ExecutableType mode) throws InterruptedExceptio
         var getExitCodeEx = assertThrowsExactly(IllegalStateException.class, result::getExitCode);
         assertEquals(("Exit code is unavailable for timed-out command"), getExitCodeEx.getMessage());
 
+        // Verify UnavailableExitCodeException
+        var expectExitCodeEx = assertThrowsExactly(CommandOutputControl.UnavailableExitCodeException.class, () -> {
+            result.expectExitCode(0);
+        });
+        assertEquals(String.format("Exit code unavailable from executing the command %s",
+                result.execAttrs().printableCommandLine()), expectExitCodeEx.getMessage());
+
         // We want to check that the saved output contains only the text emitted before the "sleep" action.
         // It works for a subprocess, but in the case of a ToolProvider, sometimes the timing is such
         // that it gets interrupted before having written anything to the stdout, and the saved output is empty.