Issue #7344 - wait for forked jetty process (#7374)

* Issue #7344 Make plugin wait for forked jetty process to stop

Signed-off-by: Jan Bartel <janb@webtide.com>

jetty-maven-plugin/src/main/java/org/eclipse/jetty/maven/plugin/JettyStopMojo.java

@@ -19,13 +19,16 @@ import java.io.OutputStream;
 import java.net.ConnectException;
 import java.net.InetAddress;
 import java.net.Socket;
+import java.util.Optional;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugins.annotations.Mojo;
 import org.apache.maven.plugins.annotations.Parameter;
 
-
 /**
  * This goal stops a running instance of jetty.
  *
@@ -72,38 +75,146 @@ public class JettyStopMojo extends AbstractWebAppMojo
 
         String command = "forcestop";
 
-        try (Socket s = new Socket(InetAddress.getByName("127.0.0.1"), stopPort);)
+        if (stopWait > 0)
         {
-            OutputStream out = s.getOutputStream();
+            //try to get the pid of the forked jetty process
-            out.write((stopKey + "\r\n" + command + "\r\n").getBytes());
+            Long pid = null;
-            out.flush();
+            try
-
-            if (stopWait > 0)
             {
-                s.setSoTimeout(stopWait * 1000);
+                String response = send(stopKey + "\r\n" + "pid" + "\r\n", stopWait);
-                s.getInputStream();
+                pid = Long.valueOf(response);
+            }
+            catch (NumberFormatException e)
+            {
+                getLog().info("Server returned bad pid");
+            }
+            catch (ConnectException e)
+            {
+                //jetty not running, no point continuing
+                getLog().info("Jetty not running!");
+                return;
+            }
+            catch (Exception e)
+            {
+                //jetty running, try to stop it regardless of error
+                getLog().error(e);
+            }
 
-                getLog().info("Waiting " + stopWait + " seconds for jetty to stop");
+            //now send the stop command and wait for confirmation - either an ack from jetty, or
-                LineNumberReader lin = new LineNumberReader(new InputStreamReader(s.getInputStream()));
+            //that the process has stopped
-                String response;
+            if (pid == null)
-                boolean stopped = false;
+            {
-                while (!stopped && ((response = lin.readLine()) != null))
+                //no pid, so just wait until jetty reports itself stopped
+                try
                 {
+                    getLog().info("Waiting " + stopWait + " seconds for jetty to stop");
+                    String response = send(stopKey + "\r\n" + command + "\r\n", stopWait);
+
                     if ("Stopped".equals(response))
-                    {
-                        stopped = true;
                         getLog().info("Server reports itself as stopped");
-                    }
+                    else
+                        getLog().info("Couldn't verify server as stopped, received " + response);
+                }
+                catch (ConnectException e)
+                {
+                    getLog().info("Jetty not running!");
+                }
+                catch (Exception e)
+                {
+                    getLog().error(e);
                 }
             }
+            else
+            {
+                //wait for pid to stop
+                getLog().info("Waiting " + stopWait + " seconds for jetty " + pid + " to stop");
+                Optional<ProcessHandle> optional = ProcessHandle.of(pid);
+                optional.ifPresentOrElse(p -> 
+                {
+                    try
+                    {
+                        send(stopKey + "\r\n" + command + "\r\n", 0);
+                        CompletableFuture<ProcessHandle> future = p.onExit();
+                        if (p.isAlive())
+                        {
+                            p = future.get(stopWait, TimeUnit.SECONDS);
+                        }
+
+                        if (p.isAlive())
+                            getLog().info("Couldn't verify server process stop");
+                        else
+                            getLog().info("Server process stopped");
+                    }
+                    catch (ConnectException e)
+                    {
+                        //jetty not listening on the given port, don't wait for the process
+                        getLog().info("Jetty not running!");
+                    }
+                    catch (TimeoutException e)
+                    {
+                        getLog().error("Timeout expired while waiting for server process to stop");
+                    }
+                    catch (Throwable e)
+                    {
+                        getLog().error(e);
+                    }
+                }, () -> getLog().info("Process not running"));
+            }
         }
-        catch (ConnectException e)
+        else
         {
-            getLog().info("Jetty not running!");
+            //send the stop command but don't wait to verify the stop
+            getLog().info("Stopping jetty");
+            try
+            {
+                send(stopKey + "\r\n" + command + "\r\n", 0);
+            }
+            catch (ConnectException e)
+            {
+                getLog().info("Jetty not running!");
+            }
+            catch (Exception e)
+            {
+                getLog().error(e);
+            }
         }
-        catch (Exception e)
+    }
+
+    /**
+     * Send a command to a jetty process, optionally waiting for a response.
+     * 
+     * @param command the command to send
+     * @param wait length of time in sec to wait for a response
+     * @return the response, if any, to the command
+     * @throws Exception
+     */
+    private String send(String command, int wait)
+        throws Exception
+    {
+        String response = null;
+        try (Socket s = new Socket(InetAddress.getByName("127.0.0.1"), stopPort); OutputStream out = s.getOutputStream();)
         {
-            getLog().error(e);
+            out.write(command.getBytes());
+            out.flush();
+
+            if (wait > 0)
+            {
+                //Wait for a response
+                s.setSoTimeout(wait * 1000);
+
+                try (LineNumberReader lin = new LineNumberReader(new InputStreamReader(s.getInputStream()));)
+                {
+                    response = lin.readLine();
+                }
+            }
+            else
+            {
+                //Wait only a small amount of time to ensure TCP has sent the message
+                s.setSoTimeout(1000);
+                s.getInputStream().read();
+            }
+            
+            return response;
         }
     }
 }

