geronimo-scm mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From xuhaih...@apache.org
Subject svn commit: r1029577 - /geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java
Date Mon, 01 Nov 2010 08:57:25 GMT
Author: xuhaihong
Date: Mon Nov  1 08:57:24 2010
New Revision: 1029577

URL: http://svn.apache.org/viewvc?rev=1029577&view=rev
Log:
GERONIMO-5665 destoryInternal method is called twice while destorying the children directly

Modified:
    geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java

Modified: geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java
URL: http://svn.apache.org/viewvc/geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java?rev=1029577&r1=1029576&r2=1029577&view=diff
==============================================================================
--- geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java
(original)
+++ geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java
Mon Nov  1 08:57:24 2010
@@ -5,9 +5,9 @@
  * 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.
@@ -114,9 +114,9 @@ import org.apache.tomcat.util.res.String
  * </table>
  * Subclasses that fire additional events should document them in the
  * class comments of the implementation class.
- * 
- * TODO: Review synchronisation around background processing. See bug 47024. 
- * 
+ *
+ * TODO: Review synchronisation around background processing. See bug 47024.
+ *
  * @author Craig R. McClanahan
  */
 public abstract class ContainerBase extends LifecycleMBeanBase
@@ -187,7 +187,7 @@ public abstract class ContainerBase exte
      * Associated logger name.
      */
     protected String logName = null;
-    
+
 
     /**
      * The Manager implementation with which this Container is associated.
@@ -200,7 +200,7 @@ public abstract class ContainerBase exte
      */
     protected Cluster cluster = null;
 
-    
+
     /**
      * The human-readable name of this Container.
      */
@@ -281,10 +281,10 @@ public abstract class ContainerBase exte
     /**
      * Get the delay between the invocation of the backgroundProcess method on
      * this container and its children. Child containers will not be invoked
-     * if their delay value is not negative (which would mean they are using 
-     * their own thread). Setting this to a positive value will cause 
-     * a thread to be spawn. After waiting the specified amount of time, 
-     * the thread will invoke the executePeriodic method on this container 
+     * if their delay value is not negative (which would mean they are using
+     * their own thread). Setting this to a positive value will cause
+     * a thread to be spawn. After waiting the specified amount of time,
+     * the thread will invoke the executePeriodic method on this container
      * and all its children.
      */
     @Override
@@ -296,8 +296,8 @@ public abstract class ContainerBase exte
     /**
      * Set the delay between the invocation of the execute method on this
      * container and its children.
-     * 
-     * @param delay The delay in seconds between the invocation of 
+     *
+     * @param delay The delay in seconds between the invocation of
      *              backgroundProcess methods
      */
     @Override
