tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r491619 - in /tomcat/tc6.0.x/trunk: java/org/apache/catalina/core/ApplicationDispatcher.java webapps/docs/changelog.xml
Date Mon, 01 Jan 2007 19:31:30 GMT
Author: markt
Date: Mon Jan  1 11:31:29 2007
New Revision: 491619

URL: http://svn.apache.org/viewvc?view=rev&rev=491619
Log:
Port RD thread safety patch from TC5

Modified:
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java?view=diff&rev=491619&r1=491618&r2=491619
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/core/ApplicationDispatcher.java Mon Jan
 1 11:31:29 2007
@@ -100,6 +100,48 @@
         }
     }
 
+    
+    /**
+     * Used to pass state when the request dispatcher is used. Using instance
+     * variables causes threading issues and state is too complex to pass and
+     * return single ServletRequest or ServletResponse objects.
+     */
+    private class State {
+        State(ServletRequest request, ServletResponse response,
+                boolean including) {
+            this.outerRequest = request;
+            this.outerResponse = response;
+            this.including = including;
+        }
+
+        /**
+         * The outermost request that will be passed on to the invoked servlet.
+         */
+        ServletRequest outerRequest = null;
+
+
+        /**
+         * The outermost response that will be passed on to the invoked servlet.
+         */
+        ServletResponse outerResponse = null;
+        
+        /**
+         * The request wrapper we have created and installed (if any).
+         */
+        ServletRequest wrapRequest = null;
+
+
+        /**
+         * The response wrapper we have created and installed (if any).
+         */
+        ServletResponse wrapResponse = null;
+        
+        /**
+         * Are we performing an include() instead of a forward()?
+         */
+        boolean including = false;
+    }
+
     // ----------------------------------------------------------- Constructors
 
 
@@ -131,7 +173,6 @@
         this.context = (Context) wrapper.getParent();
         this.requestURI = requestURI;
         this.servletPath = servletPath;
-        this.origServletPath = servletPath;
         this.pathInfo = pathInfo;
         this.queryString = queryString;
         this.name = name;
@@ -153,30 +194,12 @@
     private static Log log = LogFactory.getLog(ApplicationDispatcher.class);
 
     /**
-     * The request specified by the dispatching application.
-     */
-    private ServletRequest appRequest = null;
-
-
-    /**
-     * The response specified by the dispatching application.
-     */
-    private ServletResponse appResponse = null;
-
-
-    /**
      * The Context this RequestDispatcher is associated with.
      */
     private Context context = null;
 
 
     /**
-     * Are we performing an include() instead of a forward()?
-     */
-    private boolean including = false;
-
-
-    /**
      * Descriptive information about this implementation.
      */
     private static final String info =
@@ -190,18 +213,6 @@
 
 
     /**
-     * The outermost request that will be passed on to the invoked servlet.
-     */
-    private ServletRequest outerRequest = null;
-
-
-    /**
-     * The outermost response that will be passed on to the invoked servlet.
-     */
-    private ServletResponse outerResponse = null;
-
-
-    /**
      * The extra path information for this RequestDispatcher.
      */
     private String pathInfo = null;
@@ -218,13 +229,13 @@
      */
     private String requestURI = null;
 
+
     /**
      * The servlet path for this RequestDispatcher.
      */
     private String servletPath = null;
 
-    private String origServletPath = null;
-    
+
     /**
      * The StringManager for this package.
      */
@@ -246,18 +257,6 @@
     private Wrapper wrapper = null;
 
 
-    /**
-     * The request wrapper we have created and installed (if any).
-     */
-    private ServletRequest wrapRequest = null;
-
-
-    /**
-     * The response wrapper we have created and installed (if any).
-     */
-    private ServletResponse wrapResponse = null;
-
-
     // ------------------------------------------------------------- Properties
 
 
@@ -323,11 +322,11 @@
         }
 
         // Set up to handle the specified request and response
