From 9848b33ec9c7efd897d6faa89aed214f1f8feb7c Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 16 Sep 2016 11:33:21 +0200 Subject: [PATCH] Fixes #915 - The jetty-maven-plugin:stop goal doesn't stop everything completely. Closing the serverSocket when exiting ShutdownMonitorRunnable. --- .../eclipse/jetty/server/ShutdownMonitor.java | 3 +- .../jetty/server/ShutdownMonitorTest.java | 79 +++++++++++++++---- 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java index 9e418932111..378c7727000 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java @@ -34,6 +34,7 @@ import java.util.List; import java.util.Set; import java.util.function.Predicate; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.thread.ShutdownThread; @@ -257,7 +258,6 @@ public class ShutdownMonitor } } - private ServerSocket listen() { int port = getPort(); @@ -415,6 +415,7 @@ public class ShutdownMonitor } finally { + IO.close(serverSocket); stop(); debug("Stopped"); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java index f0e4e7f1f67..3bb2ee8ec0b 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ShutdownMonitorTest.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.server; +import java.io.BufferedReader; import java.io.Closeable; import java.io.InputStreamReader; import java.io.LineNumberReader; @@ -26,7 +27,8 @@ import java.net.InetAddress; import java.net.Socket; import org.eclipse.jetty.util.thread.ShutdownThread; -import org.junit.AfterClass; +import org.junit.After; +import org.junit.Assert; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -34,14 +36,55 @@ import static org.junit.Assert.assertTrue; public class ShutdownMonitorTest { - @AfterClass - public static void afterClass() + @After + public void dispose() { ShutdownMonitor.reset(); } - + @Test - public void testShutdownMonitor() throws Exception + public void testStatus() throws Exception + { + ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setDebug(true); + 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 = "status"; + output.write((key + "\r\n" + command + "\r\n").getBytes()); + output.flush(); + + BufferedReader input = new BufferedReader(new InputStreamReader(socket.getInputStream())); + String reply = input.readLine(); + assertEquals("OK", reply); + // Socket must be closed afterwards. + Assert.assertNull(input.readLine()); + } + } + } + + @Test + public void testStartStopDifferentPortDifferentKey() throws Exception + { + testStartStop(false); + } + + @Test + public void testStartStopSamePortDifferentKey() throws Exception + { + testStartStop(true); + } + + private void testStartStop(boolean reusePort) throws Exception { ShutdownMonitor monitor = ShutdownMonitor.getInstance(); monitor.setDebug(true); @@ -58,8 +101,8 @@ public class ShutdownMonitorTest monitor.await(); assertTrue(!monitor.isAlive()); - // should be able to change port and key because it is stopped - monitor.setPort(0); + // Should be able to change port and key because it is stopped. + monitor.setPort(reusePort ? port : 0); String newKey = "foo"; monitor.setKey(newKey); monitor.start(); @@ -78,8 +121,12 @@ public class ShutdownMonitorTest public void testForceStopCommand() throws Exception { ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setDebug(true); monitor.setPort(0); - try(CloseableServer server = new CloseableServer()) + monitor.setExitVm(false); + monitor.start(); + + try (CloseableServer server = new CloseableServer()) { server.start(); @@ -105,10 +152,12 @@ public class ShutdownMonitorTest public void testOldStopCommandWithStopOnShutdownTrue() throws Exception { ShutdownMonitor monitor = ShutdownMonitor.getInstance(); - monitor.setExitVm(false); - + monitor.setDebug(true); monitor.setPort(0); - try(CloseableServer server = new CloseableServer()) + monitor.setExitVm(false); + monitor.start(); + + try (CloseableServer server = new CloseableServer()) { server.setStopAtShutdown(true); server.start(); @@ -135,9 +184,12 @@ public class ShutdownMonitorTest public void testOldStopCommandWithStopOnShutdownFalse() throws Exception { ShutdownMonitor monitor = ShutdownMonitor.getInstance(); - monitor.setExitVm(false); + monitor.setDebug(true); monitor.setPort(0); - try(CloseableServer server = new CloseableServer()) + monitor.setExitVm(false); + monitor.start(); + + try (CloseableServer server = new CloseableServer()) { server.setStopAtShutdown(false); server.start(); @@ -222,7 +274,6 @@ public class ShutdownMonitorTest { throw new RuntimeException(e); } - } } }