tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: AsyncContext.dispatch(path) invoked more than once
Date Tue, 28 May 2013 19:11:41 GMT
2013/5/28 Violeta Georgieva <milesg78@gmail.com>:
> Hi,
>
> In the AsyncContext.dispatch(path) javadoc we have:
>
> "There can be at most one asynchronous dispatch operation per asynchronous
> cycle, which is started by a call to one of the
> ServletRequest.startAsync()<file:///C:/vily/my%20documents/servlet-3_0-final-javadoc/javax/servlet/ServletRequest.html#startAsync()>methods.
> Any attempt to perform an additional asynchronous dispatch
> operation within the same asynchronous cycle will result in an
> IllegalStateException. If startAsync is subsequently called on the
> dispatched request, then any of the dispatch or
> complete()<file:///C:/vily/my%20documents/servlet-3_0-final-javadoc/javax/servlet/AsyncContext.html#complete()>methods
> may be called."
>
> If we have the following scenario:
>
> AsyncContext asyncContext = request.startAsync(request, response);
> asyncContext .dispatch("/resourceA");
> asyncContext .dispatch("/resourceB");
>
> I would assume that the second dispatch will throw ISE and a dispatching to
> the "resourceA" will happen.
>
> The current implementation (tomcat7/8) throws ISE but the dispatching is to
> the "resourceB".
>
> I prepared a change (below) that moves the check for ISE a little bit
> earlier. o.a.catalina.core.AsyncContextImpl.dispatch field is not
> overridden with information from the second dispatch() invocation, thus a
> dispatching to "resourceA" will happen.
>
> What do you think? Are my assumptions correct?
>
> Thanks
> Violeta
>
>
> Index: C:/tc8.0.x/java/org/apache/catalina/core/AsyncContextImpl.java
> ===================================================================
> --- C:/tc8.0.x/java/org/apache/catalina/core/AsyncContextImpl.java (revision
> 1486660)
> +++ C:/tc8.0.x/java/org/apache/catalina/core/AsyncContextImpl.java (working
> copy)
> @@ -216,8 +216,8 @@
>              }
>          };
>
> +        this.request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCH,
> null);
>          this.dispatch = run;
> -        this.request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCH,
> null);
>      }
>
>      @Override

I think that your patch is wrong.

Looking at how ActionCode.ASYNC_DISPATCH is handled in different
connector implementations in Tomcat 7, the code is like the following:

            if (asyncStateMachine.asyncDispatch()) {
                ((AprEndpoint)endpoint).processSocketAsync(this.socket,
                        SocketStatus.OPEN);
            }

In your example dispatching does not happen immediately as
asyncDispatch() call returns "false" and asyncStateMachine state
changes to AsyncState.MUST_DISPATCH.  Thus your patch works.

But, there is a case when the call returns "true" and dispatching
happens immediately. Such dispatching will be broken.

BTW, I wonder if the following can be an alternative:

if (this.dispatch != null) throw new IllegalStateException("..");

Best regards,
Konstantin Kolinko

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


Mime
View raw message