-        setup(request, response, false);
+        State state = new State(request, response, false);
 
         if (Globals.STRICT_SERVLET_COMPLIANCE) {
             // Check SRV.8.2 / SRV.14.2.5.1 compliance
-            checkSameObjects();
+            checkSameObjects(request, response);
         }
 
         // Identify the HTTP-specific request and response objects (if any)
@@ -344,7 +343,7 @@
             if ( log.isDebugEnabled() )
                 log.debug(" Non-HTTP Forward");
             
-            processRequest(hrequest,hresponse);
+            processRequest(hrequest,hresponse,state);
 
         }
 
@@ -355,17 +354,17 @@
                 log.debug(" Named Dispatcher Forward");
             
             ApplicationHttpRequest wrequest =
-                (ApplicationHttpRequest) wrapRequest();
+                (ApplicationHttpRequest) wrapRequest(state);
             wrequest.setRequestURI(hrequest.getRequestURI());
             wrequest.setContextPath(hrequest.getContextPath());
             wrequest.setServletPath(hrequest.getServletPath());
             wrequest.setPathInfo(hrequest.getPathInfo());
             wrequest.setQueryString(hrequest.getQueryString());
 
-            processRequest(request,response);
+            processRequest(request,response,state);
 
             wrequest.recycle();
-            unwrapRequest();
+            unwrapRequest(state);
 
         }
 
@@ -376,7 +375,7 @@
                 log.debug(" Path Based Forward");
 
             ApplicationHttpRequest wrequest =
-                (ApplicationHttpRequest) wrapRequest();
+                (ApplicationHttpRequest) wrapRequest(state);
             String contextPath = context.getPath();
 
             if (hrequest.getAttribute(Globals.FORWARD_REQUEST_URI_ATTR) == null) {
@@ -401,10 +400,10 @@
                 wrequest.setQueryParams(queryString);
             }
 
-            processRequest(request,response);
+            processRequest(request,response,state);
 
             wrequest.recycle();
-            unwrapRequest();
+            unwrapRequest(state);
 
         }
 
@@ -443,32 +442,33 @@
     }
 
     
-
     /**
      * Prepare the request based on the filter configuration.
      * @param request The servlet request we are processing
      * @param response The servlet response we are creating
+     * @param state The RD state
      *
      * @exception IOException if an input/output error occurs
      * @exception ServletException if a servlet error occurs
      */
     private void processRequest(ServletRequest request, 
-                                ServletResponse response)
+                                ServletResponse response,
+                                State state)
         throws IOException, ServletException {
                 
         Integer disInt = (Integer) request.getAttribute
             (ApplicationFilterFactory.DISPATCHER_TYPE_ATTR);
         if (disInt != null) {
             if (disInt.intValue() != ApplicationFilterFactory.ERROR) {
-                outerRequest.setAttribute
+                state.outerRequest.setAttribute
                     (ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
-                     origServletPath);
-                outerRequest.setAttribute
+                     servletPath);
+                state.outerRequest.setAttribute
                     (ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
                      Integer.valueOf(ApplicationFilterFactory.FORWARD));
-                invoke(outerRequest, response);
+                invoke(state.outerRequest, response, state);
             } else {
-                invoke(outerRequest, response);
+                invoke(state.outerRequest, response, state);
             }
         }
 