jetty-maven-plugin/src/test/java/org/eclipse/jetty/maven/plugin/MockShutdownMonitor.java

@@ -0,0 +1,74 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.maven.plugin;
+
+import java.net.ServerSocket;
+
+import org.eclipse.jetty.toolchain.test.IO;
+
+/**
+ * MockShutdownMonitor
+ * A helper class that grabs a ServerSocket, spawns a thread and then
+ * passes the ServerSocket to the Runnable. This class has a main so
+ * that it can be used for forking, to mimic the actions of the
+ * org.eclipse.jetty.server.ShutdownMonitor.
+ */
+public class MockShutdownMonitor
+{
+    String key;
+    MockShutdownMonitorRunnable testerRunnable;
+    ServerSocket serverSocket;
+
+    public MockShutdownMonitor(String key, MockShutdownMonitorRunnable testerRunnable)
+        throws Exception
+    {
+        this.key = key;
+        this.testerRunnable = testerRunnable;
+        listen();
+    }
+
+    private ServerSocket listen()
+        throws Exception
+    {
+        serverSocket = new ServerSocket(0);
+        try
+        {
+            serverSocket.setReuseAddress(true);
+            return serverSocket;
+        }
+        catch (Throwable e)
+        {
+            IO.close(serverSocket);
+            throw e;
+        }
+    }
+    
+    public int getPort()
+    {
+        if (serverSocket == null)
+            return 0;
+        return serverSocket.getLocalPort();
+    }
+
+    public void start()
+        throws Exception
+    {
+        testerRunnable.setServerSocket(serverSocket);
+        testerRunnable.setKey(key);
+        Thread thread = new Thread(testerRunnable);
+        thread.setDaemon(true);
+        thread.setName("Tester Thread");
+        thread.start();
+    }
+}

jetty-maven-plugin/src/test/java/org/eclipse/jetty/maven/plugin/MockShutdownMonitorRunnable.java

@@ -0,0 +1,111 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.maven.plugin;
+
+import java.io.InputStreamReader;
+import java.io.LineNumberReader;
+import java.io.OutputStream;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+
+import org.eclipse.jetty.toolchain.test.IO;
+
+/**
+ * MockShutdownMonitorRunnable
+ * 
+ * Mimics the actions of the org.eclipse.jetty.server.ShutdownMonitor.ShutdownMonitorRunnable
+ * to aid testing.
+ */
+public class MockShutdownMonitorRunnable implements Runnable
+{
+    ServerSocket serverSocket;
+    String key;
+    String statusResponse = "OK";
+    String pidResponse;
+    String defaultResponse = "Stopped";
+    boolean exit;
+    
+    public void setExit(boolean exit)
+    {
+        this.exit = exit;
+    }
+
+    public void setKey(String key)
+    {
+        this.key = key;
+    }
+    
+    public void setServerSocket(ServerSocket serverSocket)
+    {
+        this.serverSocket = serverSocket;
+    }
+
+    public void setPidResponse(String pidResponse)
+    {
+        this.pidResponse = pidResponse;
+    }
+
+    public void run()
+    {
+        try
+        {
+            while (true)
+            {
+                try (Socket socket = serverSocket.accept())
+                {
+                    LineNumberReader reader = new LineNumberReader(new InputStreamReader(socket.getInputStream()));
+                    String receivedKey = reader.readLine();
+                    if (!key.equals(receivedKey))
+                    {
+                        continue;
+                    }
+
+                    String cmd = reader.readLine();
+                    OutputStream out = socket.getOutputStream();
+
+                    if ("status".equalsIgnoreCase(cmd))
+                    {
+                        out.write((statusResponse + "\r\n").getBytes(StandardCharsets.UTF_8));
+                        out.flush();
+                    }
+                    else if ("pid".equalsIgnoreCase(cmd))
+                    { 
+                        out.write((pidResponse + "\r\n").getBytes(StandardCharsets.UTF_8));
+                        out.flush();
+                    }
+                    else
+                    {
+                        out.write((defaultResponse + "\r\n").getBytes(StandardCharsets.UTF_8));
+                        out.flush();
+                        if (exit)
+                            System.exit(0);
+                    }
+                }
+                catch (Throwable x)
+                {
+                    x.printStackTrace();
+                }
+            }
+        }
+        catch (Throwable x)
+        {
+            x.printStackTrace();
+        }
+        finally
+        {
+            IO.close(serverSocket);
+        }
+    }
+}

