This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new a1cbdab Fix BZ 64080 - graceful close a1cbdab is described below commit a1cbdab44b243515a5e8a9fec3c6fa1ac9ba254a Author: Mark Thomas <ma...@apache.org> 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 e6fbaed..921d78b 100644 --- a/java/org/apache/coyote/AbstractProtocol.java +++ b/java/org/apache/coyote/AbstractProtocol.java @@ -711,6 +711,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 c6e1565..dfa5a25 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 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 @@ 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; + } } @@ -709,7 +722,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; @@ -1302,6 +1320,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(); @@ -1313,6 +1341,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 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> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org