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