Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=50157

Ensure MapperListener is only added to a container object once.
Also
 - Improve debug logging for MapperListener registration.
 - Expose names of LifecycleListeners and ContainerListers for StandardContext via JMX.

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

diff --git a/java/org/apache/catalina/connector/LocalStrings.properties b/java/org/apache/catalina/connector/LocalStrings.properties
index 50b63af..02d5efb 100644 (file)
--- a/java/org/apache/catalina/connector/LocalStrings.properties
+++ b/java/org/apache/catalina/connector/LocalStrings.properties
@@ -73,12 +73,13 @@ cometEvent.nullRequest=The event object has been recycled and is no longer assoc
 #
 # MapperListener
 #
-mapperListener.unknownDefaultHost=Unknown default host: {0}
-mapperListener.registerHost=Register host {0} at domain {1}
-mapperListener.unregisterHost=Unregister host {0} at domain {1}
-mapperListener.registerContext=Register Context {0} 
-mapperListener.unregisterContext=Unregister Context {0}
-mapperListener.registerWrapper=Register Wrapper {0} in Context {1}
+mapperListener.unknownDefaultHost=Unknown default host [{0}] for connector [{1}]
+mapperListener.registerHost=Register host [{0}] at domain [{1}] for connector [{2}]
+mapperListener.unregisterHost=Unregister host [{0}] at domain [{1}] for connector [{2}]
+mapperListener.registerContext=Register Context [{0}] for connector [{1}]
+mapperListener.unregisterContext=Unregister Context [{0}] for connector [{1}]
+mapperListener.registerWrapper=Register Wrapper [{0}] in Context [{1}] for connector [{2}]
+mapperListener.unregisterWrapper=Unregister Wrapper [{0}] in Context [{1}] for connector [{2}]
 mapperListener.addMBeanListenerFail=Failed to add MBean notification listener for connector [{0}] in domain [{1}]. Adding Hosts, Contexts and Wrappers will not be visible to the connector.
 mapperListener.removeMBeanListenerFail=Failed to remove MBean notification listener for connector [{0}] in domain [{1}]. This may result in a memory leak.
 mapperListener.lifecycleListenerFail=Failed to add Lifecycle listener to object [{0}]. Changes in the object state may not be correctly reflected in the mapper for connector [{1}] in domain [{2}].

