From d36fbd0e783fbeb8f1b2913e201e3873c9cdcebf Mon Sep 17 00:00:00 2001 From: markt Date: Thu, 9 Dec 2010 19:49:24 +0000 Subject: [PATCH] Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50360 Add an option to control when the socket is bound git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1044110 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/tomcat/util/net/AbstractEndpoint.java | 67 +++++++++++++++++++--- java/org/apache/tomcat/util/net/AprEndpoint.java | 23 ++------ java/org/apache/tomcat/util/net/JIoEndpoint.java | 19 ++---- java/org/apache/tomcat/util/net/NioEndpoint.java | 21 ++----- .../apache/tomcat/util/net/TestXxxEndpoint.java | 54 ++++++++++++++--- webapps/docs/changelog.xml | 10 ++++ webapps/docs/config/http.xml | 7 +++ 7 files changed, 137 insertions(+), 64 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java index 074489133..e1c48b3e6 100644 --- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java +++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java @@ -97,6 +97,10 @@ public abstract class AbstractEndpoint { public void recycle(); } + protected enum BindState { + UNBOUND, BOUND_ON_INIT, BOUND_ON_START + } + // Standard SSL Configuration attributes // JSSE // Standard configuration attribute names @@ -143,11 +147,6 @@ public abstract class AbstractEndpoint { protected volatile boolean paused = false; /** - * Track the initialization state of the endpoint. - */ - protected boolean initialized = false; - - /** * Are we using an internal executor */ protected volatile boolean internalExecutor = false; @@ -202,6 +201,17 @@ public abstract class AbstractEndpoint { public int getBacklog() { return backlog; } /** + * Controls when the Endpoint binds the port. true, the default + * binds the port on {@link #init()} and unbinds it on {@link #destroy()}. + * If set to false the port is bound on {@link #start()} and + * unbound on {@link #stop()}. + */ + private boolean bindOnInit = true; + public boolean getBindOnInit() { return bindOnInit; } + public void setBindOnInit(boolean b) { this.bindOnInit = b; } + private BindState bindState = BindState.UNBOUND; + + /** * Keepalive timeout, if lesser or equal to 0 then soTimeout will be used. */ private int keepAliveTimeout = -1; @@ -503,8 +513,34 @@ public abstract class AbstractEndpoint { } - public abstract void init() throws Exception; - public abstract void start() throws Exception; + // ------------------------------------------------------- Lifecycle methods + + /* + * NOTE: There is no maintenance of state or checking for valid transitions + * within this class other than ensuring that bind/unbind are called in the + * right place. It is expected that the calling code will maintain state and + * prevent invalid state transitions. + */ + + public abstract void bind() throws Exception; + public abstract void unbind() throws Exception; + public abstract void startInternal() throws Exception; + public abstract void stopInternal() throws Exception; + + public final void init() throws Exception { + if (bindOnInit) { + bind(); + bindState = BindState.BOUND_ON_INIT; + } + } + + public final void start() throws Exception { + if (bindState == BindState.UNBOUND) { + bind(); + bindState = BindState.BOUND_ON_START; + } + startInternal(); + } /** * Pause the endpoint, which will stop it accepting new connections. @@ -532,8 +568,21 @@ public abstract class AbstractEndpoint { } } - public abstract void stop() throws Exception; - public abstract void destroy() throws Exception; + public final void stop() throws Exception { + stopInternal(); + if (bindState == BindState.BOUND_ON_START) { + unbind(); + bindState = BindState.UNBOUND; + } + } + + public final void destroy() throws Exception { + if (bindState == BindState.BOUND_ON_INIT) { + unbind(); + bindState = BindState.UNBOUND; + } + } + public String adjustRelativePath(String path, String relativeTo) { String newPath = path; diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 994cc886b..393b44eb2 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -365,11 +365,7 @@ public class AprEndpoint extends AbstractEndpoint { * Initialize the endpoint. */ @Override - public void init() - throws Exception { - - if (initialized) - return; + public void bind() throws Exception { // Create the root APR memory pool try { @@ -518,9 +514,6 @@ public class AprEndpoint extends AbstractEndpoint { // For now, sendfile is not supported with SSL useSendfile = false; } - - initialized = true; - } @@ -528,12 +521,8 @@ public class AprEndpoint extends AbstractEndpoint { * Start the APR endpoint, creating acceptor, poller and sendfile threads. */ @Override - public void start() - throws Exception { - // Initialize socket if not done before - if (!initialized) { - init(); - } + public void startInternal() throws Exception { + if (!running) { running = true; paused = false; @@ -602,7 +591,7 @@ public class AprEndpoint extends AbstractEndpoint { * Stop the endpoint. This will cause all processing threads to stop. */ @Override - public void stop() { + public void stopInternal() { if (!paused) { pause(); } @@ -665,7 +654,7 @@ public class AprEndpoint extends AbstractEndpoint { * Deallocate APR memory pools, and close server socket. */ @Override - public void destroy() throws Exception { + public void unbind() throws Exception { if (running) { stop(); } @@ -691,8 +680,6 @@ public class AprEndpoint extends AbstractEndpoint { } handler.recycle(); - - initialized = false; } diff --git a/java/org/apache/tomcat/util/net/JIoEndpoint.java b/java/org/apache/tomcat/util/net/JIoEndpoint.java index 1ca3c923b..38893e613 100644 --- a/java/org/apache/tomcat/util/net/JIoEndpoint.java +++ b/java/org/apache/tomcat/util/net/JIoEndpoint.java @@ -324,12 +324,8 @@ public class JIoEndpoint extends AbstractEndpoint { // -------------------- Public methods -------------------- @Override - public void init() - throws Exception { + public void bind() throws Exception { - if (initialized) - return; - // Initialize thread count defaults for acceptor if (acceptorThreadCount == 0) { acceptorThreadCount = 1; @@ -365,15 +361,11 @@ public class JIoEndpoint extends AbstractEndpoint { } } - initialized = true; } @Override - public void start() throws Exception { - // Initialize socket if not done before - if (!initialized) { - init(); - } + public void startInternal() throws Exception { + if (!running) { running = true; paused = false; @@ -402,7 +394,7 @@ public class JIoEndpoint extends AbstractEndpoint { } @Override - public void stop() { + public void stopInternal() { if (!paused) { pause(); } @@ -417,7 +409,7 @@ public class JIoEndpoint extends AbstractEndpoint { * Deallocate APR memory pools, and close server socket. */ @Override - public void destroy() throws Exception { + public void unbind() throws Exception { if (running) { stop(); } @@ -431,7 +423,6 @@ public class JIoEndpoint extends AbstractEndpoint { serverSocket = null; } handler.recycle(); - initialized = false ; } diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 11d557ae8..c48101ec7 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -455,11 +455,7 @@ public class NioEndpoint extends AbstractEndpoint { * Initialize the endpoint. */ @Override - public void init() - throws Exception { - - if (initialized) - return; + public void bind() throws Exception { serverSock = ServerSocketChannel.open(); socketProperties.setProperties(serverSock.socket()); @@ -545,8 +541,6 @@ public class NioEndpoint extends AbstractEndpoint { if (oomParachute>0) reclaimParachute(true); selectorPool.open(); - initialized = true; - } public KeyManager[] wrap(KeyManager[] managers) { @@ -567,12 +561,8 @@ public class NioEndpoint extends AbstractEndpoint { * Start the NIO endpoint, creating acceptor, poller threads. */ @Override - public void start() - throws Exception { - // Initialize socket if not done before - if (!initialized) { - init(); - } + public void startInternal() throws Exception { + if (!running) { running = true; paused = false; @@ -607,7 +597,7 @@ public class NioEndpoint extends AbstractEndpoint { * Stop the endpoint. This will cause all processing threads to stop. */ @Override - public void stop() { + public void stopInternal() { if (!paused) { pause(); } @@ -634,7 +624,7 @@ public class NioEndpoint extends AbstractEndpoint { * Deallocate NIO memory pools, and close server socket. */ @Override - public void destroy() throws Exception { + public void unbind() throws Exception { if (log.isDebugEnabled()) { log.debug("Destroy initiated for "+new InetSocketAddress(getAddress(),getPort())); } @@ -646,7 +636,6 @@ public class NioEndpoint extends AbstractEndpoint { serverSock.close(); serverSock = null; sslContext = null; - initialized = false; releaseCaches(); selectorPool.close(); if (log.isDebugEnabled()) { diff --git a/test/org/apache/tomcat/util/net/TestXxxEndpoint.java b/test/org/apache/tomcat/util/net/TestXxxEndpoint.java index ab3c59503..d09632b1e 100644 --- a/test/org/apache/tomcat/util/net/TestXxxEndpoint.java +++ b/test/org/apache/tomcat/util/net/TestXxxEndpoint.java @@ -20,6 +20,7 @@ package org.apache.tomcat.util.net; import java.io.File; import java.net.ServerSocket; +import org.apache.catalina.connector.Connector; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; @@ -29,7 +30,7 @@ import org.apache.catalina.startup.TomcatBaseTest; */ public class TestXxxEndpoint extends TomcatBaseTest { - public void disbaletestStartStop() throws Exception { + public void testStartStopBindOnInit() throws Exception { Tomcat tomcat = getTomcatInstance(); File appDir = new File(getBuildDirectory(), "webapps/examples"); tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath()); @@ -39,13 +40,52 @@ public class TestXxxEndpoint extends TomcatBaseTest { tomcat.start(); tomcat.getConnector().stop(); - // This will throw an exception if the socket is not released - ServerSocket socket = new ServerSocket(port); - socket.close(); + // This will should throw an Exception + Exception e = null; + ServerSocket s = null; + try { + s = new ServerSocket(port); + } catch (Exception e1) { + e = e1; + } finally { + if (s != null) { + try { + s.close(); + } catch (Exception e2) { /* Ignore */ } + } + } + assertNotNull(e); tomcat.getConnector().start(); } - - public void testDummy() throws Exception { - // NO-OP + + public void testStartStopBindOnStart() throws Exception { + Tomcat tomcat = getTomcatInstance(); + Connector c = tomcat.getConnector(); + c.setProperty("bindOnInit", "false"); + + File appDir = new File(getBuildDirectory(), "webapps/examples"); + tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath()); + + int port = getPort(); + + tomcat.start(); + + tomcat.getConnector().stop(); + // This should not throw an Exception + Exception e = null; + ServerSocket s = null; + try { + s = new ServerSocket(port); + } catch (Exception e1) { + e = e1; + } finally { + if (s != null) { + try { + s.close(); + } catch (Exception e2) { /* Ignore */ } + } + } + assertNull(e); + tomcat.getConnector().start(); } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 957e0df71..4dbeeb14f 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -130,6 +130,16 @@ 50108: Add get/set methods for Connector property minSpareThreads. Patch provided by Eiji Takahashi. (markt) + + 50360: Provide an option to control when the socket + associated with a connector is bound. By default, the socket is bound on + Connector.init() and released on + Connector.destroy() as per the current behaviour but this + can be changed so that the socket is bound on + Connector.start() and released on + Connector.stop(). This fix also includes further Lifecycle + refactoring for Connectors and associated components. (markt) + diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml index d6ee33651..058dc0505 100644 --- a/webapps/docs/config/http.xml +++ b/webapps/docs/config/http.xml @@ -265,6 +265,13 @@ associated with the server.

+ +

Controls when the socket used by the connector is bound. By default it + is bound when the connector is initiated and unbund when the connector is + destroyed. If set to false, the socket will be bound when the + connector is started and unbound when it is stopped.

+ +

The value is a comma separated list of MIME types for which HTTP compression may be used. -- 2.11.0