From 74c7985b1dfb2f37dc173b18a06bd60687bac856 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 1 Jun 2022 14:42:14 +1000 Subject: [PATCH 1/3] Issue #8088 Add STOP.EXIT System property to configure ShutdownMonitor.exitVM --- .../eclipse/jetty/server/ShutdownMonitor.java | 6 ++-- .../jetty/server/ShutdownMonitorTest.java | 33 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 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 4c46e291e1eb..cb665631c96c 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 @@ -90,7 +90,7 @@ public static boolean isRegistered(LifeCycle lifeCycle) private final String host; private int port; private String key; - private boolean exitVm; + private boolean exitVm = true; private boolean alive; /** @@ -107,7 +107,7 @@ private ShutdownMonitor() this.host = System.getProperty("STOP.HOST", "127.0.0.1"); this.port = Integer.getInteger("STOP.PORT", -1); this.key = System.getProperty("STOP.KEY", null); - this.exitVm = true; + this.exitVm = Boolean.getBoolean("STOP.EXIT"); } private void addLifeCycles(LifeCycle... lifeCycles) @@ -308,6 +308,8 @@ private ServerSocket listen() // establish the port and key that are in use debug("STOP.PORT=%d", port); debug("STOP.KEY=%s", key); + //also show if we're exiting the jvm or not + debug("STOP.EXIT=%b", exitVm); } } 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 dfc78fb37c8b..b8e5433e5017 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 @@ -32,6 +32,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -120,6 +121,38 @@ private void testStartStop(boolean reusePort) throws Exception assertTrue(!monitor.isAlive()); } + @Test + public void testNoExitSystemProperty() throws Exception + { + System.setProperty("STOP.EXIT", "false"); + ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setPort(0); + assertFalse(monitor.isExitVm()); + monitor.start(); + + try (CloseableServer server = new CloseableServer()) + { + server.setStopAtShutdown(true); + server.start(); + + //shouldn't be registered for shutdown on jvm + assertTrue(ShutdownThread.isRegistered(server)); + assertTrue(ShutdownMonitor.isRegistered(server)); + + String key = monitor.getKey(); + int port = monitor.getPort(); + + stop("stop", port, key, true); + monitor.await(); + + assertTrue(!monitor.isAlive()); + assertTrue(server.stopped); + assertTrue(!server.destroyed); + assertTrue(!ShutdownThread.isRegistered(server)); + assertTrue(!ShutdownMonitor.isRegistered(server)); + } + } + @Test public void testForceStopCommand() throws Exception { From 7339978ffee7e2b147ed0475e24bb93b73a5d509 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 2 Jun 2022 14:14:18 +1000 Subject: [PATCH 2/3] Ensure missing STOP.EXIT doesn't override default exitVm=true --- .../eclipse/jetty/server/ShutdownMonitor.java | 4 +- .../jetty/server/ShutdownMonitorTest.java | 41 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) 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 cb665631c96c..fcb1bd453e00 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 @@ -35,6 +35,7 @@ import java.util.function.Predicate; import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.thread.ShutdownThread; @@ -107,7 +108,8 @@ private ShutdownMonitor() this.host = System.getProperty("STOP.HOST", "127.0.0.1"); this.port = Integer.getInteger("STOP.PORT", -1); this.key = System.getProperty("STOP.KEY", null); - this.exitVm = Boolean.getBoolean("STOP.EXIT"); + //only change the default exitVm setting if STOP.EXIT is explicitly set + this.exitVm = Boolean.valueOf(System.getProperty("STOP.EXIT", "true")); } private void addLifeCycles(LifeCycle... lifeCycles) 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 b8e5433e5017..ddb50c70c7d3 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 @@ -152,7 +152,48 @@ public void testNoExitSystemProperty() throws Exception assertTrue(!ShutdownMonitor.isRegistered(server)); } } + + @Test + public void testExitVmDefault() throws Exception + { + //Test that the default is to exit + ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setPort(0); + assertTrue(monitor.isExitVm()); + } + /* + * Disable these config tests because ShutdownMonitor is a + * static singleton that cannot be unset, and thus would + * need each of these methods executed it its own jvm - + * current surefire settings only fork for a single test + * class. + * + * Undisable to test individually as needed. + */ + @Disabled + @Test + public void testExitVmTrue() throws Exception + { + //Test setting exit true + System.setProperty("STOP.EXIT", "true"); + ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setPort(0); + assertTrue(monitor.isExitVm()); + } + + @Disabled + @Test + public void testExitVmFalse() throws Exception + { + //Test setting exit false + System.setProperty("STOP.EXIT", "false"); + ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setPort(0); + assertFalse(monitor.isExitVm()); + } + + @Disabled @Test public void testForceStopCommand() throws Exception { From ad710ab3d58405e0c1b67b9734cded7c731a9c1d Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 2 Jun 2022 16:26:46 +1000 Subject: [PATCH 3/3] Disable another test --- .../jetty/server/ShutdownMonitorTest.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) 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 ddb50c70c7d3..2a6047b91e35 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 @@ -152,15 +152,6 @@ public void testNoExitSystemProperty() throws Exception assertTrue(!ShutdownMonitor.isRegistered(server)); } } - - @Test - public void testExitVmDefault() throws Exception - { - //Test that the default is to exit - ShutdownMonitor monitor = ShutdownMonitor.getInstance(); - monitor.setPort(0); - assertTrue(monitor.isExitVm()); - } /* * Disable these config tests because ShutdownMonitor is a @@ -171,6 +162,16 @@ public void testExitVmDefault() throws Exception * * Undisable to test individually as needed. */ + @Disabled + @Test + public void testExitVmDefault() throws Exception + { + //Test that the default is to exit + ShutdownMonitor monitor = ShutdownMonitor.getInstance(); + monitor.setPort(0); + assertTrue(monitor.isExitVm()); + } + @Disabled @Test public void testExitVmTrue() throws Exception