Handle unexpected exit code from server process (#87098)

When running Elasticsearch in the foreground, the cli process waits
indefinitely on the server. If the server dies unexpectedly, the
ServerProcess throws an exception. However, the exit code is hidden in
the exception message. This commit changes waitFor to return the exit
code, so it can be propagated to the cli main. Note that when stopping
in a shutdown hook the exit code must be ignored because calling
System.exit from a shutdownhook results in a deadlock.

Author: rjernst

distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerCli.java

@@ -95,7 +95,10 @@ public void execute(Terminal terminal, OptionSet options, Environment env, Proce
         }
 
         // we are running in the foreground, so wait for the server to exit
-        server.waitFor();
+        int exitCode = server.waitFor();
+        if (exitCode != ExitCodes.OK) {
+            throw new UserException(exitCode, "Elasticsearch exited unexpectedly");
+        }
     }
 
     private void printVersion(Terminal terminal) {

distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcess.java

@@ -149,12 +149,9 @@ public synchronized void detach() throws IOException {
     /**
      * Waits for the subprocess to exit.
      */
-    public void waitFor() {
+    public int waitFor() {
         errorPump.drain();
-        int exitCode = nonInterruptible(jvmProcess::waitFor);
-        if (exitCode != ExitCodes.OK) {
-            throw new RuntimeException("server process exited with status code " + exitCode);
-        }
+        return nonInterruptible(jvmProcess::waitFor);
     }
 
     /**
@@ -171,7 +168,7 @@ public synchronized void stop() {
         }
 
         sendShutdownMarker();
-        waitFor();
+        waitFor(); // ignore exit code, we are already shutting down
     }
 
     private static void sendArgs(ServerArgs args, OutputStream processStdin) {

distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerCliTests.java

@@ -271,12 +271,19 @@ public void testIgnoreNullExceptionOutput() throws Exception {
         assertThat(terminal.getErrorOutput(), not(containsString("null")));
     }
 
+    public void testServerExitsNonZero() throws Exception {
+        mockServerExitCode = 140;
+        int exitCode = executeMain();
+        assertThat(exitCode, equalTo(140));
+    }
+
     interface AutoConfigMethod {
         void autoconfig(Terminal terminal, OptionSet options, Environment env, ProcessInfo processInfo) throws UserException;
     }
 
     Consumer<ServerArgs> argsValidator;
     private final MockServerProcess mockServer = new MockServerProcess();
+    int mockServerExitCode = 0;
 
     AutoConfigMethod autoConfigCallback;
     private final MockAutoConfigCli AUTO_CONFIG_CLI = new MockAutoConfigCli();
@@ -285,6 +292,7 @@ interface AutoConfigMethod {
     public void resetCommand() {
         argsValidator = null;
         autoConfigCallback = null;
+        mockServerExitCode = 0;
     }
 
     private class MockAutoConfigCli extends EnvironmentAwareCommand {
@@ -330,9 +338,10 @@ public void detach() {
         }
 
         @Override
-        public void waitFor() {
+        public int waitFor() {
             assert waitForCalled == false;
             waitForCalled = true;
+            return mockServerExitCode;
         }
 
         @Override

distribution/tools/server-cli/src/test/java/org/elasticsearch/server/cli/ServerProcessTests.java

@@ -407,8 +407,9 @@ public void testWaitFor() throws Exception {
             nonInterruptibleVoid(mainReady::await);
             server.stop();
         }).start();
-        server.waitFor();
+        int exitCode = server.waitFor();
         assertThat(process.main.isDone(), is(true));
+        assertThat(exitCode, equalTo(0));
         assertThat(terminal.getErrorOutput(), containsString("final message"));
     }
 
@@ -423,8 +424,7 @@ public void testProcessDies() throws Exception {
         };
         var server = startProcess(false, false, "");
         mainExit.countDown();
-        var e = expectThrows(RuntimeException.class, server::waitFor);
-        assertThat(e.getMessage(), equalTo("server process exited with status code -9"));
-        assertThat(terminal.getErrorOutput(), containsString("fatal message"));
+        int exitCode = server.waitFor();
+        assertThat(exitCode, equalTo(-9));
     }
 }