jetty-maven-plugin/src/test/java/org/eclipse/jetty/maven/plugin/TestJettyStopMojo.java

@@ -0,0 +1,279 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.maven.plugin;
+
+import java.io.File;
+import java.io.FileReader;
+import java.io.LineNumberReader;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.jetty.server.ShutdownMonitor;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+public class TestJettyStopMojo
+{
+    /**
+     * ShutdownMonitorMain
+     * Kick off the ShutdownMonitor and wait for it to exit.
+     */
+    public static final class ShutdownMonitorMain
+    {
+        public static void main(String[] args)
+        {
+            try
+            {
+                ShutdownMonitor monitor = ShutdownMonitor.getInstance();
+                monitor.setPort(0);
+                monitor.start();
+                monitor.await();
+            }
+            catch (Exception e)
+            {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    public static class TestLog implements org.apache.maven.plugin.logging.Log
+    {
+        List<String> sink = new ArrayList<>();
+
+        @Override
+        public boolean isDebugEnabled()
+        {
+            return true;
+        }
+
+        @Override
+        public void debug(CharSequence content)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void debug(CharSequence content, Throwable error)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void debug(Throwable error)
+        {
+        }
+
+        @Override
+        public boolean isInfoEnabled()
+        {
+            return true;
+        }
+
+        @Override
+        public void info(CharSequence content)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void info(CharSequence content, Throwable error)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void info(Throwable error)
+        {
+        }
+
+        @Override
+        public boolean isWarnEnabled()
+        {
+            return true;
+        }
+
+        @Override
+        public void warn(CharSequence content)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void warn(CharSequence content, Throwable error)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void warn(Throwable error)
+        {
+        }
+
+        @Override
+        public boolean isErrorEnabled()
+        {
+            return true;
+        }
+
+        @Override
+        public void error(CharSequence content)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void error(CharSequence content, Throwable error)
+        {
+            sink.add(content.toString());
+        }
+
+        @Override
+        public void error(Throwable error)
+        {
+        }
+
+        public void assertContains(String str)
+        {
+            assertThat(sink, Matchers.hasItem(str));
+        }
+        
+        public void dumpStdErr()
+        {
+            for (String s : sink)
+            {
+                System.err.println(s);
+            }
+        }
+    }
+
+    @Test
+    public void testStopNoWait() throws Exception
+    {
+        //send a stop message and don't wait for the reply or the process to shutdown
+        String stopKey = "foo";
+        MockShutdownMonitorRunnable runnable = new MockShutdownMonitorRunnable();
+        runnable.setPidResponse("abcd");
+        MockShutdownMonitor monitor = new MockShutdownMonitor(stopKey, runnable);
+        monitor.start();
+        
+        TestLog log = new TestLog();
+        JettyStopMojo mojo = new JettyStopMojo();
+        mojo.stopKey = stopKey;
+        mojo.stopPort = monitor.getPort();
+        mojo.setLog(log);
+        
+        mojo.execute();
+        
+        log.assertContains("Stopping jetty");
+    }
+    
+    @Test
+    public void testStopWaitBadPid() throws Exception
+    {
+        //test that even if we receive a bad pid, we still send the stop command and wait to
+        //receive acknowledgement, but we don't wait for the process to exit
+        String stopKey = "foo";
+        MockShutdownMonitorRunnable runnable = new MockShutdownMonitorRunnable();
+        runnable.setPidResponse("abcd");
+        MockShutdownMonitor monitor = new MockShutdownMonitor(stopKey, runnable);
+        monitor.start();
+        
+        TestLog log = new TestLog();
+        JettyStopMojo mojo = new JettyStopMojo();
+        mojo.stopWait = 5;
+        mojo.stopKey = stopKey;
+        mojo.stopPort = monitor.getPort();
+        mojo.setLog(log);
+
+        mojo.execute();
+        
+        log.assertContains("Server returned bad pid");
+        log.assertContains("Server reports itself as stopped");
+    }
+
+    @Test
+    public void testStopWait() throws Exception
+    {
+        //test that we will communicate with a remote process and wait for it to exit
+        String stopKey = "foo";
+        List<String> cmd = new ArrayList<>();
+        String java = "java";
+        String[] javaexes = new String[]{"java", "java.exe"};
+        File javaHomeDir = new File(System.getProperty("java.home"));
+        Path javaHomePath = javaHomeDir.toPath();
+        for (String javaexe : javaexes)
+        {
+            Path javaBinPath = javaHomePath.resolve(Paths.get("bin", javaexe));
+            if (Files.exists(javaBinPath) && !Files.isDirectory(javaBinPath))
+                java = javaBinPath.toFile().getAbsolutePath();
+        }
+
+        cmd.add(java);
+        cmd.add("-DSTOP.KEY=" + stopKey);
+        cmd.add("-DDEBUG=true");
+        cmd.add("-cp");
+        cmd.add(System.getProperty("java.class.path"));
+        cmd.add(ShutdownMonitorMain.class.getName());
+
+        ProcessBuilder command = new ProcessBuilder(cmd);
+        File file = MavenTestingUtils.getTargetFile("tester.out");
+        command.redirectOutput(file);
+        command.redirectErrorStream(true);
+        command.directory(MavenTestingUtils.getTargetDir());
+        Process fork = command.start();
+
+        Thread.sleep(500);
+        while (!file.exists() && file.length() == 0)
+        {
+            Thread.sleep(300);
+        }
+
+        String tmp = "";
+        String port = null;
+        try (LineNumberReader reader = new LineNumberReader(new FileReader(file)))
+        {
+            while (port == null && tmp != null)
+            {
+                tmp = reader.readLine();
+                if (tmp != null)
+                {
+                    if (tmp.startsWith("STOP.PORT="))
+                        port = tmp.substring(10);
+                }
+            }
+        }
+
+        assertNotNull(port);
+
+        TestLog log = new TestLog();
+        JettyStopMojo mojo = new JettyStopMojo();
+        mojo.stopWait = 5;
+        mojo.stopKey = stopKey;
+        mojo.stopPort = Integer.parseInt(port);
+        mojo.setLog(log);
+
+        mojo.execute();
+
+        log.dumpStdErr();
+        log.assertContains("Waiting " + mojo.stopWait + " seconds for jetty " + fork.pid() + " to stop");
+        log.assertContains("Server process stopped");
+    }
+}

jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java

@@ -205,7 +205,7 @@ public class ShutdownMonitor
         }
     }
 
-    protected void start() throws Exception
+    public void start() throws Exception
     {
         try (AutoLock l = _lock.lock())
         {
@@ -236,7 +236,7 @@ public class ShutdownMonitor
     }
 
     // For test purposes only.
-    void await() throws InterruptedException
+    public void await() throws InterruptedException
     {
         try (AutoLock.WithCondition l = _lock.lock())
         {
@@ -407,6 +407,10 @@ public class ShutdownMonitor
                             // Reply to client
                             informClient(out, "OK\r\n");
                         }
+                        else if ("pid".equalsIgnoreCase(cmd))
+                        {
+                            informClient(out, Long.toString(ProcessHandle.current().pid()));
+                        }
                     }
                     catch (Throwable x)
                     {

jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java

@@ -37,6 +37,36 @@ public class ShutdownMonitorTest
     {
         ShutdownMonitor.reset();
     }
+    
+    @Test
+    public void testPid() throws Exception
+    {
+        ShutdownMonitor monitor = ShutdownMonitor.getInstance();
+        monitor.setPort(0);
+        monitor.setExitVm(false);
+        monitor.start();
+        String key = monitor.getKey();
+        int port = monitor.getPort();
+
+        // Try more than once to be sure that the ServerSocket has not been closed.
+        for (int i = 0; i < 2; ++i)
+        {
+            try (Socket socket = new Socket("localhost", port))
+            {
+                OutputStream output = socket.getOutputStream();
+                String command = "pid";
+                output.write((key + "\r\n" + command + "\r\n").getBytes());
+                output.flush();
+
+                BufferedReader input = new BufferedReader(new InputStreamReader(socket.getInputStream()));
+                String reply = input.readLine();
+                String pid = String.valueOf(ProcessHandle.current().pid());
+                assertEquals(pid, reply);
+                // Socket must be closed afterwards.
+                assertNull(input.readLine());
+            }
+        }
+    }
 
     @Test
     public void testStatus() throws Exception