Author: markt
Date: Thu Jun 21 19:58:20 2012
New Revision: 1352664
URL: http://svn.apache.org/viewvc?rev=1352664&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53450
Correct regression in fix for
https://issues.apache.org/bugzilla/show_bug.cgi?id=52999 that was likely to
cause a deadlock when deploying a ROOT web application.
Modified:
tomcat/tc7.0.x/trunk/ (props changed)
tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ContainerBase.java
tomcat/tc7.0.x/trunk/res/maven/mvn-pub.xml
tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
Merged /tomcat/trunk:r1352661,1352663
Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ContainerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ContainerBase.java?rev=1352664&r1=1352663&r2=1352664&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ContainerBase.java
(original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/ContainerBase.java Thu
Jun 21 19:58:20 2012
@@ -29,6 +29,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.Callable;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Future;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadFactory;
@@ -176,12 +177,13 @@ public abstract class ContainerBase exte
/**
- * The container event listeners for this Container.
+ * The container event listeners for this Container. Implemented as a
+ * CopyOnWriteArrayList since listeners may invoke methods to add/remove
+ * themselves or other listeners and with a ReadWriteLock that would
trigger
+ * a deadlock.
*/
- protected ArrayList<ContainerListener> listeners =
- new ArrayList<ContainerListener>();
- protected final ReadWriteLock listenersLock = new ReentrantReadWriteLock();
-
+ protected List<ContainerListener> listeners =
+ new CopyOnWriteArrayList<ContainerListener>();
/**
* The Loader implementation with which this Container is associated.
@@ -915,14 +917,7 @@ public abstract class ContainerBase exte
*/
@Override
public void addContainerListener(ContainerListener listener) {
-
- Lock write = listenersLock.writeLock();
- write.lock();
- try {
- listeners.add(listener);
- } finally {
- write.unlock();
- }
+ listeners.add(listener);
}
@@ -979,16 +974,9 @@ public abstract class ContainerBase exte
*/
@Override
public ContainerListener[] findContainerListeners() {
-
- Lock read = listenersLock.readLock();
- read.lock();
- try {
- ContainerListener[] results =
- new ContainerListener[listeners.size()];
- return listeners.toArray(results);
- } finally {
- read.unlock();
- }
+ ContainerListener[] results =
+ new ContainerListener[0];
+ return listeners.toArray(results);
}
@@ -1066,14 +1054,7 @@ public abstract class ContainerBase exte
*/
@Override
public void removeContainerListener(ContainerListener listener) {
-
- Lock write = listenersLock.writeLock();
- write.lock();
- try {
- listeners.remove(listener);
- } finally {
- write.unlock();
- }
+ listeners.remove(listener);
}
@@ -1408,30 +1389,13 @@ public abstract class ContainerBase exte
@Override
public void fireContainerEvent(String type, Object data) {
- /*
- * Implementation note
- * There are two options here.
- * 1) Take a copy of listeners and fire the events outside of the read
- * lock
- * 2) Don't take a copy and fire the events inside the read lock
- *
- * Approach 2 has been used here since holding the read lock only
- * prevents writes and that is preferable to creating lots of array
- * objects. Since writes occur on start / stop (unless an external
- * management tool is used) then holding the read lock for a relatively
- * long time should not be an issue.
- */
- Lock read = listenersLock.readLock();
- read.lock();
- try {
- if (listeners.size() < 1)
- return;
- ContainerEvent event = new ContainerEvent(this, type, data);
- for (ContainerListener listener : listeners) {
- listener.containerEvent(event);
- }
- } finally {
- read.unlock();
+ if (listeners.size() < 1)
+ return;
+
+ ContainerEvent event = new ContainerEvent(this, type, data);
+ // Note for each uses an iterator internally so this is safe
+ for (ContainerListener listener : listeners) {
+ listener.containerEvent(event);
}
}
Modified: tomcat/tc7.0.x/trunk/res/maven/mvn-pub.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/res/maven/mvn-pub.xml?rev=1352664&r1=1352663&r2=1352664&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/res/maven/mvn-pub.xml (original)
+++ tomcat/tc7.0.x/trunk/res/maven/mvn-pub.xml Thu Jun 21 19:58:20 2012
@@ -220,7 +220,6 @@
jarFileName="tomcat-jdbc.jar"
srcJarFileName="tomcat-jdbc-src.jar"/>
-
<doMavenDeploy artifactId="tomcat-el-api"
jarFileName="el-api.jar"
srcJarFileName="el-api-src.jar"/>
Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1352664&r1=1352663&r2=1352664&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Jun 21 19:58:20 2012
@@ -65,6 +65,11 @@
stuckThreadNames property as a pair for the stuckThreadIds one,
add thread ids to the log messages. (kkolinko)
</update>
+ <fix>
+ <bug>53450</bug>: Correct regression in fix for <bug>52999</bug> that
+ could easily trigger a deadlock when deploying a ROOT web application.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
