The 60s timeout waiting for session info from other nodes will block processing of...

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

diff --git a/java/org/apache/catalina/core/ContainerBase.java b/java/org/apache/catalina/core/ContainerBase.java
index 2b6c78b..f45e3fc 100644 (file)
--- a/java/org/apache/catalina/core/ContainerBase.java
+++ b/java/org/apache/catalina/core/ContainerBase.java
@@ -799,29 +799,32 @@ public abstract class ContainerBase extends LifecycleMBeanBase
                                                    "' is not unique");
             child.setParent(this);  // May throw IAE
             children.put(child.getName(), child);
+        }
 
-            // Start child
-            if ((getState().isAvailable() ||
-                    LifecycleState.STARTING_PREP.equals(getState())) &&
-                    startChildren) {
-                boolean success = false;
-                try {
-                    child.start();
-                    success = true;
-                } catch (LifecycleException e) {
-                    log.error("ContainerBase.addChild: start: ", e);
-                    throw new IllegalStateException
-                        ("ContainerBase.addChild: start: " + e);
-                } finally {
-                    if (!success) {
+        // Start child
+        // Don't do this inside sync block - start can be a slow process and
+        // locking the children object can cause problems elsewhere
+        if ((getState().isAvailable() ||
+                LifecycleState.STARTING_PREP.equals(getState())) &&
+                startChildren) {
+            boolean success = false;
+            try {
+                child.start();
+                success = true;
+            } catch (LifecycleException e) {
+                log.error("ContainerBase.addChild: start: ", e);
+                throw new IllegalStateException
+                    ("ContainerBase.addChild: start: " + e);
+            } finally {
+                if (!success) {
+                    synchronized (children) {
                         children.remove(child.getName());
                     }
                 }
             }
-
-            fireContainerEvent(ADD_CHILD_EVENT, child);
         }
 
+        fireContainerEvent(ADD_CHILD_EVENT, child);
     }
 
 
@@ -864,7 +867,7 @@ public abstract class ContainerBase extends LifecycleMBeanBase
 
         if (name == null)
             return (null);
-        synchronized (children) {       // Required by post-start changes
+        synchronized (children) {
             return children.get(name);
         }
 

diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4a308d2..806cd4a 100644 (file)
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -140,6 +140,12 @@
         Reduce synchronization in session managers to improve performance of
         session creation. (markt)
       </add>
+      <fix>
+        If starting children automatically when adding them to a container (e.g.
+        when adding a Context to a Host) don&apos;t lock the parent&apos;s set
+        of children whilst the new child is being started since this can block
+        other threads and cause issues such as lost cluster messages. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">
@@ -205,6 +211,11 @@
         send options to be set for the reply message. Based on a patch by Ariel.
         (markt)
       </fix>
+      <fix>
+        Ensure that a new Context waiting for session data from other nodes in
+        the cluster does not block the processing of clustering messages for
+        other Contexts. (markt) 
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Web applications">