On 11/02/2015 13:54, Romain Manni-Bucau wrote:
> that's it I think

OK. Patch applied to trunk and 8.0.x. Let us know how you get on.

Mark


> 
> If you want servlet code let's do it (wanted to avoid code in mail
> since I never manage to format it well):
> 
> 1) request 1 (start the async context) does:
> 
> // field context has type AsyncContext
> context = req.startAsync(req, resp);
> context.addListener(this); // not important for us but it is mainly to
> handle errors etc
> context.setTimeout(valueFromAnywhere);
> 
> 2) request 2 does in its thread:
> 
> // i skip some jaxrs impl internals like getting the response to push
> to the client for request 1 etc
> context.dispatch();
> 
> And that's all in term of servlet API then you go back in servlet
> chain and end on JAXRS servlet (or filter) which identified this
> dispatch as a continuation and returns the expected data instead of
> doing again request 1 call.
> 
> 
> In term of tomcat calls we end up in
> org.apache.catalina.core.AsyncContextImpl#dispatch(javax.servlet.ServletContext,
> java.lang.String) and then
> this.request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCH,
> null);.
> 
> Finally we are in
> org.apache.coyote.http11.AbstractHttp11Processor#action which does
> 
> if (asyncStateMachine.asyncDispatch()) {
>     endpoint.processSocket(this.socketWrapper, SocketStatus.OPEN_READ, true);
> }
> 
> and org.apache.coyote.AsyncStateMachine#asyncDispatch checks
> ContainerThreadMarker.isContainerThread() which is obviously true
> since request 2 is a normal request so dispatch is skipped and
> response is never sent (well not as expected and we get a timeout).
> 
> 
> 
> Romain Manni-Bucau
> @rmannibucau
> http://www.tomitribe.com
> http://rmannibucau.wordpress.com
> https://github.com/rmannibucau
> 
> 
> 2015-02-11 14:27 GMT+01:00 Mark Thomas <ma...@apache.org>:
>> On 11/02/2015 09:35, Romain Manni-Bucau wrote:
>>> Ok, let's look a jaxrs 2 sample and bind underlying implementation:
>>>
>>>     @Path("touch")
>>>     @ApplicationScoped
>>>     public class Endpoint {
>>>         private volatile AsyncResponse current;
>>>
>>>         @GET
>>>         public void async(@Suspended final AsyncResponse response) {
>>>             if (current == null) {
>>>                 current = response;
>>>             } else {
>>>                 throw new IllegalStateException("we shouldnt go here
>>> back since");
>>>             }
>>>         }
>>>
>>>         @POST
>>>         @Path("answer")
>>>         public void async(final String response) {
>>>             current.resume(response /* response content */); // spec
>>> doesnt mandate a new thread here but tomcat does
>>>         }
>>>     }
>>>
>>> So we have 2 methods:
>>> 1) GET: this initiate a request. JAXRS implementation uses servlet
>>> container to get an AsyncContext no more
>>> 2) POST: the JAXRS AsyncResponse#resume method will just call servlet
>>> AsyncContext#dispatch
>>>
>>> Issue is:dispatch is called in the ThreadLocal environment/context of
>>> POST request so resume/dispatch inherit from it. Finally when you go
>>> to the async dispatch in Coyoterequest you have a marker bound in
>>> ContainerThreadMarker (the POST one) so resume/dispatch is ignored but
>>> actually it should work.
>>>
>>> Is it clearer?
>>
>> Not really for me. The description is assuming a familiarity with JAXRS2
>> that I don't have. Reading between the lines of what you have written I
>> think I have found a problem but I am not sure it is the problem you are
>> trying to describe.
>>
>> Consider the following which is described purely in terms of Servlet async:
>>
>> The first HTTP request (Req1) calls startAsync which creates the
>> AsyncContext (AC1), stashes AC1 somewhere and then returns.
>>
>> The client then waits for the response (Res1).
>>
>>
>> Some time later (but before the AsyncContext and client read timeout) a
>> new HTTP request (Req2) retrieves the AsyncContext and calls dispatch().
>>
>> This dispatch() *always* needs to be on a new container thread because
>> the current container thread is processing Req2/Res2 and the dispatch is
>> for Req1/Res1.
>>
>> The bug is the test introduced in r1594198 is that the dispatch() was
>> processed in the context of Req2/Res2.
>>
>>
>> Note in all of the above I am ignoring how/where the AsyncContext is
>> stashed and retrieved.
>>
>>
>> I think I can put together a test case for the above. If this is the
>> problem you are seeing then the patch should be fairly easy. The part
>> that will take the time is reviewing the other Async states affected by
>> r1594198 to determine if the same problem might occur for any of them.
>>
>> Mark
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


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

Reply via email to