From 8de81cb06e7039e95989f6035565f810edeaba4b Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 29 Nov 2023 14:02:43 +0000 Subject: [PATCH] Disable bind on init for all Tomcat connectors If a connector is bound on init, it won't be unbound when stop() is called. This leaves the connector running when it should have been stopped. We currently disable bind on init for the main connector but not for any additional connectors. This commit disables bind on it for all connectors unless it is been explicitly enabled through the bindOnInit property. Closes gh-38564 Co-authored-by: Moritz Halbritter --- .../TomcatReactiveWebServerFactory.java | 2 -- .../tomcat/TomcatServletWebServerFactory.java | 2 -- .../web/embedded/tomcat/TomcatWebServer.java | 24 +++++++++++++++++-- .../TomcatServletWebServerFactoryTests.java | 16 +++++++++++-- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java index ff6d8e729c..4981bb81e9 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatReactiveWebServerFactory.java @@ -200,8 +200,6 @@ public class TomcatReactiveWebServerFactory extends AbstractReactiveWebServerFac if (getUriEncoding() != null) { connector.setURIEncoding(getUriEncoding().name()); } - // Don't bind to the socket prematurely if ApplicationContext is slow to start - connector.setProperty("bindOnInit", "false"); if (getHttp2() != null && getHttp2().isEnabled()) { connector.addUpgradeProtocol(new Http2Protocol()); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java index 5c60b0c7a9..5faed28060 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactory.java @@ -342,8 +342,6 @@ public class TomcatServletWebServerFactory extends AbstractServletWebServerFacto if (getUriEncoding() != null) { connector.setURIEncoding(getUriEncoding().name()); } - // Don't bind to the socket prematurely if ApplicationContext is slow to start - connector.setProperty("bindOnInit", "false"); if (getHttp2() != null && getHttp2().isEnabled()) { connector.addUpgradeProtocol(new Http2Protocol()); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java index 7d7ce96546..6aa6a8cefb 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.BiConsumer; import java.util.stream.Collectors; import javax.naming.NamingException; @@ -119,6 +120,8 @@ public class TomcatWebServer implements WebServer { } }); + disableBindOnInit(); + // Start the server to trigger initialization listeners this.tomcat.start(); @@ -162,12 +165,29 @@ public class TomcatWebServer implements WebServer { } private void removeServiceConnectors() { - for (Service service : this.tomcat.getServer().findServices()) { - Connector[] connectors = service.findConnectors().clone(); + doWithConnectors((service, connectors) -> { this.serviceConnectors.put(service, connectors); for (Connector connector : connectors) { service.removeConnector(connector); } + }); + } + + private void disableBindOnInit() { + doWithConnectors((service, connectors) -> { + for (Connector connector : connectors) { + Object bindOnInit = connector.getProperty("bindOnInit"); + if (bindOnInit == null) { + connector.setProperty("bindOnInit", "false"); + } + } + }); + } + + private void doWithConnectors(BiConsumer consumer) { + for (Service service : this.tomcat.getServer().findServices()) { + Connector[] connectors = service.findConnectors().clone(); + consumer.accept(service, connectors); } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java index 5dafd69bc4..b9a9f58e5c 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java @@ -237,11 +237,23 @@ class TomcatServletWebServerFactoryTests extends AbstractServletWebServerFactory void tomcatAdditionalConnectors() { TomcatServletWebServerFactory factory = getFactory(); Connector[] connectors = new Connector[4]; - Arrays.setAll(connectors, (i) -> new Connector()); + Arrays.setAll(connectors, (i) -> { + Connector connector = new Connector(); + connector.setPort(0); + return connector; + }); factory.addAdditionalTomcatConnectors(connectors); this.webServer = factory.getWebServer(); - Map connectorsByService = ((TomcatWebServer) this.webServer).getServiceConnectors(); + Map connectorsByService = new HashMap<>( + ((TomcatWebServer) this.webServer).getServiceConnectors()); assertThat(connectorsByService.values().iterator().next()).hasSize(connectors.length + 1); + this.webServer.start(); + this.webServer.stop(); + connectorsByService.forEach((service, serviceConnectors) -> { + for (Connector connector : serviceConnectors) { + assertThat(connector.getProtocolHandler()).extracting("endpoint.serverSock").isNull(); + } + }); } @Test