[prev in list] [next in list] [prev in thread] [next in thread] 

List:       tomcat-dev
Subject:    Re: AsyncContext.dispatch(path) invoked more than once
From:       Violeta Georgieva <milesg78 () gmail ! com>
Date:       2013-05-31 7:55:18
Message-ID: CAFmzfTWkPauqueuP_M6Vf1nU7zEMcFiLFxAaAZepYpHzrNm_Pg () mail ! gmail ! com
[Download RAW message or body]


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();
             }




> 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
> >
>


[prev in list] [next in list] [prev in thread] [next in thread]