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 2012/05/08 20:54:45 UTC

svn commit: r1335692 - in /tomcat/trunk/java/org/apache/catalina: connector/CoyoteAdapter.java connector/MapperListener.java core/StandardContext.java core/StandardContextValve.java startup/HostConfig.java

Author: markt
Date: Tue May  8 18:54:44 2012
New Revision: 1335692

URL: http://svn.apache.org/viewvc?rev=1335692&view=rev
Log:
It appears that pausing requests for a Context during reload was relying on the mapper not being cleaned up correctly. The Lifecycle refactoring cleaned up the mapper registrations and broke the handling of paused requests. This commit restores that functionality and extends it. The key changes are:
- Contexts are no longer from the mapper if they are stopped while they are paused
- The CoyoteAdapter pauses for 1s if a request is mapped to a paused Context and then tries to re-map it. This replaces the reloading handling in the StandardContextValve
- The reload handling has been removed from the StandardContextValve
- Editing a watched resource now triggers a reload (that pauses requests received during the reload) rather than a stop/start which will return 404s for requests received while the context is stopping and starting

As with previous iterations of this feature it is impossible to completely eliminate the chances of a 404 without a fair amount of locking that would slow things down unnecessarily in production.

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
    tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java
    tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
    tomcat/trunk/java/org/apache/catalina/core/StandardContextValve.java
    tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1335692&r1=1335691&r2=1335692&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Tue May  8 18:54:44 2012
@@ -719,7 +719,19 @@ public class CoyoteAdapter implements Ad
                     }
                 }
             }
-
+            if (!mapRequired && request.getContext().getPaused()) {
+                // Found a matching context but it is paused. Mapping data will
+                // be wrong since some Wrappers may not be registered at this
+                // point.
+                try {
+                    Thread.sleep(1000);
+                } catch (InterruptedException e) {
+                    // Should never happen
+                }
+                // Reset mapping
+                request.getMappingData().recycle();
+                mapRequired = true;
+            }
         }
 
         // Possible redirect

Modified: tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java?rev=1335692&r1=1335691&r2=1335692&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/MapperListener.java Tue May  8 18:54:44 2012
@@ -437,7 +437,12 @@ public class MapperListener extends Life
             if (obj instanceof Wrapper) {
                 unregisterWrapper((Wrapper) obj);
             } else if (obj instanceof Context) {
-                unregisterContext((Context) obj);
+                Context c = (Context) obj;
+                // Only unregister if not paused. If paused, need to keep
+                // registration in place to prevent 404's during reload
+                if (!c.getPaused()) {
+                    unregisterContext(c);
+                }
             } else if (obj instanceof Host) {
                 unregisterHost((Host) obj);
             }

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContext.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContext.java?rev=1335692&r1=1335691&r2=1335692&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContext.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContext.java Tue May  8 18:54:44 2012
@@ -3767,9 +3767,12 @@ public class StandardContext extends Con
      * <p>
      * <b>IMPLEMENTATION NOTE</b>:  This method is designed to deal with
      * reloads required by changes to classes in the underlying repositories
-     * of our class loader.  It does not handle changes to the web application
-     * deployment descriptor.  If that has occurred, you should stop this
-     * Context and create (and start) a new Context instance instead.
+     * of our class loader and changes to the web.xml file. It does not handle
+     * changes to any context.xml file. If the context.xml has changed, you
+     * should stop this Context and create (and start) a new Context instance
+     * instead. Note that there is additional code in
+     * <code>CoyoteAdapter#postParseRequest()</code> to handle mapping requests
+     * to paused Contexts.
      *
      * @exception IllegalStateException if the <code>reloadable</code>
      *  property is set to <code>false</code>.
@@ -3786,7 +3789,7 @@ public class StandardContext extends Con
             log.info(sm.getString("standardContext.reloadingStarted",
                     getName()));
 