@@ -510,16 +510,15 @@
         throws ServletException, IOException
     {
         // Set up to handle the specified request and response
-        setup(request, response, true);
+        State state = new State(request, response, true);
 
         if (Globals.STRICT_SERVLET_COMPLIANCE) {
             // Check SRV.8.2 / SRV.14.2.5.1 compliance
-            checkSameObjects();
+            checkSameObjects(request, response);
         }
         
         // Create a wrapped response to use for this request
-        // ServletResponse wresponse = null;
-        ServletResponse wresponse = wrapResponse();
+        wrapResponse(state);
 
         // Handle a non-HTTP include
         if (!(request instanceof HttpServletRequest) ||
@@ -529,8 +528,10 @@
                 log.debug(" Non-HTTP Include");
             request.setAttribute(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
                     Integer.valueOf(ApplicationFilterFactory.INCLUDE));
-            request.setAttribute(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR, origServletPath);
-            invoke(request, outerResponse);
+            request.setAttribute(
+                    ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
+                    servletPath);
+            invoke(request, state.outerResponse, state);
         }
 
         // Handle an HTTP named dispatcher include
@@ -540,14 +541,16 @@
                 log.debug(" Named Dispatcher Include");
 
             ApplicationHttpRequest wrequest =
-                (ApplicationHttpRequest) wrapRequest();
+                (ApplicationHttpRequest) wrapRequest(state);
             wrequest.setAttribute(Globals.NAMED_DISPATCHER_ATTR, name);
             if (servletPath != null)
                 wrequest.setServletPath(servletPath);
             wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
                     Integer.valueOf(ApplicationFilterFactory.INCLUDE));
-            wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
origServletPath);
-            invoke(outerRequest, outerResponse);
+            wrequest.setAttribute(
+                    ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
+                    servletPath);
+            invoke(state.outerRequest, state.outerResponse, state);
 
             wrequest.recycle();
         }
@@ -559,7 +562,7 @@
                 log.debug(" Path Based Include");
 
             ApplicationHttpRequest wrequest =
-                (ApplicationHttpRequest) wrapRequest();
+                (ApplicationHttpRequest) wrapRequest(state);
             String contextPath = context.getPath();
             if (requestURI != null)
                 wrequest.setAttribute(Globals.INCLUDE_REQUEST_URI_ATTR,
@@ -581,8 +584,10 @@
             
             wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
                     Integer.valueOf(ApplicationFilterFactory.INCLUDE));
-            wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
origServletPath);
-            invoke(outerRequest, outerResponse);
+            wrequest.setAttribute(
+                    ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
+                    servletPath);
+            invoke(state.outerRequest, state.outerResponse, state);
 
             wrequest.recycle();
         }
@@ -608,8 +613,8 @@
      * @exception IOException if an input/output error occurs
      * @exception ServletException if a servlet error occurs
      */