diff --git a/java/org/apache/catalina/connector/MapperListener.java b/java/org/apache/catalina/connector/MapperListener.java
index 32c99a3..27650ec 100644 (file)
--- a/java/org/apache/catalina/connector/MapperListener.java
+++ b/java/org/apache/catalina/connector/MapperListener.java
@@ -101,13 +101,12 @@ public class MapperListener implements ContainerListener, LifecycleListener {
         findDefaultHost();
         
         Engine engine = (Engine) connector.getService().getContainer();
-        engine.addContainerListener(this);
+        addListeners(engine);
         
         Container[] conHosts = engine.findChildren();
         for (Container conHost : conHosts) {
             Host host = (Host) conHost;
             if (!LifecycleState.NEW.equals(host.getState())) {
-                host.addLifecycleListener(this);
                 // Registering the host will register the context and wrappers
                 registerHost(host);
             }
@@ -125,29 +124,28 @@ public class MapperListener implements ContainerListener, LifecycleListener {
 
     // --------------------------------------------- Container Listener methods
 
+    @Override
     public void containerEvent(ContainerEvent event) {
 
         if (event.getType() == Container.ADD_CHILD_EVENT) {
             Container child = (Container) event.getData();
-            child.addLifecycleListener(this);
-            child.addContainerListener(this);
-            if (child instanceof Host) {
-                registerHost((Host) child);
-            } else if (child instanceof Context) {
-                registerContext((Context) child);
-            } else if (child instanceof Wrapper) {
-                registerWrapper((Wrapper) child);
+            addListeners(child);
+            // If child is started then it is too late for life-cycle listener
+            // to register the child so register it here
+            if (child.getState().isAvailable()) {
+                if (child instanceof Host) {
+                    registerHost((Host) child);
+                } else if (child instanceof Context) {
+                    registerContext((Context) child);
+                } else if (child instanceof Wrapper) {
+                    registerWrapper((Wrapper) child);
+                }
             }
         } else if (event.getType() == Container.REMOVE_CHILD_EVENT) {
             Container child = (Container) event.getData();
             removeListeners(child);
-            if (child instanceof Host) {
-                unregisterHost((Host) child);
-            } else if (child instanceof Context) {
-                unregisterContext((Context) child);
-            } else if (child instanceof Wrapper) {
-                unregisterWrapper((Wrapper) child);
-            }
+            // No need to unregister - life-cycle listener will handle this when
+            // the child stops
         } else if (event.getType() == Host.ADD_ALIAS_EVENT) {
             // Handle dynamically adding host aliases
             mapper.addHostAlias(((Host) event.getSource()).getName(),
@@ -260,7 +258,7 @@ public class MapperListener implements ContainerListener, LifecycleListener {
             mapper.setDefaultHostName(defaultHost);
         } else {
             log.warn(sm.getString("mapperListener.unknownDefaultHost",
-                    defaultHost));
+                    defaultHost, connector));
         }
     }
 
@@ -273,14 +271,12 @@ public class MapperListener implements ContainerListener, LifecycleListener {
         String[] aliases = host.findAliases();
         mapper.addHost(host.getName(), aliases, host);
         
-        host.addContainerListener(this);
-        
         for (Container container : host.findChildren()) {
             registerContext((Context) container);
         }
         if(log.isDebugEnabled()) {
-            log.debug(sm.getString
-                 ("mapperListener.registerHost", host.getName(), domain));
+            log.debug(sm.getString("mapperListener.registerHost",
+                    host.getName(), domain, connector));
         }
     }
 
@@ -296,7 +292,7 @@ public class MapperListener implements ContainerListener, LifecycleListener {
 
         if(log.isDebugEnabled())
             log.debug(sm.getString("mapperListener.unregisterHost", hostname,
-                    domain));
+                    domain, connector));
     }
 
     
@@ -306,6 +302,8 @@ public class MapperListener implements ContainerListener, LifecycleListener {
     private void unregisterWrapper(Wrapper wrapper) {
 
         String contextName = wrapper.getParent().getName();
+        String wrapperName = wrapper.getName();
+
         if ("/".equals(contextName)) {
             contextName = "";
         }
@@ -316,6 +314,11 @@ public class MapperListener implements ContainerListener, LifecycleListener {
         for (String mapping : mappings) {
             mapper.removeWrapper(hostName, contextName, mapping);
         }
+        
+        if(log.isDebugEnabled()) {
+            log.debug(sm.getString("mapperListener.unregisterWrapper",
+                    wrapperName, contextName, connector));
+        }
     }
 
     
@@ -336,15 +339,13 @@ public class MapperListener implements ContainerListener, LifecycleListener {
         mapper.addContext(host.getName(), host, contextName, context,
                 welcomeFiles, resources);
 
-        context.addContainerListener(this);
-       
         for (Container container : context.findChildren()) {
             registerWrapper((Wrapper) container);
         }
 
         if(log.isDebugEnabled()) {
-            log.debug(sm.getString
-                 ("mapperListener.registerContext", contextName));
+            log.debug(sm.getString("mapperListener.registerContext",
+                    contextName, connector));
         }
     }
 
@@ -366,8 +367,8 @@ public class MapperListener implements ContainerListener, LifecycleListener {
         String hostName = context.getParent().getName();
 
         if(log.isDebugEnabled())
-            log.debug(sm.getString
-                  ("mapperListener.unregisterContext", contextName));
+            log.debug(sm.getString("mapperListener.unregisterContext",
+                    contextName, connector));
 
         mapper.removeContext(hostName, contextName);
     }
@@ -394,11 +395,9 @@ public class MapperListener implements ContainerListener, LifecycleListener {
                               jspWildCard);
         }
 
-        wrapper.addContainerListener(this);
-
         if(log.isDebugEnabled()) {
             log.debug(sm.getString("mapperListener.registerWrapper",
-                    wrapperName, contextName));
+                    wrapperName, contextName, connector));
         }
     }
 
@@ -425,6 +424,21 @@ public class MapperListener implements ContainerListener, LifecycleListener {
         }
     }
 
