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 2023/09/15 07:40:39 UTC
[tomcat] branch 10.1.x updated: Prevent duplicate error handling.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new 4f8c6efc35 Prevent duplicate error handling.
4f8c6efc35 is described below
commit 4f8c6efc35cfd2636df382456feb49f1aa07845f
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Sep 14 20:27:18 2023 +0100
Prevent duplicate error handling.
When an error occurs during asynchronous processing, ensure that the error
handling process is only triggered once per asynchronous cycle.
---
.../org/apache/catalina/core/AsyncContextImpl.java | 6 +++
java/org/apache/coyote/AsyncStateMachine.java | 49 +++++++++-------------
test/org/apache/coyote/http2/TestAsyncError.java | 2 +-
webapps/docs/changelog.xml | 5 +++
4 files changed, 32 insertions(+), 30 deletions(-)
diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java b/java/org/apache/catalina/core/AsyncContextImpl.java
index cceddd1e6a..1e4b03741d 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -74,6 +74,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback {
private long timeout = -1;
private AsyncEvent event = null;
private volatile Request request;
+ AtomicBoolean hasProcessedError = new AtomicBoolean(false);
public AsyncContextImpl(Request request) {
if (log.isDebugEnabled()) {
@@ -280,6 +281,7 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback {
request = null;
clearServletRequestResponse();
timeout = -1;
+ hasProcessedError.set(false);
}
private void clearServletRequestResponse() {
@@ -378,6 +380,10 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback {
public void setErrorState(Throwable t, boolean fireOnError) {
+ if (!hasProcessedError.compareAndSet(false, true)) {
+ // Skip duplicate error processing
+ return;
+ }
if (t != null) {
request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t);
}
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index c81f93735c..a651def252 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -186,14 +186,12 @@ class AsyncStateMachine {
*/
private final AtomicLong generation = new AtomicLong(0);
/*
- * Error processing should only be triggered once per async generation. These fields track the last generation of
- * async processing for which error processing was triggered and are used to ensure that the second and subsequent
- * attempts to trigger async error processing for a given generation are NO-OPs.
+ * Error processing should only be triggered once per async generation. This field tracks whether the async
+ * processing has entered the error state during this async cycle.
*
* Guarded by this
*/
- private long lastErrorGeneration = -1;
- private long lastErrorGenerationMust = -1;
+ private boolean hasProcessedError = false;
// Need this to fire listener on complete
private AsyncContextCallback asyncCtxt = null;
@@ -422,23 +420,6 @@ class AsyncStateMachine {
Request request = processor.getRequest();
boolean containerThread = (request != null && request.isRequestThread());
- // Ensure the error processing is only started once per generation
- if (lastErrorGeneration == getCurrentGeneration()) {
- if (state == AsyncState.MUST_ERROR && containerThread && lastErrorGenerationMust != getCurrentGeneration()) {
- // This is the first container thread call after state was set to MUST_ERROR so don't skip
- lastErrorGenerationMust = getCurrentGeneration();
- } else {
- // Duplicate call. Skip.
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("asyncStateMachine.asyncError.skip"));
- }
- return false;
- }
- } else {
- // First call for this generation, don't skip.
- lastErrorGeneration = getCurrentGeneration();
- }
-
if (log.isDebugEnabled()) {
log.debug(sm.getString("asyncStateMachine.asyncError.start"));
}
@@ -446,14 +427,23 @@ class AsyncStateMachine {
clearNonBlockingListeners();
if (state == AsyncState.STARTING) {
updateState(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();
- updateState(AsyncState.ERROR);
} else {
- updateState(AsyncState.ERROR);
+ if (hasProcessedError) {
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("asyncStateMachine.asyncError.skip"));
+ }
+ return false;
+ }
+ hasProcessedError = true;
+ 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();
+ updateState(AsyncState.ERROR);
+ } else {
+ updateState(AsyncState.ERROR);
+ }
}
// Return true for non-container threads to trigger a dispatch
@@ -519,6 +509,7 @@ class AsyncStateMachine {
asyncCtxt = null;
state = AsyncState.DISPATCHED;
lastAsyncStart = 0;
+ hasProcessedError = false;
}
diff --git a/test/org/apache/coyote/http2/TestAsyncError.java b/test/org/apache/coyote/http2/TestAsyncError.java
index a741bbd256..344bfa1ab3 100644
--- a/test/org/apache/coyote/http2/TestAsyncError.java
+++ b/test/org/apache/coyote/http2/TestAsyncError.java
@@ -86,7 +86,7 @@ public class TestAsyncError extends Http2TestBase {
Thread.sleep(100);
}
- Assert.assertTrue(TestListener.getErrorCount() > 0);
+ Assert.assertEquals(1, TestListener.getErrorCount());
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index eeacd653b0..bc3257c93f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -124,6 +124,11 @@
<code>AsyncListener</code> handles an error with a dispatch rather than
a complete. (markt)
</fix>
+ <fix>
+ When an error occurs during asynchronous processing, ensure that the
+ error handling process is only triggered once per asynchronous cycle.
+ (markt)
+ </fix>
</changelog>
</subsection>
</section>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org