-    private void invoke(ServletRequest request, ServletResponse response)
-            throws IOException, ServletException {
+    private void invoke(ServletRequest request, ServletResponse response,
+            State state) throws IOException, ServletException {
 
         // Checking to see if the context classloader is the current context
         // classloader. If it's not, we're saving it, and setting the context
@@ -624,7 +629,6 @@
         }
 
         // Initialize local variables we may need
-        HttpServletRequest hrequest = (HttpServletRequest) request;
         HttpServletResponse hresponse = (HttpServletResponse) response;
         Servlet servlet = null;
         IOException ioException = null;
@@ -758,8 +762,8 @@
         
         // Unwrap request/response if needed
         // See Bugzilla 30949
-        unwrapRequest();
-        unwrapResponse();
+        unwrapRequest(state);
+        unwrapResponse(state);
 
         // Rethrow an exception if one was thrown by the invoked servlet
         if (ioException != null)
@@ -773,35 +777,15 @@
 
 
     /**
-     * Set up to handle the specified request and response
-     *
-     * @param request The servlet request specified by the caller
-     * @param response The servlet response specified by the caller
-     * @param including Are we performing an include() as opposed to
-     *  a forward()?
-     */
-    private void setup(ServletRequest request, ServletResponse response,
-                       boolean including) {
-
-        this.appRequest = request;
-        this.appResponse = response;
-        this.outerRequest = request;
-        this.outerResponse = response;
-        this.including = including;
-
-    }
-
-
-    /**
      * Unwrap the request if we have wrapped it.
      */
-    private void unwrapRequest() {
+    private void unwrapRequest(State state) {
 
-        if (wrapRequest == null)
+        if (state.wrapRequest == null)
             return;
 
         ServletRequest previous = null;
-        ServletRequest current = outerRequest;
+        ServletRequest current = state.outerRequest;
         while (current != null) {
 
             // If we run into the container request we are done
@@ -810,11 +794,11 @@
                 break;
 
             // Remove the current request if it is our wrapper
-            if (current == wrapRequest) {
+            if (current == state.wrapRequest) {
                 ServletRequest next =
                   ((ServletRequestWrapper) current).getRequest();
                 if (previous == null)
-                    outerRequest = next;
+                    state.outerRequest = next;
                 else
                     ((ServletRequestWrapper) previous).setRequest(next);
                 break;
@@ -832,13 +816,13 @@
     /**
      * Unwrap the response if we have wrapped it.
      */
-    private void unwrapResponse() {
+    private void unwrapResponse(State state) {
 
-        if (wrapResponse == null)
+        if (state.wrapResponse == null)
             return;
 
         ServletResponse previous = null;
-        ServletResponse current = outerResponse;
+        ServletResponse current = state.outerResponse;
         while (current != null) {
 
             // If we run into the container response we are done
@@ -847,11 +831,11 @@
                 break;
 
             // Remove the current response if it is our wrapper
-            if (current == wrapResponse) {
+            if (current == state.wrapResponse) {
                 ServletResponse next =
                   ((ServletResponseWrapper) current).getResponse();
                 if (previous == null)
-                    outerResponse = next;
+                    state.outerResponse = next;
                 else
                     ((ServletResponseWrapper) previous).setResponse(next);
                 break;
@@ -870,11 +854,11 @@
      * Create and return a request wrapper that has been inserted in the
      * appropriate spot in the request chain.
      */
-    private ServletRequest wrapRequest() {
+    private ServletRequest wrapRequest(State state) {
 
         // Locate the request we should insert in front of
         ServletRequest previous = null;
-        ServletRequest current = outerRequest;
+        ServletRequest current = state.outerRequest;
         while (current != null) {
             if ("org.apache.catalina.servlets.InvokerHttpRequest".
                 equals(current.getClass().getName()))
@@ -899,11 +883,11 @@
             // Compute a crossContext flag
             HttpServletRequest hcurrent = (HttpServletRequest) current;
             boolean crossContext = false;
-            if ((outerRequest instanceof ApplicationHttpRequest) ||
-                (outerRequest instanceof Request) ||
-                (outerRequest instanceof HttpServletRequest)) {
+            if ((state.outerRequest instanceof ApplicationHttpRequest) ||
+                (state.outerRequest instanceof Request) ||
+                (state.outerRequest instanceof HttpServletRequest)) {
                 HttpServletRequest houterRequest = 
-                    (HttpServletRequest) outerRequest;
+                    (HttpServletRequest) state.outerRequest;
                 Object contextPath = houterRequest.getAttribute
                     (Globals.INCLUDE_CONTEXT_PATH_ATTR);
                 if (contextPath == null) {
@@ -918,10 +902,10 @@
             wrapper = new ApplicationRequest(current);
         }
         if (previous == null)
-            outerRequest = wrapper;
+            state.outerRequest = wrapper;
         else
             ((ServletRequestWrapper) previous).setRequest(wrapper);
-        wrapRequest = wrapper;
+        state.wrapRequest = wrapper;
         return (wrapper);
 
     }
@@ -931,11 +915,11 @@
      * Create and return a response wrapper that has been inserted in the
      * appropriate spot in the response chain.
      */
-    private ServletResponse wrapResponse() {
+    private ServletResponse wrapResponse(State state) {
 
         // Locate the response we should insert in front of
         ServletResponse previous = null;
-        ServletResponse current = outerResponse;
+        ServletResponse current = state.outerResponse;
         while (current != null) {
             if (!(current instanceof ServletResponseWrapper))
                 break;
@@ -956,21 +940,24 @@
             (current instanceof HttpServletResponse))
             wrapper =
                 new ApplicationHttpResponse((HttpServletResponse) current,
-                                            including);
+                        state.including);
         else
-            wrapper = new ApplicationResponse(current, including);
+            wrapper = new ApplicationResponse(current, state.including);
         if (previous == null)
-            outerResponse = wrapper;
+            state.outerResponse = wrapper;
         else
             ((ServletResponseWrapper) previous).setResponse(wrapper);
-        wrapResponse = wrapper;
+        state.wrapResponse = wrapper;
         return (wrapper);
 
     }
 
-    private void checkSameObjects() throws ServletException {
-        ServletRequest originalRequest = ApplicationFilterChain.getLastServicedRequest();
-        ServletResponse originalResponse = ApplicationFilterChain.getLastServicedResponse();
+    private void checkSameObjects(ServletRequest appRequest,
+            ServletResponse appResponse) throws ServletException {
+        ServletRequest originalRequest =
+            ApplicationFilterChain.getLastServicedRequest();
+        ServletResponse originalResponse =
+            ApplicationFilterChain.getLastServicedResponse();
         
         // Some forwards, eg from valves will not set original values 
         if (originalRequest == null || originalResponse == null) {
@@ -980,9 +967,11 @@
         boolean same = false;
         ServletRequest dispatchedRequest = appRequest;
         
-        //find the bottom most request, the one that was passed into the service method
-        while ( originalRequest instanceof ServletRequestWrapper && ((ServletRequestWrapper)
originalRequest).getRequest()!=null ) {
-            originalRequest = ((ServletRequestWrapper) originalRequest).getRequest();
+        //find the request that was passed into the service method
+        while (originalRequest instanceof ServletRequestWrapper &&
+                ((ServletRequestWrapper) originalRequest).getRequest()!=null ) {
+            originalRequest =
+                ((ServletRequestWrapper) originalRequest).getRequest();
         }
         //compare with the dispatched request
         while (!same) {
@@ -990,21 +979,26 @@
                 same = true;
             }
             if (!same && dispatchedRequest instanceof ServletRequestWrapper) {
-                dispatchedRequest = ((ServletRequestWrapper) dispatchedRequest).getRequest();
+                dispatchedRequest =
+                    ((ServletRequestWrapper) dispatchedRequest).getRequest();
             } else {
                 break;
             }
         }
         if (!same) {
-            throw new ServletException(sm.getString("applicationDispatcher.specViolation.request"));
+            throw new ServletException(sm.getString(
+                    "applicationDispatcher.specViolation.request"));
         }
         
         same = false;
         ServletResponse dispatchedResponse = appResponse;
         
-        //find the bottom most response, the one that was passed into the service method
-        while ( originalResponse instanceof ServletResponseWrapper && ((ServletResponseWrapper)
originalResponse).getResponse()!=null ) {
-            originalResponse = ((ServletResponseWrapper) originalResponse).getResponse();
+        //find the response that was passed into the service method
+        while (originalResponse instanceof ServletResponseWrapper &&
+                ((ServletResponseWrapper) originalResponse).getResponse() != 
+                    null ) {
+            originalResponse =
+                ((ServletResponseWrapper) originalResponse).getResponse();
         }
         //compare with the dispatched response
         while (!same) {
@@ -1013,14 +1007,16 @@
             }
             
             if (!same && dispatchedResponse instanceof ServletResponseWrapper) {
-                dispatchedResponse = ((ServletResponseWrapper) dispatchedResponse).getResponse();
+                dispatchedResponse =
+                    ((ServletResponseWrapper) dispatchedResponse).getResponse();
             } else {
                 break;
             }
         }
 
         if (!same) {
-            throw new ServletException(sm.getString("applicationDispatcher.specViolation.response"));
+            throw new ServletException(sm.getString(
+                    "applicationDispatcher.specViolation.response"));
         }
     }
 }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?view=diff&rev=491619&r1=491618&r2=491619
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Mon Jan  1 11:31:29 2007
@@ -15,6 +15,13 @@
 
 <body>
 <section name="Tomcat 6.0.8 (remm)">
+  <subsection name="Catalina">
+    <changelog>
+      <fix>
+        Make provided instances of RequestDispatcher thread safe. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Jasper">
     <changelog>
       <fix>



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


Mime
View raw message