This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new b1133ad Fix BZ 64080 - graceful close
b1133ad is described below
commit b1133ada73a48b738793de1827ae4ca9735babb3
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Nov 19 11:06:55 2020 +0000
Fix BZ 64080 - graceful close
https://bz.apache.org/bugzilla/show_bug.cgi?id=64080
---
java/org/apache/catalina/core/StandardService.java | 35 ++++++++++++--
java/org/apache/coyote/AbstractProtocol.java | 8 ++++
java/org/apache/coyote/LocalStrings.properties | 1 +
java/org/apache/coyote/ProtocolHandler.java | 13 +++++
.../apache/tomcat/util/net/AbstractEndpoint.java | 56 +++++++++++++++++++++-
webapps/docs/changelog.xml | 7 +++
webapps/docs/config/service.xml | 10 ++++
7 files changed, 123 insertions(+), 7 deletions(-)
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 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
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,21 +466,33 @@ public class StandardService extends LifecycleMBeanBase
implements Service {
@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.pause();
- // Close server socket if bound on start
- // Note: test is in AbstractEndpoint
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();
+ }
}
if(log.isInfoEnabled())
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 c226b72..089a07c 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -734,6 +734,14 @@ public abstract class AbstractProtocol<S> implements
ProtocolHandler,
}
+ @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.pushrequest.notsupported=Server push
requests are not supporte
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 34bd47f..0b884d3 100644
--- a/java/org/apache/coyote/ProtocolHandler.java
+++ b/java/org/apache/coyote/ProtocolHandler.java
@@ -136,6 +136,19 @@ public interface ProtocolHandler {
/**
+ * 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 fdb172d..ef4618b 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -135,7 +135,20 @@ public abstract class AbstractEndpoint<S,U> {
}
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;
+ }
}
@@ -740,7 +753,12 @@ public abstract class AbstractEndpoint<S,U> {
*/
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;
@@ -1331,6 +1349,16 @@ public abstract class AbstractEndpoint<S,U> {
*/
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();
@@ -1342,6 +1370,30 @@ public abstract class AbstractEndpoint<S,U> {
/**
+ * 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 398046c..cb963b6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -106,6 +106,13 @@
<section name="Tomcat 9.0.41 (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>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