@@ -740,8 +740,8 @@ public abstract class ContainerBase exte
     @Override
     public synchronized void setResources(DirContext resources) {
         // Called from StandardContext.setResources()
-        //              <- StandardContext.start() 
-        //              <- ContainerBase.addChildInternal() 
+        //              <- StandardContext.start()
+        //              <- ContainerBase.addChildInternal()
 
         // Change components if necessary
         DirContext oldResources = this.resources;
@@ -896,7 +896,7 @@ public abstract class ContainerBase exte
     public ContainerListener[] findContainerListeners() {
 
         synchronized (listeners) {
-            ContainerListener[] results = 
+            ContainerListener[] results =
                 new ContainerListener[listeners.size()];
             return listeners.toArray(results);
         }
@@ -940,13 +940,9 @@ public abstract class ContainerBase exte
         if (child == null) {
             return;
         }
-        
-        synchronized(children) {
-            if (children.get(child.getName()) == null)
-                return;
-            children.remove(child.getName());
-        }
-        
+
+        removeChildInternal(child);
+
         try {
             if (child.getState().isAvailable()) {
                 child.stop();
@@ -954,9 +950,9 @@ public abstract class ContainerBase exte
         } catch (LifecycleException e) {
             log.error("ContainerBase.removeChild: stop: ", e);
         }
-        
+
         fireContainerEvent(REMOVE_CHILD_EVENT, child);
-        
+
         // Set child's parent to null to prevent a loop
         child.setParent(null);
         try {
@@ -967,6 +963,13 @@ public abstract class ContainerBase exte
 
     }
 
+    protected void removeChildInternal(Container child) {
+        synchronized(children) {
+            if (children.get(child.getName()) == null)
+                return;
+            children.remove(child.getName());
+        }
+    }
 
     /**
      * Remove a container event listener from this component.
@@ -1091,25 +1094,23 @@ public abstract class ContainerBase exte
     @Override
     protected void destroyInternal() throws LifecycleException {
 
-        // Stop the Valves in our pipeline (including the basic), if any
-        if (pipeline instanceof Lifecycle) {
-            ((Lifecycle) pipeline).destroy();
-        }
-
-        // Remove children now this container is being destroyed
-        for (Container child : findChildren()) {
-            removeChild(child);
-        }
-
         // Required if the child is destroyed directly.
         if (parent != null) {
             parent.removeChild(this);
+        } else {
+            // Stop the Valves in our pipeline (including the basic), if any
+            if (pipeline instanceof Lifecycle) {
+                ((Lifecycle) pipeline).destroy();
+            }
+            // Remove children now this container is being destroyed
+            for (Container child : findChildren()) {
+                removeChildInternal(child);
+            }
+            super.destroyInternal();
         }
-
-        super.destroyInternal();
     }
 
-    
+
     /**
      * Check this container for an access log and if none is found, look to the
      * parent. If there is no parent and still none is found, use the NoOp
@@ -1118,14 +1119,14 @@ public abstract class ContainerBase exte
     @Override
     public void logAccess(Request request, Response response, long time,
             boolean useDefault) {
-        
+
         boolean logged = false;
-        
+
         if (getAccessLog() != null) {
             getAccessLog().log(request, response, time);
             logged = true;
         }
-        
+
         if (getParent() != null) {
             // No need to use default logger once request/response has been logged
             // once
@@ -1135,11 +1136,11 @@ public abstract class ContainerBase exte
 
     @Override
     public AccessLog getAccessLog() {
-        
+
         if (accessLogScanComplete) {
             return accessLog;
         }
-        
+
         Valve valves[] = getPipeline().getValves();
         for (Valve valve : valves) {
             if (valve instanceof AccessLog) {
@@ -1184,7 +1185,7 @@ public abstract class ContainerBase exte
      */
     @Override
     public void backgroundProcess() {
-        
+
         if (!getState().isAvailable())
             return;
 
@@ -1192,28 +1193,28 @@ public abstract class ContainerBase exte
             try {
                 cluster.backgroundProcess();
             } catch (Exception e) {
-                log.warn(sm.getString("containerBase.backgroundProcess.cluster", cluster),
e);                
+                log.warn(sm.getString("containerBase.backgroundProcess.cluster", cluster),
e);
             }
         }
         if (loader != null) {
             try {
                 loader.backgroundProcess();
             } catch (Exception e) {
-                log.warn(sm.getString("containerBase.backgroundProcess.loader", loader),
e);                
+                log.warn(sm.getString("containerBase.backgroundProcess.loader", loader),
e);
             }
         }
         if (manager != null) {
             try {
                 manager.backgroundProcess();
             } catch (Exception e) {
-                log.warn(sm.getString("containerBase.backgroundProcess.manager", manager),
e);                
+                log.warn(sm.getString("containerBase.backgroundProcess.manager", manager),
e);
             }
         }
         if (realm != null) {
             try {
                 realm.backgroundProcess();
             } catch (Exception e) {
-                log.warn(sm.getString("containerBase.backgroundProcess.realm", realm), e);
               
+                log.warn(sm.getString("containerBase.backgroundProcess.realm", realm), e);
             }
         }
         Valve current = pipeline.getFirst();
@@ -1221,7 +1222,7 @@ public abstract class ContainerBase exte
             try {
                 current.backgroundProcess();
             } catch (Exception e) {
-                log.warn(sm.getString("containerBase.backgroundProcess.valve", current),
e);                
+                log.warn(sm.getString("containerBase.backgroundProcess.valve", current),
e);
             }
             current = current.getNext();
         }
@@ -1271,16 +1272,16 @@ public abstract class ContainerBase exte
             if ((name == null) || (name.equals(""))) {
                 name = "/";
             }
-            loggerName = "[" + name + "]" 
+            loggerName = "[" + name + "]"
                 + ((loggerName != null) ? ("." + loggerName) : "");
             current = current.getParent();
         }
         logName = ContainerBase.class.getName() + "." + loggerName;
         return logName;
-        
+
     }
 
-    
+
     // -------------------- JMX and Registration  --------------------
 
     @Override
@@ -1301,7 +1302,7 @@ public abstract class ContainerBase exte
         return result;
     }
 
-    
+
     // -------------------- Background Thread --------------------
 
     /**
@@ -1350,7 +1351,7 @@ public abstract class ContainerBase exte
 
 
     /**
-     * Private thread class to invoke the backgroundProcess method 
+     * Private thread class to invoke the backgroundProcess method
      * of this container and its children after a fixed delay.
      */
     protected class ContainerBackgroundProcessor implements Runnable {
@@ -1365,7 +1366,7 @@ public abstract class ContainerBase exte
                 }
                 if (!threadDone) {
                     Container parent = (Container) getMappingObject();
-                    ClassLoader cl = 
+                    ClassLoader cl =
                         Thread.currentThread().getContextClassLoader();
                     if (parent.getLoader() != null) {
                         cl = parent.getLoader().getClassLoader();



Mime
View raw message