Fix BZ 64080 - graceful close
https://bz.apache.org/bugzilla/show_bug.cgi?id=64080
diff --git a/java/org/apache/catalina/core/StandardService.java b/java/org/apache/catalina/core/StandardService.java
index 05965ca..5597cb0 100644
--- a/java/org/apache/catalina/core/StandardService.java
+++ b/java/org/apache/catalina/core/StandardService.java
@@ -105,8 +105,21 @@
protected final MapperListener mapperListener = new MapperListener(this);
+ private long gracefulStopAwaitMillis = 0;
+
+
// ------------------------------------------------------------- Properties
+ public long getGracefulStopAwaitMillis() {
+ return gracefulStopAwaitMillis;
+ }
+
+
+ public void setGracefulStopAwaitMillis(long gracefulStopAwaitMillis) {
+ this.gracefulStopAwaitMillis = gracefulStopAwaitMillis;
+ }
+
+
@Override
public Mapper getMapper() {
return mapper;
@@ -453,13 +466,25 @@
@Override
protected void stopInternal() throws LifecycleException {
- // Pause connectors first
synchronized (connectorsLock) {
+ // Initiate a graceful stop for each connector
+ // This will only work if the bindOnInit==false which is not the
+ // default.
+ for (Connector connector: connectors) {
+ connector.getProtocolHandler().closeServerSocketGraceful();
+ }
+
+ // Wait for the graceful shutdown to complete
+ long waitMillis = gracefulStopAwaitMillis;
+ if (waitMillis > 0) {
+ for (Connector connector: connectors) {
+ waitMillis = connector.getProtocolHandler().awaitConnectionsClose(waitMillis);
+ }
+ }
+
+ // Pause the connectors
for (Connector connector: connectors) {
connector.pause();
- // Close server socket if bound on start
- // Note: test is in AbstractEndpoint
- connector.getProtocolHandler().closeServerSocketGraceful();
}
}
@@ -467,7 +492,7 @@
log.info(sm.getString("standardService.stop.name", this.name));
setState(LifecycleState.STOPPING);
- // Stop our defined Container second
+ // Stop our defined Container once the Connectors are all paused
if (engine != null) {
synchronized (engine) {
engine.stop();
diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java
index e6fbaed..921d78b 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -711,6 +711,14 @@
}
+ @Override
+ public long awaitConnectionsClose(long waitMillis) {
+ getLog().info(sm.getString("abstractProtocol.closeConnectionsAwait",
+ Long.valueOf(waitMillis), getName()));
+ return endpoint.awaitConnectionsClose(waitMillis);
+ }
+
+
private void logPortOffset() {
if (getPort() != getPortWithOffset()) {
getLog().info(sm.getString("abstractProtocolHandler.portOffset", getName(),
diff --git a/java/org/apache/coyote/LocalStrings.properties b/java/org/apache/coyote/LocalStrings.properties
index 3009a36..595cfb2 100644
--- a/java/org/apache/coyote/LocalStrings.properties
+++ b/java/org/apache/coyote/LocalStrings.properties
@@ -34,6 +34,7 @@
abstractProcessor.setErrorState=Error state [{0}] reported while processing request
abstractProcessor.socket.ssl=Exception getting SSL attributes
+abstractProtocol.closeConnectionsAwait=Waiting [{0}] milliseconds for existing connections to [{1}] to complete and close.
abstractProtocol.mbeanDeregistrationFailed=Failed to deregister MBean named [{0}] from MBean server [{1}]
abstractProtocol.processorRegisterError=Error registering request processor
abstractProtocol.processorUnregisterError=Error unregistering request processor
diff --git a/java/org/apache/coyote/ProtocolHandler.java b/java/org/apache/coyote/ProtocolHandler.java
index c6e1565..dfa5a25 100644
--- a/java/org/apache/coyote/ProtocolHandler.java
+++ b/java/org/apache/coyote/ProtocolHandler.java
@@ -136,6 +136,19 @@
/**
+ * Wait for the client connections to the server to close gracefully. The
+ * method will return when all of the client connections have closed or the
+ * method has been waiting for {@code waitTimeMillis}.
+ *
+ * @param waitMillis The maximum time to wait in milliseconds for the
+ * client connections to close.
+ *
+ * @return The wait time, if any remaining when the method returned
+ */
+ public long awaitConnectionsClose(long waitMillis);
+
+
+ /**
* Requires APR/native library
*
* @return <code>true</code> if this Protocol Handler requires the
diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 23a2c16..1dcdb2a 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -124,7 +124,20 @@
}
protected enum BindState {
- UNBOUND, BOUND_ON_INIT, BOUND_ON_START, SOCKET_CLOSED_ON_STOP
+ UNBOUND(false),
+ BOUND_ON_INIT(true),
+ BOUND_ON_START(true),
+ SOCKET_CLOSED_ON_STOP(false);
+
+ private final boolean bound;
+
+ private BindState(boolean bound) {
+ this.bound = bound;
+ }
+
+ public boolean isBound() {
+ return bound;
+ }
}
@@ -709,7 +722,12 @@
*/
private int maxKeepAliveRequests=100; // as in Apache HTTPD server
public int getMaxKeepAliveRequests() {
- return maxKeepAliveRequests;
+ // Disable keep-alive if the server socket is not bound
+ if (bindState.isBound()) {
+ return maxKeepAliveRequests;
+ } else {
+ return 1;
+ }
}
public void setMaxKeepAliveRequests(int maxKeepAliveRequests) {
this.maxKeepAliveRequests = maxKeepAliveRequests;
@@ -1302,6 +1320,16 @@
*/
public final void closeServerSocketGraceful() {
if (bindState == BindState.BOUND_ON_START) {
+ // Stop accepting new connections
+ acceptor.stop(-1);
+ // Release locks that may be preventing the acceptor from stopping
+ releaseConnectionLatch();
+ unlockAccept();
+ // Signal to any multiplexed protocols (HTTP/2) that they may wish
+ // to stop accepting new streams
+ getHandler().pause();
+ // Update the bindState. This has the side-effect of disabling
+ // keep-alive for any in-progress connections
bindState = BindState.SOCKET_CLOSED_ON_STOP;
try {
doCloseServerSocket();
@@ -1313,6 +1341,30 @@
/**
+ * Wait for the client connections to the server to close gracefully. The
+ * method will return when all of the client connections have closed or the
+ * method has been waiting for {@code waitTimeMillis}.
+ *
+ * @param waitMillis The maximum time to wait in milliseconds for the
+ * client connections to close.
+ *
+ * @return The wait time, if any remaining when the method returned
+ */
+ public final long awaitConnectionsClose(long waitMillis) {
+ while (waitMillis > 0 && !connections.isEmpty()) {
+ try {
+ Thread.sleep(50);
+ waitMillis -= 50;
+ } catch (InterruptedException e) {
+ Thread.interrupted();
+ waitMillis = 0;
+ }
+ }
+ return waitMillis;
+ }
+
+
+ /**
* Actually close the server socket but don't perform any other clean-up.
*
* @throws IOException If an error occurs closing the socket
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2251111..5603d10 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -106,6 +106,13 @@
<section name="Tomcat 10.0.0-M11 (markt)" rtext="in development">
<subsection name="Catalina">
<changelog>
+ <add>
+ <bug>64080</bug>: Enhance the graceful shutdown feature. Includes a new
+ option for <code>StandardService</code>,
+ <code>gracefulStopAwaitMillis</code>, that allows a time to be
+ specified to wait for client connections to complete and close before
+ the Container hierarchy is stopped. (markt)
+ </add>
<fix>
<bug>64921</bug>: Ensure that the <code>LoadBalancerDrainingValve</code>
uses the correct setting for the secure attribute for any session
diff --git a/webapps/docs/config/service.xml b/webapps/docs/config/service.xml
index bb8d94d..5fd0acd 100644
--- a/webapps/docs/config/service.xml
+++ b/webapps/docs/config/service.xml
@@ -80,6 +80,16 @@
common attributes listed above):</p>
<attributes>
+
+ <attribute name="gracefulStopAwaitMillis" required="false">
+ <p>The time to wait, in milliseconds, when stopping the Service for the
+ client connections to finish processing and close before the Service's
+ container hierarchy is stopped. The wait only applies to Connectors
+ configured with a <code>bindOnInit</code> value of <code>false</code>.
+ Any value of zero or less means there will be no wait. If not specified,
+ the default value of zero will be used.</p>
+ </attribute>
+
</attributes>
</subsection>
