You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/10/11 12:38:45 UTC
[tomcat] branch master updated: asyncStarted should be false once
complete/dispatch in onTimeout
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push:
new 71389c0 asyncStarted should be false once complete/dispatch in onTimeout
71389c0 is described below
commit 71389c058286e70f88235c92283ed0fa45a2b027
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Oct 10 16:57:55 2019 +0100
asyncStarted should be false once complete/dispatch in onTimeout
---
java/org/apache/coyote/AsyncStateMachine.java | 16 +++++----
.../apache/catalina/core/TestAsyncContextImpl.java | 38 +++++++++++++++++++---
webapps/docs/changelog.xml | 10 ++++++
3 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index 6d66567..33cbff4 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -321,8 +321,7 @@ class AsyncStateMachine {
private synchronized boolean doComplete() {
clearNonBlockingListeners();
boolean triggerDispatch = false;
- if (state == AsyncState.STARTING || state == AsyncState.TIMING_OUT ||
- state == AsyncState.ERROR) {
+ if (state == AsyncState.STARTING || state == AsyncState.ERROR) {
// Processing is on a container thread so no need to transfer
// processing to a new container thread
state = AsyncState.MUST_COMPLETE;
@@ -335,10 +334,13 @@ 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) {
+ } else if (state == AsyncState.READ_WRITE_OP || state == AsyncState.TIMING_OUT) {
// 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.
// 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.
@@ -383,8 +385,7 @@ class AsyncStateMachine {
private synchronized boolean doDispatch() {
clearNonBlockingListeners();
boolean triggerDispatch = false;
- if (state == AsyncState.STARTING || state == AsyncState.TIMING_OUT ||
- state == AsyncState.ERROR) {
+ if (state == AsyncState.STARTING || state == AsyncState.ERROR) {
// Processing is on a container thread so no need to transfer
// processing to a new container thread
state = AsyncState.MUST_DISPATCH;
@@ -397,10 +398,13 @@ 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) {
+ } else if (state == AsyncState.READ_WRITE_OP || state == AsyncState.TIMING_OUT) {
// 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.
// 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 d748086..3d496f4 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImpl.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImpl.java
@@ -555,19 +555,25 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
alv.validateAccessLog(1, 200, timeoutDelay,
timeoutDelay + TIMEOUT_MARGIN + REQUEST_TIME);
}
+
+ Assert.assertTrue(timeout.isAsyncStartedCorrect());
}
private static class TimeoutServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
private final Boolean completeOnTimeout;
- private final String dispatchUrl;
+ private final TrackingListener trackingListener;
public static final long ASYNC_TIMEOUT = 100;
public TimeoutServlet(Boolean completeOnTimeout, String dispatchUrl) {
this.completeOnTimeout = completeOnTimeout;
- this.dispatchUrl = dispatchUrl;
+ if (completeOnTimeout == null) {
+ this.trackingListener = null;
+ } else {
+ this.trackingListener = new TrackingListener(false, completeOnTimeout.booleanValue(), dispatchUrl);
+ }
}
@Override
@@ -579,13 +585,19 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
ac.setTimeout(ASYNC_TIMEOUT);
if (completeOnTimeout != null) {
- ac.addListener(new TrackingListener(false,
- completeOnTimeout.booleanValue(), dispatchUrl));
+ ac.addListener(trackingListener);
}
} else {
resp.getWriter().print("FAIL: Async unsupported");
}
}
+
+ public boolean isAsyncStartedCorrect() {
+ if (trackingListener == null) {
+ return true;
+ }
+ return trackingListener.isAsyncStartedCorrect();
+ }
}
@Test
@@ -827,6 +839,7 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
private final boolean completeOnError;
private final boolean completeOnTimeout;
private final String dispatchUrl;
+ private boolean asyncStartedCorrect = true;
public TrackingListener(boolean completeOnError,
boolean completeOnTimeout, String dispatchUrl) {
@@ -842,27 +855,44 @@ public class TestAsyncContextImpl extends TomcatBaseTest {
@Override
public void onTimeout(AsyncEvent event) throws IOException {
+ boolean expectedAsyncStarted = true;
+
TestAsyncContextImpl.track("onTimeout-");
if (completeOnTimeout){
event.getAsyncContext().complete();
+ expectedAsyncStarted = false;
}
if (dispatchUrl != null) {
event.getAsyncContext().dispatch(dispatchUrl);
+ expectedAsyncStarted = false;
}
+
+ ServletRequest req = event.getSuppliedRequest();
+ asyncStartedCorrect = (expectedAsyncStarted == req.isAsyncStarted());
}
@Override
public void onError(AsyncEvent event) throws IOException {
+ boolean expectedAsyncStarted = true;
+
TestAsyncContextImpl.track("onError-");
if (completeOnError) {
event.getAsyncContext().complete();
+ expectedAsyncStarted = false;
}
+
+ ServletRequest req = event.getSuppliedRequest();
+ asyncStartedCorrect = (expectedAsyncStarted == req.isAsyncStarted());
}
@Override
public void onStartAsync(AsyncEvent event) throws IOException {
TestAsyncContextImpl.track("onStartAsync-");
}
+
+ public boolean isAsyncStartedCorrect() {
+ return asyncStartedCorrect;
+ }
}
private static class StickyTrackingListener extends TrackingListener {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2fb65e2..38ee812 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -53,6 +53,16 @@
</fix>
</changelog>
</subsection>
+ <subsection name="Coyote">
+ <changelog>
+ <fix>
+ 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)
+ </fix>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 9.0.27 (markt)" rtext="release in progress">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org