Author: markt Date: Mon Jul 5 20:51:21 2010 New Revision: 960692 URL: http://svn.apache.org/viewvc?rev=960692&view=rev Log: 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
Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=960692&r1=960691&r2=960692&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Mon Jul 5 20:51:21 2010 @@ -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 || Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=960692&r1=960691&r2=960692&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Mon Jul 5 20:51:21 2010 @@ -48,8 +48,8 @@ import org.apache.juli.logging.LogFactor 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 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 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 } 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 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 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) { Modified: tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=960692&r1=960691&r2=960692&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java (original) +++ tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Mon Jul 5 20:51:21 2010 @@ -50,17 +50,17 @@ public class TestAsyncContextImpl extend // 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 extend 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 extend // 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"); - } - } - } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org