tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Violeta Georgieva <miles...@gmail.com>
Subject Re: AsyncContext.dispatch(path) invoked more than once
Date Tue, 04 Jun 2013 12:55:53 GMT
2013/5/31 Violeta Georgieva wrote:
>
> 2013/5/29 Violeta Georgieva wrote:
> >
> > 2013/5/28 Konstantin Kolinko wrote:
> > >
> > >
> > > 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.
> >
> > Thanks for pointing this.
> >
> > >
> > > BTW, I wonder if the following can be an alternative:
> > >
> > > if (this.dispatch != null) throw new IllegalStateException("..");
> >
> > Unfortunately this will break another scenario:
> >
> > Servlet1 does dispatch to Servlet2
> >
> > AsyncContext asyncContext = request.startAsync(request, response);
> > asyncContext .dispatch("/Servlet2");
> >
> > Servlet2 does dispatch to resourceA
> >
> > AsyncContext asyncContext = request.startAsync(request, response);
> > asyncContext .dispatch("/Servlet2");         <==== here we still do not
have invocation to AsyncContextImpl.recycle and AsyncContextImpl.dispatch
field is not null
> >
> >
> > So if we introduce a null check we will break double dispatch scenario.
> >
> > The solution might be to separate the check for state and the actual
action invocation.
>
> Let's put this as plan B for now.
>
> I made a small change in the AsyncContextImpl.doInternalDispatch().
>
> Can you comment on the patch?
>
>
> Index: C:/tc8.0.x/java/org/apache/catalina/core/AsyncContextImpl.java
> ===================================================================
> --- C:/tc8.0.x/java/org/apache/catalina/core/AsyncContextImpl.java
(revision 1488110)
> +++ C:/tc8.0.x/java/org/apache/catalina/core/AsyncContextImpl.java
(working copy)
> @@ -185,6 +185,10 @@
>              logDebug("dispatch   ");
>          }
>          check();
> +        if (dispatch != null) {
> +            throw new IllegalStateException(
> +                    sm.getString("asyncContextImpl.dispatchingStarted"));
> +        }
>          if (request.getAttribute(ASYNC_REQUEST_URI)==null) {
>              request.setAttribute(ASYNC_REQUEST_URI,
request.getRequestURI());
>              request.setAttribute(ASYNC_CONTEXT_PATH,
request.getContextPath());
> @@ -347,7 +351,9 @@
>              logDebug("intDispatch");
>          }
>          try {
> -            dispatch.run();
> +            Runnable runnable = dispatch;
> +            dispatch = null;
> +            runnable.run();
>              if (!request.isAsync()) {
>                  fireOnComplete();
>              }
>
>

Can you comment?
Any other suggestions?

Thanks

>
>
> > Wdyt?
> >
> > Violeta
> >
> > >
> > > Best regards,
> > > Konstantin Kolinko
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> > > For additional commands, e-mail: dev-help@tomcat.apache.org
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message