-        // Stop accepting requests temporarily
+        // Stop accepting requests temporarily.
         setPaused(true);
 
         try {

Modified: tomcat/trunk/java/org/apache/catalina/core/StandardContextValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/StandardContextValve.java?rev=1335692&r1=1335691&r2=1335692&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/StandardContextValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/StandardContextValve.java Tue May  8 18:54:44 2012
@@ -14,11 +14,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-
-
 package org.apache.catalina.core;
 
-
 import java.io.IOException;
 
 import javax.servlet.RequestDispatcher;
@@ -26,7 +23,6 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.catalina.Container;
-import org.apache.catalina.Context;
 import org.apache.catalina.Wrapper;
 import org.apache.catalina.comet.CometEvent;
 import org.apache.catalina.connector.Request;
@@ -44,22 +40,13 @@ import org.apache.tomcat.util.buf.Messag
  * @author Craig R. McClanahan
  * @version $Id$
  */
-
 final class StandardContextValve extends ValveBase {
 
-    //------------------------------------------------------ Constructor
     public StandardContextValve() {
         super(true);
     }
 
 
-    // ----------------------------------------------------- Instance Variables
-
-    private Context context = null;
-
-
-    // --------------------------------------------------------- Public Methods
-
     /**
      * Cast to a StandardContext right away, as it will be needed later.
      *
@@ -68,7 +55,6 @@ final class StandardContextValve extends
     @Override
     public void setContainer(Container container) {
         super.setContainer(container);
-        context = (Context) container;
     }
 
 
@@ -97,38 +83,11 @@ final class StandardContextValve extends
             return;
         }
 
-        // Wait if we are reloading
-        boolean reloaded = false;
-        while (context.getPaused()) {
-            reloaded = true;
-            try {
-                Thread.sleep(1000);
-            } catch (InterruptedException e) {
-                // Ignore
-            }
-        }
-
-        // Reloading will have stopped the old webappclassloader and
-        // created a new one
-        if (reloaded &&
-                context.getLoader() != null &&
-                context.getLoader().getClassLoader() != null) {
-            Thread.currentThread().setContextClassLoader(
-                    context.getLoader().getClassLoader());
-        }
-
         // Select the Wrapper to be used for this Request
         Wrapper wrapper = request.getWrapper();
-        if (wrapper == null) {
+        if (wrapper == null || wrapper.isUnavailable()) {
             response.sendError(HttpServletResponse.SC_NOT_FOUND);
             return;
-        } else if (wrapper.isUnavailable()) {
-            // May be as a result of a reload, try and find the new wrapper
-            wrapper = (Wrapper) container.findChild(wrapper.getName());
-            if (wrapper == null) {
-                response.sendError(HttpServletResponse.SC_NOT_FOUND);
-                return;
-            }
         }
 
         // Acknowledge the request

Modified: tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java?rev=1335692&r1=1335691&r2=1335692&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/HostConfig.java Tue May  8 18:54:44 2012
@@ -1325,23 +1325,19 @@ public class HostConfig
                 // Reload application
                 if(log.isInfoEnabled())
                     log.info(sm.getString("hostConfig.reload", app.name));
-                Container context = host.findChild(app.name);
-                try {
-                    // Might not have started if start failed last time
-                    if (context.getState().isAvailable()) {
-                        context.stop();
+                Context context = (Context) host.findChild(app.name);
+                if (context.getState().isAvailable()) {
+                    // Reload catches and logs exceptions
+                    context.reload();
+                } else {
+                    // If the context was not started (for example an error
+                    // in web.xml) we'll still get to try to start
+                    try {
+                        context.start();
+                    } catch (Exception e) {
+                        log.warn(sm.getString
+                                 ("hostConfig.context.restart", app.name), e);
                     }
-                } catch (Exception e) {
-                    log.warn(sm.getString
-                             ("hostConfig.context.restart", app.name), e);
-                }
-                // If the context was not started (for example an error
-                // in web.xml) we'll still get to try to start
-                try {
-                    context.start();
-                } catch (Exception e) {
-                    log.warn(sm.getString
-                             ("hostConfig.context.restart", app.name), e);
                 }
                 // Update times
                 app.reloadResources.put(resources[i],



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