Author: markt Date: Wed Dec 1 18:37:29 2010 New Revision: 1041120 URL: http://svn.apache.org/viewvc?rev=1041120&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50360 Bind/release socket on start()/stop() Register/deregister MBeans on init()/destroy()
Added: tomcat/trunk/test/org/apache/tomcat/util/net/ tomcat/trunk/test/org/apache/tomcat/util/net/TestXxxEndpoint.java (with props) Modified: tomcat/trunk/java/org/apache/catalina/connector/Connector.java tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProtocol.java tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/catalina/connector/Connector.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Connector.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/Connector.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/Connector.java Wed Dec 1 18:37:29 2010 @@ -864,6 +864,7 @@ public class Connector extends Lifecycle // Initialize adapter adapter = new CoyoteAdapter(this); protocolHandler.setAdapter(adapter); + protocolHandler.setDomain(getDomain()); try { protocolHandler.init(); Modified: tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java Wed Dec 1 18:37:29 2010 @@ -91,4 +91,11 @@ public interface ProtocolHandler { * Destroy the protocol (optional). */ public void destroy() throws Exception; + + + /** + * Domain for registering handler with JMX. + */ + public void setDomain(String domain); + public String getDomain(); } Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProtocol.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProtocol.java Wed Dec 1 18:37:29 2010 @@ -256,6 +256,12 @@ public abstract class AbstractAjpProtoco return oname; } + @Override + public void setDomain(String domain) { + this.domain = domain; + } + + @Override public String getDomain() { return domain; } Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java Wed Dec 1 18:37:29 2010 @@ -28,6 +28,7 @@ import javax.management.ObjectName; import org.apache.coyote.Adapter; import org.apache.coyote.ProtocolHandler; +import org.apache.coyote.RequestGroupInfo; import org.apache.juli.logging.Log; import org.apache.tomcat.util.modeler.Registry; import org.apache.tomcat.util.net.AbstractEndpoint; @@ -122,7 +123,38 @@ public abstract class AbstractHttp11Prot return ("http-" + encodedAddr + endpoint.getPort()); } + @Override + public void init() throws Exception { + if (this.domain != null) { + try { + tpOname = new ObjectName + (domain + ":" + "type=ThreadPool,name=" + getName()); + Registry.getRegistry(null, null) + .registerComponent(endpoint, tpOname, null ); + } catch (Exception e) { + getLog().error("Can't register endpoint"); + } + rgOname=new ObjectName(domain + + ":type=GlobalRequestProcessor,name=" + getName()); + Registry.getRegistry(null, null).registerComponent( + getRequestGroupInfo(), rgOname, null ); + } + + endpoint.setName(getName()); + + try { + endpoint.init(); + } catch (Exception ex) { + getLog().error( + sm.getString("http11protocol.endpoint.initerror"), ex); + throw ex; + } + if (getLog().isInfoEnabled()) + getLog().info(sm.getString("http11protocol.init", getName())); + } + protected abstract RequestGroupInfo getRequestGroupInfo(); + @Override public void pause() throws Exception { try { @@ -369,9 +401,6 @@ public abstract class AbstractHttp11Prot public int getSoLinger() { return endpoint.getSoLinger(); } public void setSoLinger(int soLinger) { endpoint.setSoLinger(soLinger); } - @Override - public abstract void init() throws Exception; - // -------------------- JMX related methods -------------------- protected String domain; @@ -382,11 +411,17 @@ public abstract class AbstractHttp11Prot return oname; } + @Override public String getDomain() { return domain; } @Override + public void setDomain(String domain) { + this.domain = domain; + } + + @Override public ObjectName preRegister(MBeanServer server, ObjectName name) throws Exception { oname=name; Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProtocol.java Wed Dec 1 18:37:29 2010 @@ -73,37 +73,14 @@ public class Http11AprProtocol extends A */ @Override public void init() throws Exception { - endpoint.setName(getName()); - ((AprEndpoint)endpoint).setHandler(cHandler); - - try { - endpoint.init(); - } catch (Exception ex) { - log.error(sm.getString("http11protocol.endpoint.initerror"), ex); - throw ex; - } - if(log.isInfoEnabled()) - log.info(sm.getString("http11protocol.init", getName())); + ((AprEndpoint)endpoint).setHandler(cHandler); + + super.init(); } @Override public void start() throws Exception { - if( this.domain != null ) { - try { - tpOname=new ObjectName - (domain + ":" + "type=ThreadPool,name=" + getName()); - Registry.getRegistry(null, null) - .registerComponent(endpoint, tpOname, null ); - } catch (Exception e) { - log.error("Can't register threadpool" ); - } - rgOname=new ObjectName - (domain + ":type=GlobalRequestProcessor,name=" + getName()); - Registry.getRegistry(null, null).registerComponent - ( cHandler.global, rgOname, null ); - } - try { endpoint.start(); } catch (Exception ex) { @@ -120,6 +97,12 @@ public class Http11AprProtocol extends A super.destroy(); } + @Override + protected RequestGroupInfo getRequestGroupInfo() { + return cHandler.global; + } + + private Http11ConnectionHandler cHandler; public boolean getUseSendfile() { return ((AprEndpoint)endpoint).getUseSendfile(); } Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProtocol.java Wed Dec 1 18:37:29 2010 @@ -76,39 +76,16 @@ public class Http11NioProtocol extends A */ @Override public void init() throws Exception { - endpoint.setName(getName()); - ((NioEndpoint)endpoint).setHandler(cHandler); - try { - endpoint.init(); - sslImplementation = new JSSEImplementation(); - } catch (Exception ex) { - log.error(sm.getString("http11protocol.endpoint.initerror"), ex); - throw ex; - } - if(log.isInfoEnabled()) - log.info(sm.getString("http11protocol.init", getName())); + ((NioEndpoint)endpoint).setHandler(cHandler); + super.init(); } @Override public void start() throws Exception { - if( this.domain != null ) { - try { - tpOname=new ObjectName - (domain + ":" + "type=ThreadPool,name=" + getName()); - Registry.getRegistry(null, null) - .registerComponent(endpoint, tpOname, null ); - } catch (Exception e) { - log.error("Can't register threadpool" ); - } - rgOname=new ObjectName - (domain + ":type=GlobalRequestProcessor,name=" + getName()); - Registry.getRegistry(null, null).registerComponent - ( cHandler.global, rgOname, null ); - } - try { + sslImplementation = new JSSEImplementation(); endpoint.start(); } catch (Exception ex) { log.error(sm.getString("http11protocol.endpoint.starterror"), ex); @@ -119,6 +96,12 @@ public class Http11NioProtocol extends A } + @Override + protected RequestGroupInfo getRequestGroupInfo() { + return cHandler.global; + } + + // -------------------- Properties-------------------- Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Protocol.java Wed Dec 1 18:37:29 2010 @@ -85,9 +85,14 @@ public class Http11Protocol extends Abst @Override public void init() throws Exception { - ((JIoEndpoint)endpoint).setName(getName()); + ((JIoEndpoint)endpoint).setHandler(cHandler); + super.init(); + } + + @Override + public void start() throws Exception { // Verify the validity of the configured socket factory try { if (isSSLEnabled()) { @@ -115,34 +120,6 @@ public class Http11Protocol extends Abst } try { - endpoint.init(); - } catch (Exception ex) { - log.error(sm.getString("http11protocol.endpoint.initerror"), ex); - throw ex; - } - if (log.isInfoEnabled()) - log.info(sm.getString("http11protocol.init", getName())); - - } - - @Override - public void start() throws Exception { - if (this.domain != null) { - try { - tpOname = new ObjectName - (domain + ":" + "type=ThreadPool,name=" + getName()); - Registry.getRegistry(null, null) - .registerComponent(endpoint, tpOname, null ); - } catch (Exception e) { - log.error("Can't register endpoint"); - } - rgOname=new ObjectName - (domain + ":type=GlobalRequestProcessor,name=" + getName()); - Registry.getRegistry(null, null).registerComponent - ( cHandler.global, rgOname, null ); - } - - try { endpoint.start(); } catch (Exception ex) { log.error(sm.getString("http11protocol.endpoint.starterror"), ex); @@ -158,6 +135,14 @@ public class Http11Protocol extends Abst cHandler.recycledProcessors.clear(); super.destroy(); } + + + @Override + protected RequestGroupInfo getRequestGroupInfo() { + return cHandler.global; + } + + // ------------------------------------------------------------- Properties @@ -369,6 +354,4 @@ public class Http11Protocol extends Abst } } - - } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Wed Dec 1 18:37:29 2010 @@ -128,11 +128,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; @@ -449,7 +444,10 @@ public abstract class AbstractEndpoint { } - public abstract void init() throws Exception; + public void init() throws Exception { + // TODO JMX Registration? + } + public abstract void start() throws Exception; /** Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Wed Dec 1 18:37:29 2010 @@ -362,14 +362,10 @@ public class AprEndpoint extends Abstrac /** - * Initialize the endpoint. + * Start the APR endpoint, creating acceptor, poller and sendfile threads. */ @Override - public void init() - throws Exception { - - if (initialized) - return; + public void start() throws Exception { // Create the root APR memory pool try { @@ -519,21 +515,6 @@ public class AprEndpoint extends Abstrac useSendfile = false; } - initialized = true; - - } - - - /** - * 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(); - } if (!running) { running = true; paused = false; @@ -658,18 +639,7 @@ public class AprEndpoint extends Abstrac } } shutdownExecutor(); - } - - - /** - * Deallocate APR memory pools, and close server socket. - */ - @Override - public void destroy() throws Exception { - if (running) { - stop(); - } - + // Destroy pool if it was initialised if (serverSockPool != 0) { Pool.destroy(serverSockPool); @@ -689,8 +659,17 @@ public class AprEndpoint extends Abstrac Pool.destroy(rootPool); rootPool = 0; } + } + - initialized = false; + /** + * Deallocate APR memory pools, and close server socket. + */ + @Override + public void destroy() throws Exception { + if (running) { + stop(); + } } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java Wed Dec 1 18:37:29 2010 @@ -322,12 +322,8 @@ public class JIoEndpoint extends Abstrac // -------------------- Public methods -------------------- @Override - public void init() - throws Exception { + public void start() throws Exception { - if (initialized) - return; - // Initialize thread count defaults for acceptor if (acceptorThreadCount == 0) { acceptorThreadCount = 1; @@ -397,19 +393,7 @@ public class JIoEndpoint extends Abstrac throw be; } } - //if( serverTimeout >= 0 ) - // serverSocket.setSoTimeout( serverTimeout ); - - initialized = true; - - } - - @Override - public void start() throws Exception { - // Initialize socket if not done before - if (!initialized) { - init(); - } + if (!running) { running = true; paused = false; @@ -447,16 +431,7 @@ public class JIoEndpoint extends Abstrac unlockAccept(); } shutdownExecutor(); - } - /** - * Deallocate APR memory pools, and close server socket. - */ - @Override - public void destroy() throws Exception { - if (running) { - stop(); - } if (serverSocket != null) { try { if (serverSocket != null) @@ -466,7 +441,16 @@ public class JIoEndpoint extends Abstrac } serverSocket = null; } - initialized = false ; + } + + /** + * Deallocate APR memory pools, and close server socket. + */ + @Override + public void destroy() throws Exception { + if (running) { + stop(); + } } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Wed Dec 1 18:37:29 2010 @@ -448,18 +448,28 @@ public class NioEndpoint extends Abstrac } + public KeyManager[] wrap(KeyManager[] managers) { + if (managers==null) return null; + KeyManager[] result = new KeyManager[managers.length]; + for (int i=0; i<result.length; i++) { + if (managers[i] instanceof X509KeyManager && getKeyAlias()!=null) { + result[i] = new NioX509KeyManager((X509KeyManager)managers[i],getKeyAlias()); + } else { + result[i] = managers[i]; + } + } + return result; + } + + // ----------------------------------------------- Public Lifecycle Methods /** - * Initialize the endpoint. + * Start the NIO endpoint, creating acceptor, poller threads. */ @Override - public void init() - throws Exception { - - if (initialized) - return; + public void start() throws Exception { serverSock = ServerSocketChannel.open(); socketProperties.setProperties(serverSock.socket()); @@ -525,34 +535,7 @@ public class NioEndpoint extends Abstrac if (oomParachute>0) reclaimParachute(true); selectorPool.open(); - initialized = true; - - } - - public KeyManager[] wrap(KeyManager[] managers) { - if (managers==null) return null; - KeyManager[] result = new KeyManager[managers.length]; - for (int i=0; i<result.length; i++) { - if (managers[i] instanceof X509KeyManager && getKeyAlias()!=null) { - result[i] = new NioX509KeyManager((X509KeyManager)managers[i],getKeyAlias()); - } else { - result[i] = managers[i]; - } - } - return result; - } - - /** - * Start the NIO endpoint, creating acceptor, poller threads. - */ - @Override - public void start() - throws Exception { - // Initialize socket if not done before - if (!initialized) { - init(); - } if (!running) { running = true; paused = false; @@ -587,7 +570,7 @@ public class NioEndpoint extends Abstrac * Stop the endpoint. This will cause all processing threads to stop. */ @Override - public void stop() { + public void stop() throws Exception { if (!paused) { pause(); } @@ -607,6 +590,13 @@ public class NioEndpoint extends Abstrac processorCache.clear(); shutdownExecutor(); + // Close server socket + serverSock.socket().close(); + serverSock.close(); + serverSock = null; + sslContext = null; + releaseCaches(); + selectorPool.close(); } @@ -621,14 +611,7 @@ public class NioEndpoint extends Abstrac if (running) { stop(); } - // Close server socket - serverSock.socket().close(); - serverSock.close(); - serverSock = null; - sslContext = null; - initialized = false; - releaseCaches(); - selectorPool.close(); + if (log.isDebugEnabled()) { log.debug("Destroy completed for "+new InetSocketAddress(getAddress(),getPort())); } Added: tomcat/trunk/test/org/apache/tomcat/util/net/TestXxxEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TestXxxEndpoint.java?rev=1041120&view=auto ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/util/net/TestXxxEndpoint.java (added) +++ tomcat/trunk/test/org/apache/tomcat/util/net/TestXxxEndpoint.java Wed Dec 1 18:37:29 2010 @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.tomcat.util.net; + +import java.io.File; +import java.net.ServerSocket; + +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; + +/** + * Test case for the Endpoint implementations. The testing framework will ensure + * that each implementation is tested. + */ +public class TestXxxEndpoint extends TomcatBaseTest { + + public void testStartStop() throws Exception { + Tomcat tomcat = getTomcatInstance(); + File appDir = new File(getBuildDirectory(), "webapps/examples"); + tomcat.addWebapp(null, "/examples", appDir.getAbsolutePath()); + + int port = getPort(); + + tomcat.start(); + + tomcat.getConnector().stop(); + // This will throw an exception if the socket is not released + ServerSocket socket = new ServerSocket(port); + socket.close(); + tomcat.getConnector().start(); + } +} Propchange: tomcat/trunk/test/org/apache/tomcat/util/net/TestXxxEndpoint.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1041120&r1=1041119&r2=1041120&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Dec 1 18:37:29 2010 @@ -73,8 +73,12 @@ of the deprecated Embedded class. (markt) </fix> <fix> - Further Lifecycle refactoring for Connectors and associated components. - (markt) + <bug>50360</bug>: Further Lifecycle refactoring for Connectors and + associated components. The socket is now bound on + <code>Connector.start()</code> rather than + <code>Connector.init()</code> and the socket is released on + <code>Connector.stop()</code> rather than + <code>Connector.destroy()</code>.(markt) </fix> <fix> Correct handling of versioned web applications in deployer. (markt) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org