You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2010/11/19 17:20:09 UTC

svn commit: r1036918 - in /tomcat/trunk: java/org/apache/catalina/core/ContainerBase.java webapps/docs/changelog.xml

Author: markt
Date: Fri Nov 19 16:20:09 2010
New Revision: 1036918

URL: http://svn.apache.org/viewvc?rev=1036918&view=rev
Log:
The 60s timeout waiting for session info from other nodes will block processing of all cluster messages. As well as lost session updates, this can result in lost sessions if a fail-over occurs while the messages are being blocked.

Modified:
    tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java?rev=1036918&r1=1036917&r2=1036918&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/ContainerBase.java Fri Nov 19 16:20:09 2010
@@ -799,29 +799,32 @@ public abstract class ContainerBase exte
                                                    "' 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 exte
 
         if (name == null)
             return (null);
-        synchronized (children) {       // Required by post-start changes
+        synchronized (children) {
             return children.get(name);
         }
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1036918&r1=1036917&r2=1036918&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Nov 19 16:20:09 2010
@@ -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">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org