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 e60d1d2f8a Prevent duplicate error handling.
e60d1d2f8a is described below
commit e60d1d2f8a0b09224a46cfbb3a2f9225f110f469
Author: Mark Thomas <[email protected]>
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 c7a8f1d878..ba982cbff1 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -73,6 +73,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()) {
@@ -279,6 +280,7 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
request = null;
clearServletRequestResponse();
timeout = -1;
+ hasProcessedError.set(false);
}
private void clearServletRequestResponse() {
@@ -377,6 +379,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 fcac6102f3..ba9822318e 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -186,14 +186,12 @@ public 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 @@ public 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 @@ public 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
@@ -520,6 +510,7 @@ public 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 d04f839605..6dabc87e79 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 4b796c95ae..faeb276266 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: [email protected]
For additional commands, e-mail: [email protected]
