On 13/11/2012 14:06, Konstantin Kolinko wrote: > 2012/11/13 Mark Thomas <ma...@apache.org>: >> On 13/11/2012 00:57, Konstantin Kolinko wrote: >>> 2012/11/12 <ma...@apache.org>: >>>> Author: markt >>>> Date: Sun Nov 11 23:35:41 2012 >>>> New Revision: 1408152 >>>> >>>> URL: http://svn.apache.org/viewvc?rev=1408152&view=rev >>>> Log: >>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54123 >>>> There are two things going on here: >>>> 1. The reported bug. If there is a async timeout and no async listeners, >>>> trigger a 500 response. >>>> 2. Implement "error dispatch". This is used a couple of times in the spec >>>> without any definition. The implication from the part of the spec quoted >>>> in the bug report is: >>>> - The standard error page mechanism should be used to identify the page >>>> - An async request that has been started should be left in async mode >>>> when forwarding to the error page >>>> - The error page may call complete() or dispatch() >>>> This commit hooks into the StandardHostValve to access the error page >>>> mechanism. I could have copied and pasted but I preferred the dependency >>>> on StandardHostValve >>>> Because the error page may do a dispatch(), need to ensure that when >>>> running the dispatch(), the error page mechanism is not triggered a second >>>> time. >>>> Depending on what emerges running the full unit tests and the TCK, I mat >>>> still decide to copy the error page code to AsyncContextImpl >>>> >>>> Modified: >>>> tomcat/tc7.0.x/trunk/ (props changed) >>>> >>>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java >>>> >>>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/StandardHostValve.java >>>> tomcat/tc7.0.x/trunk/java/org/apache/coyote/AsyncStateMachine.java >>>> >>>> tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java >>>> tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml >>>> >>>> Propchange: tomcat/tc7.0.x/trunk/ >>>> ------------------------------------------------------------------------------ >>>> Merged /tomcat/trunk:r1408148 >>>> >>>> Modified: >>>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java >>>> URL: >>>> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1408152&r1=1408151&r2=1408152&view=diff >>>> ============================================================================== >>>> --- >>>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java >>>> (original) >>>> +++ >>>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/core/AsyncContextImpl.java >>>> Sun Nov 11 23:35:41 2012 >>>> @@ -41,6 +41,8 @@ import javax.servlet.http.HttpServletRes >>>> import org.apache.catalina.AsyncDispatcher; >>>> import org.apache.catalina.Context; >>>> import org.apache.catalina.Globals; >>>> +import org.apache.catalina.Host; >>>> +import org.apache.catalina.Valve; >>>> import org.apache.catalina.connector.Request; >>>> import org.apache.coyote.ActionCode; >>>> import org.apache.coyote.AsyncContextCallback; >>>> @@ -128,8 +130,8 @@ public class AsyncContextImpl implements >>>> ActionCode.ASYNC_IS_TIMINGOUT, result); >>>> return !result.get(); >>>> } else { >>>> - // No listeners, container calls complete >>>> - complete(); >>>> + // No listeners, trigger error handling >>>> + return false; >>>> } >>>> >>>> } finally { >>>> @@ -372,6 +374,23 @@ public class AsyncContextImpl implements >>>> listener.getClass().getName() + "]", ioe); >>>> } >>>> } >>> >>> >>> The spec in 2.3.3.3 says "If none of the listeners called >>> AsyncContext.complete or any of the AsyncContext.dispatch methods, >>> then perform an error dispatch...." >>> >>> As far as I see, the code below is executed unconditionally, but the >>> spec says that it has to be executed only if the listeners did not do >>> the calls.... >> >> The code below is part of AsyncContextImpl.setErrorState() and that is >> only called from timeout() if there is no configured listener that calls >> complete() or dispatch(). >> > > I am not convinced. > > I do not see other places where AsyncListenerWrapper.fireOnError(..) > were called. So it is the only place that calls onError() on those > listeners. > > Where is it checking that "none of the listeners called > AsyncContext.complete or any of the AsyncContext.dispatch methods" in > their onError() methods? Why does it go on to call > StandardHostValve.throwable(,,) in case the error has already been > handled by the listener? > > SRV.2.3.3.3 > Points "i" and "ii" on page 38 of 230. (numbered page "16" at the > bottom of the page). > servlet-3_0-mrel-spec.pdf
OK. I see where you are coming from. This needs some more work. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
