Return-Path: Delivered-To: apmail-tomcat-dev-archive@www.apache.org Received: (qmail 3797 invoked from network); 1 Jan 2007 19:32:34 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 1 Jan 2007 19:32:34 -0000 Received: (qmail 12460 invoked by uid 500); 1 Jan 2007 19:32:33 -0000 Delivered-To: apmail-tomcat-dev-archive@tomcat.apache.org Received: (qmail 12413 invoked by uid 500); 1 Jan 2007 19:32:33 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 12402 invoked by uid 500); 1 Jan 2007 19:32:33 -0000 Delivered-To: apmail-jakarta-tomcat-dev@jakarta.apache.org Received: (qmail 12399 invoked by uid 99); 1 Jan 2007 19:32:33 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 01 Jan 2007 11:32:33 -0800 X-ASF-Spam-Status: No, hits=-9.4 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 01 Jan 2007 11:32:25 -0800 Received: by eris.apache.org (Postfix, from userid 65534) id 61FF81A981A; Mon, 1 Jan 2007 11:31:30 -0800 (PST) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit 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 -0000 To: tomcat-dev@jakarta.apache.org From: markt@apache.org X-Mailer: svnmailer-1.1.0 Message-Id: <20070101193130.61FF81A981A@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 @@
+ + + + Make provided instances of RequestDispatcher thread safe. (markt) + + + --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org