Skip to content

Commit

Permalink
Disable bind on init for all Tomcat connectors
Browse files Browse the repository at this point in the history
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 <moritz.halbritter@broadcom.com>
  • Loading branch information
wilkinsona and mhalbritter committed Nov 29, 2023
1 parent 62a6d38 commit 8de81cb
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,6 @@ protected void customizeConnector(Connector connector) {
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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,6 @@ protected void customizeConnector(Connector connector) {
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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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;
Expand Down Expand Up @@ -119,6 +120,8 @@ private void initialize() throws WebServerException {
}
});

disableBindOnInit();

// Start the server to trigger initialization listeners
this.tomcat.start();

Expand Down Expand Up @@ -162,12 +165,29 @@ private void addInstanceIdToEngineName() {
}

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<Service, Connector[]> consumer) {
for (Service service : this.tomcat.getServer().findServices()) {
Connector[] connectors = service.findConnectors().clone();
consumer.accept(service, connectors);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,23 @@ void tomcatProtocolHandlerCanBeCustomized() {
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<Service, Connector[]> connectorsByService = ((TomcatWebServer) this.webServer).getServiceConnectors();
Map<Service, Connector[]> 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
Expand Down

0 comments on commit 8de81cb

Please sign in to comment.