Author: markt
Date: Sat Dec 23 10:07:34 2006
New Revision: 489910
URL: http://svn.apache.org/viewvc?view=rev&rev=489910
Log:
Make ApplicationDispatcher thread safe. After a very long thread on the users
list (http://marc.theaimsgroup.com/?t=116558790000003&r=1&w=2) this was found
to be the root of the problem.
There was also some debate if this re-use of the RequestDispatcher was valid.
The spec is not clear on this point.
This change should be functionality identical to the previous version. It uses
additional local variables and method parameters rather than instance variables.
Some fields were unnecessary duplicates and have been removed.
Modified:
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/ApplicationDispatcher.java
tomcat/container/tc5.5.x/webapps/docs/changelog.xml
Modified:
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/ApplicationDispatcher.java
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/ApplicationDispatcher.java?view=diff&rev=489910&r1=489909&r2=489910
==============================================================================
---
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/ApplicationDispatcher.java
(original)
+++
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/core/ApplicationDispatcher.java
Sat Dec 23 10:07:34 2006
@@ -131,7 +131,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 +152,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 +171,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 +187,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 +215,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 +280,12 @@
}
// Set up to handle the specified request and response
- setup(request, response, false);
-
+ ServletRequest outerRequest = request;
+ ServletResponse outerResponse = response;
+
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)
@@ -343,8 +301,9 @@
if ( log.isDebugEnabled() )
log.debug(" Non-HTTP Forward");
-
- processRequest(hrequest,hresponse);
+ // TODO - this doesn't appear to agree with the comments above
+ processRequest(hrequest,hresponse,outerRequest,outerResponse,null,
+ null);
}
@@ -355,17 +314,18 @@
log.debug(" Named Dispatcher Forward");
ApplicationHttpRequest wrequest =
- (ApplicationHttpRequest) wrapRequest();
+ (ApplicationHttpRequest) wrapRequest(outerRequest);
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,outerRequest,outerResponse,
+ wrequest,null);
wrequest.recycle();
- unwrapRequest();
+ unwrapRequest(outerRequest, wrequest);
}
@@ -376,7 +336,7 @@
log.debug(" Path Based Forward");
ApplicationHttpRequest wrequest =
- (ApplicationHttpRequest) wrapRequest();
+ (ApplicationHttpRequest) wrapRequest(outerRequest);
String contextPath = context.getPath();
if (hrequest.getAttribute(Globals.FORWARD_REQUEST_URI_ATTR) ==
null) {
@@ -401,10 +361,11 @@
wrequest.setQueryParams(queryString);
}
- processRequest(request,response);
+
processRequest(request,response,outerRequest,outerResponse,wrequest,
+ null);
wrequest.recycle();
- unwrapRequest();
+ unwrapRequest(outerRequest, wrequest);
}
@@ -453,7 +414,11 @@
* @exception ServletException if a servlet error occurs
*/
private void processRequest(ServletRequest request,
- ServletResponse response)
+ ServletResponse response,
+ ServletRequest outerRequest,
+ ServletResponse outerResponse,
+ ServletRequest wrapRequest,
+ ServletResponse wrapResponse)
throws IOException, ServletException {
Integer disInt = (Integer) request.getAttribute
@@ -462,13 +427,15 @@
if (disInt.intValue() != ApplicationFilterFactory.ERROR) {
outerRequest.setAttribute
(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
- origServletPath);
+ servletPath);
outerRequest.setAttribute
(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
new Integer(ApplicationFilterFactory.FORWARD));
- invoke(outerRequest, response);
+ invoke(outerRequest, response, outerRequest, outerResponse,
+ wrapRequest, wrapResponse);
} else {
- invoke(outerRequest, response);
+ invoke(outerRequest, response, outerRequest, outerResponse,
+ wrapRequest, wrapResponse);
}
}
@@ -510,16 +477,17 @@
throws ServletException, IOException
{
// Set up to handle the specified request and response
- setup(request, response, true);
+ ServletRequest outerRequest = request;
+ ServletResponse outerResponse = response;
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;
- wrapResponse();
+ ServletResponse wresponse = wrapResponse(outerResponse, true);
// Handle a non-HTTP include
if (!(request instanceof HttpServletRequest) ||
@@ -528,9 +496,12 @@
if ( log.isDebugEnabled() )
log.debug(" Non-HTTP Include");
request.setAttribute(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
- new
Integer(ApplicationFilterFactory.INCLUDE));
-
request.setAttribute(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
origServletPath);
- invoke(request, outerResponse);
+ new Integer(ApplicationFilterFactory.INCLUDE));
+ request.setAttribute(
+ ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
+ servletPath);
+ invoke(request, outerResponse, outerRequest, outerResponse, null,
+ wresponse);
}
// Handle an HTTP named dispatcher include
@@ -540,14 +511,18 @@
log.debug(" Named Dispatcher Include");
ApplicationHttpRequest wrequest =
- (ApplicationHttpRequest) wrapRequest();
+ (ApplicationHttpRequest) wrapRequest(outerRequest);
wrequest.setAttribute(Globals.NAMED_DISPATCHER_ATTR, name);
if (servletPath != null)
wrequest.setServletPath(servletPath);
-
wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
- new
Integer(ApplicationFilterFactory.INCLUDE));
-
wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
origServletPath);
- invoke(outerRequest, outerResponse);
+ wrequest.setAttribute(
+ ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
+ new Integer(ApplicationFilterFactory.INCLUDE));
+ wrequest.setAttribute(
+ ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
+ servletPath);
+ invoke(outerRequest, outerResponse, outerRequest, outerResponse,
+ wrequest, wresponse);
wrequest.recycle();
}
@@ -559,7 +534,7 @@
log.debug(" Path Based Include");
ApplicationHttpRequest wrequest =
- (ApplicationHttpRequest) wrapRequest();
+ (ApplicationHttpRequest) wrapRequest(outerRequest);
String contextPath = context.getPath();
if (requestURI != null)
wrequest.setAttribute(Globals.INCLUDE_REQUEST_URI_ATTR,
@@ -569,7 +544,7 @@
contextPath);
if (servletPath != null)
wrequest.setAttribute(Globals.INCLUDE_SERVLET_PATH_ATTR,
- servletPath);
+ servletPath);
if (pathInfo != null)
wrequest.setAttribute(Globals.INCLUDE_PATH_INFO_ATTR,
pathInfo);
@@ -579,10 +554,14 @@
wrequest.setQueryParams(queryString);
}
-
wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
- new
Integer(ApplicationFilterFactory.INCLUDE));
-
wrequest.setAttribute(ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
origServletPath);
- invoke(outerRequest, outerResponse);
+ wrequest.setAttribute(
+ ApplicationFilterFactory.DISPATCHER_TYPE_ATTR,
+ new Integer(ApplicationFilterFactory.INCLUDE));
+ wrequest.setAttribute(
+ ApplicationFilterFactory.DISPATCHER_REQUEST_PATH_ATTR,
+ servletPath);
+ invoke(outerRequest, outerResponse, outerRequest, outerResponse,
+ wrequest, wresponse);
wrequest.recycle();
}
@@ -608,7 +587,9 @@
* @exception IOException if an input/output error occurs
* @exception ServletException if a servlet error occurs
*/
- private void invoke(ServletRequest request, ServletResponse response)
+ private void invoke(ServletRequest request, ServletResponse response,
+ ServletRequest outerRequest, ServletResponse outerResponse,
+ ServletRequest wrapRequest, ServletResponse wrapResponse)
throws IOException, ServletException {
// Checking to see if the context classloader is the current context
@@ -651,12 +632,14 @@
servlet = wrapper.allocate();
}
} catch (ServletException e) {
-
wrapper.getLogger().error(sm.getString("applicationDispatcher.allocateException",
+ wrapper.getLogger().error(
+ sm.getString("applicationDispatcher.allocateException",
wrapper.getName()),
StandardWrapper.getRootCause(e));
servletException = e;
servlet = null;
} catch (Throwable e) {
-
wrapper.getLogger().error(sm.getString("applicationDispatcher.allocateException",
+ wrapper.getLogger().error(
+ sm.getString("applicationDispatcher.allocateException",
wrapper.getName()), e);
servletException = new ServletException
(sm.getString("applicationDispatcher.allocateException",
@@ -666,8 +649,8 @@
// Get the FilterChain Here
ApplicationFilterFactory factory =
ApplicationFilterFactory.getInstance();
- ApplicationFilterChain filterChain = factory.createFilterChain(request,
-
wrapper,servlet);
+ ApplicationFilterChain filterChain =
+ factory.createFilterChain(request,wrapper,servlet);
// Call the service() method for the allocated servlet instance
try {
String jspFile = wrapper.getJspFile();
@@ -731,7 +714,8 @@
} catch (Throwable e) {
log.error(sm.getString("standardWrapper.releaseFilters",
wrapper.getName()), e);
- //FIXME Exception handling needs to be simpiler to what is in the
StandardWrapperValue
+ //FIXME Exception handling needs to be simpiler to what is in the
+ //StandardWrapperValue
}
// Deallocate the allocated servlet instance
@@ -740,11 +724,13 @@
wrapper.deallocate(servlet);
}
} catch (ServletException e) {
-
wrapper.getLogger().error(sm.getString("applicationDispatcher.deallocateException",
+ wrapper.getLogger().error(
+ sm.getString("applicationDispatcher.deallocateException",
wrapper.getName()), e);
servletException = e;
} catch (Throwable e) {
-
wrapper.getLogger().error(sm.getString("applicationDispatcher.deallocateException",
+ wrapper.getLogger().error(
+ sm.getString("applicationDispatcher.deallocateException",
wrapper.getName()), e);
servletException = new ServletException
(sm.getString("applicationDispatcher.deallocateException",
@@ -757,8 +743,8 @@
// Unwrap request/response if needed
// See Bugzilla 30949
- unwrapRequest();
- unwrapResponse();
+ unwrapRequest(outerRequest, wrapRequest);
+ unwrapResponse(outerResponse, wrapResponse);
// Rethrow an exception if one was thrown by the invoked servlet
if (ioException != null)
@@ -772,29 +758,10 @@
/**
- * 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(ServletRequest outerRequest,
+ ServletRequest wrapRequest) {
if (wrapRequest == null)
return;
@@ -831,7 +798,8 @@
/**
* Unwrap the response if we have wrapped it.
*/
- private void unwrapResponse() {
+ private void unwrapResponse(ServletResponse outerResponse,
+ ServletResponse wrapResponse) {
if (wrapResponse == null)
return;
@@ -869,7 +837,7 @@
* Create and return a request wrapper that has been inserted in the
* appropriate spot in the request chain.
*/
- private ServletRequest wrapRequest() {
+ private ServletRequest wrapRequest(ServletRequest outerRequest) {
// Locate the request we should insert in front of
ServletRequest previous = null;
@@ -920,7 +888,6 @@
outerRequest = wrapper;
else
((ServletRequestWrapper) previous).setRequest(wrapper);
- wrapRequest = wrapper;
return (wrapper);
}
@@ -930,7 +897,8 @@
* Create and return a response wrapper that has been inserted in the
* appropriate spot in the response chain.
*/
- private ServletResponse wrapResponse() {
+ private ServletResponse wrapResponse(ServletResponse outerResponse,
+ boolean including) {
// Locate the response we should insert in front of
ServletResponse previous = null;
@@ -962,13 +930,13 @@
outerResponse = wrapper;
else
((ServletResponseWrapper) previous).setResponse(wrapper);
- wrapResponse = wrapper;
return (wrapper);
}
- private void checkSameObjects() throws ServletException {
+ private void checkSameObjects(ServletRequest appRequest,
+ ServletResponse appResponse) throws ServletException {
ServletRequest originalRequest =
ApplicationFilterChain.getLastServicedRequest();
ServletResponse originalResponse =
Modified: tomcat/container/tc5.5.x/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/changelog.xml?view=diff&rev=489910&r1=489909&r2=489910
==============================================================================
--- tomcat/container/tc5.5.x/webapps/docs/changelog.xml (original)
+++ tomcat/container/tc5.5.x/webapps/docs/changelog.xml Sat Dec 23 10:07:34 2006
@@ -144,6 +144,9 @@
Ensure Accept-Language headers conform to RFC 2616. Ignore them if
they do not. (markt)
</fix>
+ <fix>
+ Make provided instances of RequestDispatcher thread safe. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
