Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49528

Previous fix was incomplete. Improve test case and fix
TCK and test cases pass with this patch

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@960692 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index 5d42d10..c4c5d5d 100644 (file)
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -1578,6 +1578,7 @@ public class Request
         }
         
         return (asyncContext.getState()==AsyncContextImpl.AsyncState.DISPATCHING ||
+                asyncContext.getState()==AsyncContextImpl.AsyncState.DISPATCHING_RUNNABLE  ||
                 asyncContext.getState()==AsyncContextImpl.AsyncState.TIMING_OUT  ||
                 asyncContext.getState()==AsyncContextImpl.AsyncState.STARTED     ||
                 asyncContext.getState()==AsyncContextImpl.AsyncState.ERROR_DISPATCHING ||

diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java b/java/org/apache/catalina/core/AsyncContextImpl.java
index 04c94ea..5c0de4e 100644 (file)
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -48,8 +48,8 @@ import org.apache.juli.logging.LogFactory;
 public class AsyncContextImpl implements AsyncContext {
     
     public static enum AsyncState {
-        NOT_STARTED, STARTED, DISPATCHING, DISPATCHING_INTERNAL, DISPATCHED,
-        COMPLETING, TIMING_OUT, ERROR_DISPATCHING
+        NOT_STARTED, STARTED, DISPATCHING, DISPATCHING_RUNNABLE, DISPATCHED,
+        COMPLETING, COMPLETING_RUNNABLE, TIMING_OUT, ERROR_DISPATCHING
     }
     
     private static final Log log = LogFactory.getLog(AsyncContextImpl.class);
@@ -87,6 +87,9 @@ public class AsyncContextImpl implements AsyncContext {
             AtomicBoolean dispatched = new AtomicBoolean(false);
             request.getCoyoteRequest().action(ActionCode.ACTION_ASYNC_COMPLETE,dispatched);
             if (!dispatched.get()) doInternalComplete(false);
+        } else if (state.compareAndSet(AsyncState.DISPATCHING_RUNNABLE,
+                AsyncState.COMPLETING_RUNNABLE)) {
+            // do nothing
         } else {
             throw new IllegalStateException("Complete not allowed. Invalid state:"+state.get());
         }
@@ -179,8 +182,8 @@ public class AsyncContextImpl implements AsyncContext {
             log.debug("AsyncContext Start Called["+state.get()+"; "+request.getRequestURI()+"?"+request.getQueryString()+"]", new DebugException());
         }
 
-        if (state.compareAndSet(AsyncState.STARTED, AsyncState.DISPATCHING_INTERNAL) ||
-            state.compareAndSet(AsyncState.DISPATCHED, AsyncState.DISPATCHING_INTERNAL)) {
+        if (state.compareAndSet(AsyncState.STARTED, AsyncState.DISPATCHING_RUNNABLE) ||
+            state.compareAndSet(AsyncState.DISPATCHED, AsyncState.DISPATCHING_RUNNABLE)) {
             // TODO SERVLET3 - async
             final ServletContext sctx = getServletRequest().getServletContext();
             Runnable r = new Runnable() {
@@ -254,7 +257,9 @@ public class AsyncContextImpl implements AsyncContext {
     }
 
     public boolean isStarted() {
-        return (state.get() == AsyncState.STARTED || state.get() == AsyncState.DISPATCHING);
+        return (state.get() == AsyncState.STARTED ||
+                state.get() == AsyncState.DISPATCHING ||
+                state.get() == AsyncState.DISPATCHING_RUNNABLE);
     }
 
     public void setStarted(Context context) {
@@ -334,7 +339,7 @@ public class AsyncContextImpl implements AsyncContext {
                     dispatch = null;
                 }
             }
-        } else if (this.state.get() == AsyncState.DISPATCHING_INTERNAL) {
+        } else if (this.state.get() == AsyncState.DISPATCHING_RUNNABLE) {
             if (this.dispatch!=null) {
                 try {
                     dispatch.run();
@@ -345,7 +350,14 @@ public class AsyncContextImpl implements AsyncContext {
                     else throw new ServletException(x);
                 } finally {
                     dispatch = null;
-                    this.state.set(AsyncState.DISPATCHED);
+                }
+                if (this.state.compareAndSet(AsyncState.COMPLETING_RUNNABLE,
+                        AsyncState.COMPLETING)) {
+                    doInternalComplete(false);
+                } else if (this.state.get() == AsyncState.DISPATCHING_RUNNABLE) {
+                    doInternalComplete(true);
+                    throw new IllegalStateException(
+                            "Failed to call dispatch() or complete() after start()");
                 }
             }
         } else if (this.state.get()==AsyncState.COMPLETING) {

diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java
index d2078b3..09cdfde 100644 (file)
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -50,17 +50,17 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
         // Call the servlet once
         getUrl("http://localhost:" + getPort() + "/");
 
-        assertEquals("", servlet.getErrors());
+        assertEquals("1false2true3true4true5false", servlet.getResult());
     }
     
     private static class Bug49528Servlet extends HttpServlet {
 
         private static final long serialVersionUID = 1L;
         
-        private StringBuilder errors = new StringBuilder();
+        private StringBuilder result = new StringBuilder();
         
-        public String getErrors() {
-            return errors.toString();
+        public String getResult() {
+            return result.toString();
         }
 
         @Override
@@ -68,19 +68,24 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
                 final HttpServletResponse resp)
                 throws ServletException, IOException {
             
-            confirmFalse("1", req);
+            result.append('1');
+            result.append(req.isAsyncStarted());
             req.startAsync();
-            confirmTrue("2", req);
+            result.append('2');
+            result.append(req.isAsyncStarted());
             
             req.getAsyncContext().start(new Runnable() {
                 @Override
                 public void run() {
                     try {
-                        confirmTrue("3", req);
+                        result.append('3');
+                        result.append(req.isAsyncStarted());
                         Thread.sleep(1000);
-                        confirmTrue("4", req);
+                        result.append('4');
+                        result.append(req.isAsyncStarted());
                         req.getAsyncContext().complete();
-                        confirmFalse("5", req);
+                        result.append('5');
+                        result.append(req.isAsyncStarted());
                     } catch (InterruptedException e) {
                         // TODO Auto-generated catch block
                         e.printStackTrace();
@@ -91,20 +96,5 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
             // when debugging
             req.getMethod();
         }
-        
-        private void confirmFalse(String stage, HttpServletRequest req) {
-            if (req.isAsyncStarted()) {
-                errors.append("Stage " + stage +
-                        ": Async started when not expected\n");
-            }
-        }
-
-        private void confirmTrue(String stage, HttpServletRequest req) {
-            if (!req.isAsyncStarted()) {
-                errors.append("Stage " + stage +
-                        ": Async not started when expected\n");
-            }
-        }
-
     }
 }