Author: markt Date: Sun Sep 5 20:36:16 2010 New Revision: 992890 URL: http://svn.apache.org/viewvc?rev=992890&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49802 Allow the connectors to be stopped via JMX.
Modified: tomcat/trunk/java/org/apache/catalina/connector/Connector.java tomcat/trunk/java/org/apache/catalina/core/StandardService.java tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProtocol.java tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java tomcat/trunk/java/org/apache/coyote/ajp/AjpProtocol.java tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties 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=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/Connector.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/Connector.java Sun Sep 5 20:36:16 2010 @@ -916,6 +916,14 @@ public class Connector extends Lifecycle setState(LifecycleState.STOPPING); + try { + protocolHandler.stop(); + } catch (Exception e) { + throw new LifecycleException + (sm.getString + ("coyoteConnector.protocolHandlerStopFailed", e)); + } + // MapperListener doesn't follow Lifecycle conventions mapperListener.destroy(); } Modified: tomcat/trunk/java/org/apache/catalina/core/StandardService.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardService.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/StandardService.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/StandardService.java Sun Sep 5 20:36:16 2010 @@ -377,13 +377,13 @@ public class StandardService extends Lif /** * Retrieves executor by name, null if not found - * @param name String + * @param executorName String * @return Executor */ - public Executor getExecutor(String name) { + public Executor getExecutor(String executorName) { synchronized (executors) { for (Executor executor: executors) { - if (name.equals(executor.getName())) + if (executorName.equals(executor.getName())) return executor; } } @@ -462,7 +462,7 @@ public class StandardService extends Lif @Override protected void stopInternal() throws LifecycleException { - // Stop our defined Connectors first + // Pause connectors first synchronized (connectors) { for (Connector connector: connectors) { try { @@ -475,13 +475,6 @@ public class StandardService extends Lif } } - // Heuristic: Sleep for a while to ensure pause of the connector - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - // Ignore - } - if(log.isInfoEnabled()) log.info(sm.getString("standardService.stop.name", this.name)); setState(LifecycleState.STOPPING); @@ -492,14 +485,15 @@ public class StandardService extends Lif container.stop(); } } - // FIXME pero -- Why container stop first? KeepAlive connections can send request! - // Stop our defined Connectors first + + // Now stop the connectors synchronized (connectors) { for (Connector connector: connectors) { - if (LifecycleState.INITIALIZED.equals( + if (!LifecycleState.STARTED.equals( connector.getState())) { - // If Service fails to start, connectors may not have been - // started + // Connectors only need stopping if they are currently + // started. They may have failed to start or may have been + // stopped (e.g. via a JMX call) continue; } try { Modified: tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java (original) +++ tomcat/trunk/java/org/apache/coyote/ProtocolHandler.java Sun Sep 5 20:36:16 2010 @@ -82,6 +82,12 @@ public interface ProtocolHandler { /** + * Stop the protocol. + */ + public void stop() throws Exception; + + + /** * Destroy the protocol (optional). */ public void destroy() throws Exception; Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java Sun Sep 5 20:36:16 2010 @@ -378,7 +378,7 @@ public class AjpAprProcessor implements boolean openSocket = true; boolean keptAlive = false; - while (started && !error) { + while (started && !error && !endpoint.isPaused()) { // Parsing the request header try { @@ -474,14 +474,14 @@ public class AjpAprProcessor implements } // Add the socket to the poller - if (!error) { + if (!error && !endpoint.isPaused()) { endpoint.getPoller().add(socket); } else { openSocket = false; } rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (!async || error) + if (!async || error || endpoint.isPaused()) recycle(); return openSocket; Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProtocol.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpAprProtocol.java Sun Sep 5 20:36:16 2010 @@ -218,9 +218,20 @@ public class AjpAprProtocol log.info(sm.getString("ajpprotocol.resume", getName())); } - public void destroy() throws Exception { + public void stop() throws Exception { + try { + endpoint.stop(); + } catch (Exception ex) { + log.error(sm.getString("ajpprotocol.endpoint.stoperror"), ex); + throw ex; + } if (log.isInfoEnabled()) log.info(sm.getString("ajpprotocol.stop", getName())); + } + + public void destroy() throws Exception { + if (log.isInfoEnabled()) + log.info(sm.getString("ajpprotocol.destroy", getName())); endpoint.destroy(); if (tpOname!=null) Registry.getRegistry(null, null).unregisterComponent(tpOname); Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Sun Sep 5 20:36:16 2010 @@ -383,7 +383,7 @@ public class AjpProcessor implements Act // Error flag error = false; - while (started && !error) { + while (started && !error && !endpoint.isPaused()) { // Parsing the request header try { @@ -483,7 +483,7 @@ public class AjpProcessor implements Act recycle(); } - if (async && !error) { + if (async && !error && !endpoint.isPaused()) { rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); return SocketState.LONG; } else { Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProtocol.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProtocol.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/AjpProtocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProtocol.java Sun Sep 5 20:36:16 2010 @@ -220,9 +220,20 @@ public class AjpProtocol log.info(sm.getString("ajpprotocol.resume", getName())); } - public void destroy() throws Exception { + public void stop() throws Exception { + try { + endpoint.stop(); + } catch (Exception ex) { + log.error(sm.getString("ajpprotocol.endpoint.stoperror"), ex); + throw ex; + } if (log.isInfoEnabled()) log.info(sm.getString("ajpprotocol.stop", getName())); + } + + public void destroy() throws Exception { + if (log.isInfoEnabled()) + log.info(sm.getString("ajpprotocol.destroy", getName())); endpoint.destroy(); if (tpOname!=null) Registry.getRegistry(null, null).unregisterComponent(tpOname); Modified: tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/ajp/LocalStrings.properties Sun Sep 5 20:36:16 2010 @@ -23,8 +23,10 @@ # AjpAprProtocol # +ajpprotocol.destroy=Destroying Coyote AJP/1.3 on {0} ajpprotocol.endpoint.initerror=Error initializing endpoint ajpprotocol.endpoint.starterror=Error starting endpoint +ajpprotocol.endpoint.stoperror=Error stopping endpoint ajpprotocol.init=Initializing Coyote AJP/1.3 on {0} ajpprotocol.proto.error=Error reading request, ignored ajpprotocol.getattribute=Attribute {0} 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=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Protocol.java Sun Sep 5 20:36:16 2010 @@ -148,9 +148,21 @@ public abstract class AbstractHttp11Prot } @Override - public void destroy() throws Exception { + public void stop() throws Exception { + try { + endpoint.stop(); + } catch (Exception ex) { + getLog().error(sm.getString("http11protocol.endpoint.stoperror"), ex); + throw ex; + } if(getLog().isInfoEnabled()) getLog().info(sm.getString("http11protocol.stop", getName())); + } + + @Override + public void destroy() throws Exception { + if(getLog().isInfoEnabled()) + getLog().info(sm.getString("http11protocol.destroy", getName())); endpoint.destroy(); if( tpOname!=null ) Registry.getRegistry(null, null).unregisterComponent(tpOname); Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Sun Sep 5 20:36:16 2010 @@ -220,7 +220,7 @@ public class Http11AprProcessor extends boolean keptAlive = false; boolean openSocket = false; - while (!error && keepAlive && !comet && !async) { + while (!error && keepAlive && !comet && !async && !endpoint.isPaused()) { // Parsing the request header try { @@ -338,15 +338,13 @@ public class Http11AprProcessor extends rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (comet || async) { - if (error) { - inputBuffer.nextRequest(); - outputBuffer.nextRequest(); - recycle(); - return SocketState.CLOSED; - } else { - return SocketState.LONG; - } + if (error || endpoint.isPaused()) { + inputBuffer.nextRequest(); + outputBuffer.nextRequest(); + recycle(); + return SocketState.CLOSED; + } else if (comet || async) { + return SocketState.LONG; } else { recycle(); return (openSocket) ? SocketState.OPEN : SocketState.CLOSED; Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Sun Sep 5 20:36:16 2010 @@ -299,7 +299,7 @@ public class Http11NioProcessor extends boolean recycle = true; final KeyAttachment ka = (KeyAttachment)socket.getAttachment(false); - while (!error && keepAlive && !comet && !async) { + while (!error && keepAlive && !comet && !async && !endpoint.isPaused()) { //always default to our soTimeout ka.setTimeout(soTimeout); // Parsing the request header @@ -448,15 +448,13 @@ public class Http11NioProcessor extends }//while rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (comet || async) { - if (error) { - recycle(); - return SocketState.CLOSED; - } else { - return SocketState.LONG; - } + if (error || endpoint.isPaused()) { + recycle(); + return SocketState.CLOSED; + } else if (comet || async) { + return SocketState.LONG; } else { - if ( recycle ) { + if (recycle) { recycle(); } //return (openSocket) ? (SocketState.OPEN) : SocketState.CLOSED; Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Sun Sep 5 20:36:16 2010 @@ -181,7 +181,7 @@ public class Http11Processor extends Abs boolean keptAlive = socketWrapper.isKeptAlive(); - while (started && !error && keepAlive) { + while (started && !error && keepAlive && !endpoint.isPaused()) { // Parsing the request header try { @@ -309,7 +309,7 @@ public class Http11Processor extends Abs } rp.setStage(org.apache.coyote.Constants.STAGE_ENDED); - if (error) { + if (error || endpoint.isPaused()) { recycle(); return SocketState.CLOSED; } else if (async) { Modified: tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Sun Sep 5 20:36:16 2010 @@ -23,8 +23,10 @@ # Http11Protocol # +http11protocol.destroy=Destroying Coyote HTTP/1.1 on {0} http11protocol.endpoint.initerror=Error initializing endpoint http11protocol.endpoint.starterror=Error starting endpoint +http11protocol.endpoint.stoperror=Error stopping endpoint http11protocol.init=Initializing Coyote HTTP/1.1 on {0} http11protocol.proto.error=Error reading request, ignored http11protocol.proto.ioexception.debug=IOException reading request 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=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Sun Sep 5 20:36:16 2010 @@ -430,11 +430,38 @@ public abstract class AbstractEndpoint { } } - public abstract void pause(); - public abstract void resume(); + + public abstract void init() throws Exception; public abstract void start() throws Exception; + + /** + * Pause the endpoint, which will stop it accepting new connections. + */ + public void pause() { + if (running && !paused) { + paused = true; + unlockAccept(); + // Heuristic: Sleep for a while to ensure pause of the endpoint + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + // Ignore + } + } + } + + /** + * Resume the endpoint, which will make it start accepting new connections + * again. + */ + public void resume() { + if (running) { + paused = false; + } + } + + public abstract void stop() throws Exception; public abstract void destroy() throws Exception; - public abstract void init() throws Exception; public String adjustRelativePath(String path, String relativeTo) { File f = new File(path); 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=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Sun Sep 5 20:36:16 2010 @@ -640,33 +640,12 @@ public class AprEndpoint extends Abstrac } /** - * Pause the endpoint, which will make it stop accepting new sockets. - */ - @Override - public void pause() { - if (running && !paused) { - paused = true; - unlockAccept(); - } - } - - - /** - * Resume the endpoint, which will make it start accepting new sockets - * again. - */ - @Override - public void resume() { - if (running) { - paused = false; - } - } - - - /** * Stop the endpoint. This will cause all processing threads to stop. */ public void stop() { + if (!paused) { + pause(); + } if (running) { running = false; unlockAccept(); @@ -822,7 +801,10 @@ public class AprEndpoint extends Abstrac */ protected boolean processSocketWithOptions(long socket) { try { - getExecutor().execute(new SocketWithOptionsProcessor(socket)); + // During shutdown, executor may be null - avoid NPE + if (running) { + getExecutor().execute(new SocketWithOptionsProcessor(socket)); + } } catch (RejectedExecutionException x) { log.warn("Socket processing request was rejected for:"+socket,x); return false; 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=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/JIoEndpoint.java Sun Sep 5 20:36:16 2010 @@ -409,21 +409,10 @@ public class JIoEndpoint extends Abstrac } @Override - public void pause() { - if (running && !paused) { - paused = true; - unlockAccept(); - } - } - - @Override - public void resume() { - if (running) { - paused = false; - } - } - public void stop() { + if (!paused) { + pause(); + } if (running) { running = false; unlockAccept(); @@ -494,7 +483,10 @@ public class JIoEndpoint extends Abstrac try { SocketWrapper<Socket> wrapper = new SocketWrapper<Socket>(socket); wrapper.setKeepAliveLeft(getMaxKeepAliveRequests()); - getExecutor().execute(new SocketProcessor(wrapper)); + // During shutdown, executor may be null - avoid NPE + if (running) { + getExecutor().execute(new SocketProcessor(wrapper)); + } } catch (RejectedExecutionException x) { log.warn("Socket processing request was rejected for:"+socket,x); return false; 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=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Sun Sep 5 20:36:16 2010 @@ -437,28 +437,6 @@ public class NioEndpoint extends Abstrac } - /** - * Return the state of the endpoint. - * - * @return true if the endpoint is running, false otherwise - */ - @Override - public boolean isRunning() { - return running; - } - - - /** - * Return the state of the endpoint. - * - * @return true if the endpoint is paused, false otherwise - */ - @Override - public boolean isPaused() { - return paused; - } - - // ----------------------------------------------- Public Lifecycle Methods @@ -595,33 +573,12 @@ public class NioEndpoint extends Abstrac /** - * Pause the endpoint, which will make it stop accepting new sockets. - */ - @Override - public void pause() { - if (running && !paused) { - paused = true; - unlockAccept(); - } - } - - - /** - * Resume the endpoint, which will make it start accepting new sockets - * again. - */ - @Override - public void resume() { - if (running) { - paused = false; - } - } - - - /** * Stop the endpoint. This will cause all processing threads to stop. */ public void stop() { + if (!paused) { + pause(); + } if (running) { running = false; unlockAccept(); Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=992890&r1=992889&r2=992890&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Sun Sep 5 20:36:16 2010 @@ -143,6 +143,10 @@ Make sure async timeout thread is stopped when the connector is stopped. (markt) </fix> + <fix> + <bug>49802</bug>: Re-factor connector pause, stop and destroy methods so + that calling any of those methods has the expected results. (markt) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org