+
+    /**
+     * Add this mapper to the container and all child containers
+     * 
+     * @param container
+     */
+    private void addListeners(Container container) {
+        container.addContainerListener(this);
+        container.addLifecycleListener(this);
+        for (Container child : container.findChildren()) {
+            addListeners(child);
+        }
+    }
+
+
     /**
      * Remove this mapper from the container and all child containers
      * 

diff --git a/java/org/apache/catalina/core/mbeans-descriptors.xml b/java/org/apache/catalina/core/mbeans-descriptors.xml
index fb68795..9959610 100644 (file)
--- a/java/org/apache/catalina/core/mbeans-descriptors.xml
+++ b/java/org/apache/catalina/core/mbeans-descriptors.xml
@@ -583,6 +583,12 @@
                returnType="java.lang.String">    
     </operation>
     
+    <operation name="findContainerListenerNames"
+               description="Return the set of container listener class names configured for this application."
+               impact="INFO"
+               returnType="[Ljava.lang.String;">    
+    </operation>
+    
     <operation name="findErrorPage"
                description="Return the error page entry for the specified HTTP error code, if any; otherwise return null"
                impact="ACTION"
@@ -634,6 +640,12 @@
                returnType="[Ljava.lang.String;">    
     </operation>
     
+    <operation name="findLifecycleListenerNames"
+               description="Return the set of lifecycle listener class names configured for this application."
+               impact="INFO"
+               returnType="[Ljava.lang.String;">    
+    </operation>
+    
     <operation name="findMimeMapping"
                description="Return the MIME type to which the specified extension is mapped, if any; otherwise return null."
                impact="ACTION"

diff --git a/java/org/apache/catalina/mbeans/ContainerMBean.java b/java/org/apache/catalina/mbeans/ContainerMBean.java
index f2d2cd3..110d514 100644 (file)
--- a/java/org/apache/catalina/mbeans/ContainerMBean.java
+++ b/java/org/apache/catalina/mbeans/ContainerMBean.java
@@ -17,6 +17,9 @@
 
 package org.apache.catalina.mbeans;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import javax.management.InstanceNotFoundException;
 import javax.management.MBeanException;
 import javax.management.MalformedObjectNameException;
@@ -25,6 +28,7 @@ import javax.management.RuntimeOperationsException;
 import javax.management.modelmbean.InvalidTargetObjectTypeException;
 
 import org.apache.catalina.Container;
+import org.apache.catalina.ContainerListener;
 import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleListener;
 import org.apache.catalina.Valve;
@@ -265,4 +269,57 @@ public class ContainerMBean extends BaseModelMBean {
         }
     }
 
+    
+    /**
+     * List the class name of each of the lifecycle listeners added to this
+     * container.
+     */
+    public String[] findLifecycleListenerNames() throws MBeanException {
+        ContainerBase container = null;
+        List<String> result = new ArrayList<String>();
+
+        try {
+            container = (ContainerBase) getManagedResource();
+        } catch (InstanceNotFoundException e) {
+            throw new MBeanException(e);
+        } catch (RuntimeOperationsException e) {
+            throw new MBeanException(e);
+        } catch (InvalidTargetObjectTypeException e) {
+            throw new MBeanException(e);
+        }
+
+        LifecycleListener[] listeners = container.findLifecycleListeners();
+        for(LifecycleListener listener: listeners){
+            result.add(listener.getClass().getName());
+        }
+
+        return result.toArray(new String[result.size()]);
+    }
+
+    
+    /**
+     * List the class name of each of the container listeners added to this
+     * container.
+     */
+    public String[] findContainerListenerNames() throws MBeanException {
+        ContainerBase container = null;
+        List<String> result = new ArrayList<String>();
+
+        try {
+            container = (ContainerBase) getManagedResource();
+        } catch (InstanceNotFoundException e) {
+            throw new MBeanException(e);
+        } catch (RuntimeOperationsException e) {
+            throw new MBeanException(e);
+        } catch (InvalidTargetObjectTypeException e) {
+            throw new MBeanException(e);
+        }
+
+        ContainerListener[] listeners = container.findContainerListeners();
+        for(ContainerListener listener: listeners){
+            result.add(listener.getClass().getName());
+        }
+
+        return result.toArray(new String[result.size()]);
+    }
 }

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c2b12f8..d355209 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -57,6 +57,17 @@
         <bug>50138</bug>: Fix threading issues in
         <code>org.apache.catalina.security.SecurityUtil</code>. (markt)
       </fix>
+      <fix>
+        <bug>50157</bug>: Ensure MapperListener is only added to a container
+        object once. (markt)
+      </fix>
+      <add>
+        Improve debug logging for MapperListener registration. (markt)
+      </add>
+      <add>
+        Expose names of LifecycleListeners and ContainerListers for
+        StandardContext via JMX. (markt)
+      </add>
     </changelog>
   </subsection>
   <subsection name="Jasper">