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 78d9c76 Fix double counting in tracking of in-flight async requests
78d9c76 is described below
commit 78d9c76333214c752ebdd108677925dad6bdc585
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Sep 22 14:40:58 2020 +0100
Fix double counting in tracking of in-flight async requests
---
java/org/apache/catalina/core/AsyncContextImpl.java | 19 ++++++++++++-------
java/org/apache/coyote/AsyncContextCallback.java | 20 ++++++++++++++++++--
java/org/apache/coyote/AsyncStateMachine.java | 12 ++++++++++++
webapps/docs/changelog.xml | 5 +++++
4 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java
b/java/org/apache/catalina/core/AsyncContextImpl.java
index 05743b8..90851b0 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -113,7 +113,6 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
} finally {
context.fireRequestDestroyEvent(request.getRequest());
clearServletRequestResponse();
- this.context.decrementInProgressAsyncCount();
context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
}
}
@@ -206,16 +205,10 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
(AsyncDispatcher) requestDispatcher;
final ServletRequest servletRequest = getRequest();
final ServletResponse servletResponse = getResponse();
- // https://bz.apache.org/bugzilla/show_bug.cgi?id=63246
- // Take a local copy as the dispatch may complete the
- // request/response and that in turn may trigger recycling of this
- // object before the in-progress count can be decremented
- final Context context = this.context;
this.dispatch = new AsyncRunnable(
request, applicationDispatcher, servletRequest,
servletResponse);
this.request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCH,
null);
clearServletRequestResponse();
- context.decrementInProgressAsyncCount();
}
}
@@ -457,6 +450,18 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
}
+ @Override
+ public void incrementInProgressAsyncCount() {
+ context.incrementInProgressAsyncCount();
+ }
+
+
+ @Override
+ public void decrementInProgressAsyncCount() {
+ context.decrementInProgressAsyncCount();
+ }
+
+
private void logDebug(String method) {
String rHashCode;
String crHashCode;
diff --git a/java/org/apache/coyote/AsyncContextCallback.java
b/java/org/apache/coyote/AsyncContextCallback.java
index c657306..d3119af 100644
--- a/java/org/apache/coyote/AsyncContextCallback.java
+++ b/java/org/apache/coyote/AsyncContextCallback.java
@@ -23,7 +23,7 @@ package org.apache.coyote;
* org.apache.catalina package.
*/
public interface AsyncContextCallback {
- public void fireOnComplete();
+ void fireOnComplete();
/**
* Reports if the web application associated with this async request is
@@ -32,5 +32,21 @@ public interface AsyncContextCallback {
* @return {@code true} if the associated web application is available,
* otherwise {@code false}
*/
- public boolean isAvailable();
+ boolean isAvailable();
+
+ /**
+ * Used to notify the Context that async processing has started.
+ * Specifically, for the counting of in-progress async requests to work
+ * correctly, this must be called exactly once every time the
+ * {@link AsyncStateMachine} transitions from DISPATCHED to any other
state.
+ */
+ void incrementInProgressAsyncCount();
+
+ /**
+ * Used to notify the Context that async processing has ended.
+ * Specifically, for the counting of in-progress async requests to work
+ * correctly, this must be called exactly once every time the
+ * {@link AsyncStateMachine} transitions to DISPATCHED from any other
state.
+ */
+ void decrementInProgressAsyncCount();
}
diff --git a/java/org/apache/coyote/AsyncStateMachine.java
b/java/org/apache/coyote/AsyncStateMachine.java
index bf32bf9..501012c 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -235,6 +235,9 @@ public class AsyncStateMachine {
if (state == AsyncState.DISPATCHED) {
generation.incrementAndGet();
state = AsyncState.STARTING;
+ // Note: In this instance, caller is responsible for calling
+ // asyncCtxt.incrementInProgressAsyncCount() as that allows simpler
+ // error handling.
this.asyncCtxt = asyncCtxt;
lastAsyncStart = System.currentTimeMillis();
} else {
@@ -274,12 +277,14 @@ public class AsyncStateMachine {
} else if (state == AsyncState.MUST_COMPLETE || state ==
AsyncState.COMPLETING) {
asyncCtxt.fireOnComplete();
state = AsyncState.DISPATCHED;
+ asyncCtxt.decrementInProgressAsyncCount();
return SocketState.ASYNC_END;
} else if (state == AsyncState.MUST_DISPATCH) {
state = AsyncState.DISPATCHING;
return SocketState.ASYNC_END;
} else if (state == AsyncState.DISPATCHING) {
state = AsyncState.DISPATCHED;
+ asyncCtxt.decrementInProgressAsyncCount();
return SocketState.ASYNC_END;
} else if (state == AsyncState.STARTED) {
// This can occur if an async listener does a dispatch to an async
@@ -401,6 +406,7 @@ public class AsyncStateMachine {
if (state == AsyncState.DISPATCHING ||
state == AsyncState.MUST_DISPATCH) {
state = AsyncState.DISPATCHED;
+ asyncCtxt.decrementInProgressAsyncCount();
} else {
throw new IllegalStateException(
sm.getString("asyncStateMachine.invalidAsyncState",
@@ -413,6 +419,12 @@ public class AsyncStateMachine {
clearNonBlockingListeners();
if (state == AsyncState.STARTING) {
state = AsyncState.MUST_ERROR;
+ } else if (state == AsyncState.DISPATCHED) {
+ // Async error handling has moved processing back into an async
+ // state. Need to increment in progress count as it will decrement
+ // when the async state is exited again.
+ asyncCtxt.incrementInProgressAsyncCount();
+ state = AsyncState.ERROR;
} else {
state = AsyncState.ERROR;
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a189f14..d7d12c1 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -80,6 +80,11 @@
When logging HTTP/2 debug messages, use consistent formatting for
stream
identifiers. (markt)
</fix>
+ <fix>
+ Correct some double counting in the code that tracks the number of
+ in-flight asynchronous requests. The tracking enables Tomcat to
shutdown
+ gracefully when asynchronous processing is in use. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
