This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new 476c7f7 asyncStarted should be false once complete/dispatch in onError 476c7f7 is described below commit 476c7f755b4494f2e1fedfdae637be29fd86da1d Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Oct 11 14:25:17 2019 +0100 asyncStarted should be false once complete/dispatch in onError --- java/org/apache/coyote/AsyncStateMachine.java | 24 ++++++++------- .../apache/catalina/core/TestAsyncContextImpl.java | 35 +++++++++++++++------- webapps/docs/changelog.xml | 3 +- 3 files changed, 41 insertions(+), 21 deletions(-) diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java index 0d364f4..cd98f38 100644 --- a/java/org/apache/coyote/AsyncStateMachine.java +++ b/java/org/apache/coyote/AsyncStateMachine.java @@ -321,7 +321,7 @@ public class AsyncStateMachine { private synchronized boolean doComplete() { clearNonBlockingListeners(); boolean triggerDispatch = false; - if (state == AsyncState.STARTING || state == AsyncState.ERROR) { + if (state == AsyncState.STARTING) { // Processing is on a container thread so no need to transfer // processing to a new container thread state = AsyncState.MUST_COMPLETE; @@ -334,13 +334,15 @@ public class AsyncStateMachine { // request/response associated with the AsyncContext so need a new // container thread to process the different request/response. triggerDispatch = true; - } else if (state == AsyncState.READ_WRITE_OP || state == AsyncState.TIMING_OUT) { + } else if (state == AsyncState.READ_WRITE_OP || state == AsyncState.TIMING_OUT || + state == AsyncState.ERROR) { // Read/write operations can happen on or off a container thread but // while in this state the call to listener that triggers the // read/write will be in progress on a container thread. - // Processing of timeouts can happen on or off a container thread - // (on is much more likely) but while in this state the call that - // triggers the timeout will be in progress on a container thread. + // Processing of timeouts and errors can happen on or off a + // container thread (on is much more likely) but while in this state + // the call that triggers the timeout will be in progress on a + // container thread. // The socket will be added to the poller when the container thread // exits the AbstractConnectionHandler.process() method so don't do // a dispatch here which would add it to the poller a second time. @@ -385,7 +387,7 @@ public class AsyncStateMachine { private synchronized boolean doDispatch() { clearNonBlockingListeners(); boolean triggerDispatch = false; - if (state == AsyncState.STARTING || state == AsyncState.ERROR) { + if (state == AsyncState.STARTING) { // Processing is on a container thread so no need to transfer // processing to a new container thread state = AsyncState.MUST_DISPATCH; @@ -398,13 +400,15 @@ public class AsyncStateMachine { // request/response associated with the AsyncContext so need a new // container thread to process the different request/response. triggerDispatch = true; - } else if (state == AsyncState.READ_WRITE_OP || state == AsyncState.TIMING_OUT) { + } else if (state == AsyncState.READ_WRITE_OP || state == AsyncState.TIMING_OUT || + state == AsyncState.ERROR) { // Read/write operations can happen on or off a container thread but // while in this state the call to listener that triggers the // read/write will be in progress on a container thread. - // Processing of timeouts can happen on or off a container thread - // (on is much more likely) but while in this state the call that - // triggers the timeout will be in progress on a container thread. + // Processing of timeouts and errors can happen on or off a + // container thread (on is much more likely) but while in this state + // the call that triggers the timeout will be in progress on a + // container thread. // The socket will be added to the poller when the container thread // exits the AbstractConnectionHandler.process() method so don't do // a dispatch here which would add it to the poller a second time. diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java index 6e01bbe..319556e 100644 --- a/test/org/apache/catalina/core/TestAsyncContextImpl.java +++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java @@ -682,6 +682,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest { count ++; } Assert.assertEquals(expectedTrack, getTrack()); + Assert.assertTrue(dispatch.isAsyncStartedCorrect()); // Check the access log alv.validateAccessLog(1, 200, 0, REQUEST_TIME); @@ -692,13 +693,15 @@ public class TestAsyncContextImpl extends TomcatBaseTest { private static final long serialVersionUID = 1L; private static final String ITER_PARAM = "iter"; private static final String DISPATCH_CHECK = "check"; - private boolean addTrackingListener = false; - private boolean completeOnError = false; + private final TrackingListener trackingListener; public DispatchingServlet(boolean addTrackingListener, boolean completeOnError) { - this.addTrackingListener = addTrackingListener; - this.completeOnError = completeOnError; + if (addTrackingListener) { + trackingListener = new TrackingListener(completeOnError, true, null); + } else { + trackingListener = null; + } } @Override @@ -713,10 +716,8 @@ public class TestAsyncContextImpl extends TomcatBaseTest { track("DispatchingServletGet-"); final int iter = Integer.parseInt(req.getParameter(ITER_PARAM)) - 1; final AsyncContext ctxt = req.startAsync(); - if (addTrackingListener) { - TrackingListener listener = - new TrackingListener(completeOnError, true, null); - ctxt.addListener(listener); + if (trackingListener != null) { + ctxt.addListener(trackingListener); } Runnable run = new Runnable() { @Override @@ -735,6 +736,13 @@ public class TestAsyncContextImpl extends TomcatBaseTest { run.run(); } } + + public boolean isAsyncStartedCorrect() { + if (trackingListener == null) { + return true; + } + return trackingListener.isAsyncStartedCorrect(); + } } private static class NonAsyncServlet extends HttpServlet { @@ -1043,6 +1051,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest { count ++; } Assert.assertEquals(expectedTrack, getTrack()); + Assert.assertTrue(dispatch.isAsyncStartedCorrect()); // Check the access log alv.validateAccessLog(1, 500, 0, REQUEST_TIME); @@ -1871,7 +1880,8 @@ public class TestAsyncContextImpl extends TomcatBaseTest { Tomcat tomcat = getTomcatInstance(); Context ctx = tomcat.addContext("", null); - Wrapper w = tomcat.addServlet("", "async", new Bug59219Servlet()); + Bug59219Servlet bug59219Servlet = new Bug59219Servlet(); + Wrapper w = tomcat.addServlet("", "async", bug59219Servlet); w.setAsyncSupported(true); ctx.addServletMappingDecoded("/async", "async"); @@ -1887,6 +1897,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest { } Assert.assertEquals(expectedTrack, getTrack()); + Assert.assertTrue(bug59219Servlet.isAsyncStartedCorrect()); } @@ -1894,6 +1905,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest { private static final long serialVersionUID = 1L; + private final TrackingListener trackingListener = new TrackingListener(true, false, "/async"); @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { @@ -1901,7 +1913,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest { track("doGet-"); AsyncContext ctx = req.startAsync(); ctx.setTimeout(3000); - ctx.addListener(new TrackingListener(true, false, "/async")); + ctx.addListener(trackingListener); String loopsParam = req.getParameter("loops"); Integer loopsAttr = (Integer) req.getAttribute("loops"); @@ -1921,6 +1933,9 @@ public class TestAsyncContextImpl extends TomcatBaseTest { throw new ServletException(); } + public boolean isAsyncStartedCorrect() { + return trackingListener.isAsyncStartedCorrect(); + } } @Test diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 9af29b4..8ba4b26 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -51,7 +51,8 @@ Ensure that <code>ServletRequest.isAsyncStarted()</code> returns <code>false</code> once <code>AsyncContext.complete()</code> or <code>AsyncContext.dispatch()</code> has been called during - <code>AsyncListener.onTimeout()</code>. (markt) + <code>AsyncListener.onTimeout()</code> or + <code>AsyncListener.onError()</code>. (markt) </fix> </changelog> </subsection> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org