From: markt <markt@13f79535-47bb-0310-9956-ffa450edef68>
Date: Wed, 1 Dec 2010 18:37:29 +0000 (+0000)
Subject: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50360
X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=ed92b512c896bdd7b62e55ec78ed7f1fde1ef555;p=tomcat7.0

Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50360
Bind/release socket on start()/stop()
Register/deregister MBeans on init()/destroy()

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1041120 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/java/org/apache/catalina/connector/Connector.java b/java/org/apache/catalina/connector/Connector.java
index 362bfb429..64e58463d 100644
--- a/java/org/apache/catalina/connector/Connector.java
+++ b/java/org/apache/catalina/connector/Connector.java
@@ -864,6 +864,7 @@ public class Connector extends LifecycleMBeanBase  {
         // Initialize adapter
         adapter = new CoyoteAdapter(this);
         protocolHandler.setAdapter(adapter);
+        protocolHandler.setDomain(getDomain());
 
         try {
             protocolHandler.init();
diff --git a/java/org/apache/coyote/ProtocolHandler.java b/java/org/apache/coyote/ProtocolHandler.java
index 8b2f608f0..547c80949 100644
--- a/java/org/apache/coyote/ProtocolHandler.java
+++ b/java/org/apache/coyote/ProtocolHandler.java
@@ -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();
 }
diff --git a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
index efabc0ba7..d1b1a9d41 100644
--- a/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
+++ b/java/org/apache/coyote/ajp/AbstractAjpProtocol.java
@@ -256,6 +256,12 @@ public abstract class AbstractAjpProtocol implements ProtocolHandler, MBeanRegis
         return oname;
     }
 
+    @Override
+    public void setDomain(String domain) {
+        this.domain = domain;
+    }
+
+    @Override
     public String getDomain() {
         return domain;
     }
diff --git a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
index abdd144a9..01d361e84 100644
--- a/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
+++ b/java/org/apache/coyote/http11/AbstractHttp11Protocol.java
@@ -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 AbstractHttp11Protocol implements ProtocolHandler, MBeanRe
         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 AbstractHttp11Protocol implements ProtocolHandler, MBeanRe
     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 AbstractHttp11Protocol implements ProtocolHandler, MBeanRe
         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;
diff --git a/java/org/apache/coyote/http11/Http11AprProtocol.java b/java/org/apache/coyote/http11/Http11AprProtocol.java
index 13756f1b8..cd083ed55 100644
--- a/java/org/apache/coyote/http11/Http11AprProtocol.java
+++ b/java/org/apache/coyote/http11/Http11AprProtocol.java
@@ -73,37 +73,14 @@ public class Http11AprProtocol extends AbstractHttp11Protocol {
      */
     @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 AbstractHttp11Protocol {
         super.destroy();
     }
 
+    @Override
+    protected RequestGroupInfo getRequestGroupInfo() {
+        return cHandler.global;
+    }
+
+
     private Http11ConnectionHandler cHandler;
 
     public boolean getUseSendfile() { return ((AprEndpoint)endpoint).getUseSendfile(); }
diff --git a/java/org/apache/coyote/http11/Http11NioProtocol.java b/java/org/apache/coyote/http11/Http11NioProtocol.java
index c92f0f658..28e74086b 100644
--- a/java/org/apache/coyote/http11/Http11NioProtocol.java
+++ b/java/org/apache/coyote/http11/Http11NioProtocol.java
@@ -76,39 +76,16 @@ public class Http11NioProtocol extends AbstractHttp11JsseProtocol {
      */
     @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 AbstractHttp11JsseProtocol {
     }
 
 
+    @Override
+    protected RequestGroupInfo getRequestGroupInfo() {
+        return cHandler.global;
+    }
+
+
     // -------------------- Properties--------------------
     
 
diff --git a/java/org/apache/coyote/http11/Http11Protocol.java b/java/org/apache/coyote/http11/Http11Protocol.java
index e4ed1e111..58879e5c0 100644
--- a/java/org/apache/coyote/http11/Http11Protocol.java
+++ b/java/org/apache/coyote/http11/Http11Protocol.java
@@ -85,9 +85,14 @@ public class Http11Protocol extends AbstractHttp11JsseProtocol {
 
     @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 AbstractHttp11JsseProtocol {
         }
         
         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 AbstractHttp11JsseProtocol {
         cHandler.recycledProcessors.clear();
         super.destroy();
     }
+    
+
+    @Override
+    protected RequestGroupInfo getRequestGroupInfo() {
+        return cHandler.global;
+    }
+
+
     // ------------------------------------------------------------- Properties
 
 
@@ -369,6 +354,4 @@ public class Http11Protocol extends AbstractHttp11JsseProtocol {
         }
 
     }
-
-
 }
diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 59cb8486f..c92fcae0f 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -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;
 
     /**
diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index 1fda4e758..10c2f48a3 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -362,14 +362,10 @@ public class AprEndpoint extends AbstractEndpoint {
 
 
     /**
-     * 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 AbstractEndpoint {
             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 AbstractEndpoint {
             }
         }
         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 AbstractEndpoint {
             Pool.destroy(rootPool);
             rootPool = 0;
         }
+    }
+
 
-        initialized = false;
+    /**
+     * Deallocate APR memory pools, and close server socket.
+     */
+    @Override
+    public void destroy() throws Exception {
+        if (running) {
+            stop();
+        }
     }
 
 
diff --git a/java/org/apache/tomcat/util/net/JIoEndpoint.java b/java/org/apache/tomcat/util/net/JIoEndpoint.java
index 441c85a5f..eea259d3a 100644
--- a/java/org/apache/tomcat/util/net/JIoEndpoint.java
+++ b/java/org/apache/tomcat/util/net/JIoEndpoint.java
@@ -322,12 +322,8 @@ public class JIoEndpoint extends AbstractEndpoint {
     // -------------------- 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 AbstractEndpoint {
                 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 AbstractEndpoint {
             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 AbstractEndpoint {
             }
             serverSocket = null;
         }
-        initialized = false ;
+    }
+
+    /**
+     * Deallocate APR memory pools, and close server socket.
+     */
+    @Override
+    public void destroy() throws Exception {
+        if (running) {
+            stop();
+        }
     }
 
 
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index de91ba160..de81bc180 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -448,18 +448,28 @@ public class NioEndpoint extends AbstractEndpoint {
     }
 
 
+    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 AbstractEndpoint {
         
         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 AbstractEndpoint {
      * 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 AbstractEndpoint {
         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 AbstractEndpoint {
         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()));
         }
diff --git a/test/org/apache/tomcat/util/net/TestXxxEndpoint.java b/test/org/apache/tomcat/util/net/TestXxxEndpoint.java
new file mode 100644
index 000000000..ff6d5b484
--- /dev/null
+++ b/test/org/apache/tomcat/util/net/TestXxxEndpoint.java
@@ -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();
+    }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4721bb898..950992104 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -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)