You are viewing a plain text version of this content. The canonical link for it is here.
Posted to scm@geronimo.apache.org by xu...@apache.org on 2010/11/01 09:57:25 UTC
svn commit: r1029577 -
/geronimo/external/trunk/tomcat-parent-7.0.0/catalina/src/main/java/org/apache/catalina/core/ContainerBase.java
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();