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 9873b0bbb9 Further improvements to asynchronous error handling.
9873b0bbb9 is described below
commit 9873b0bbb95eb76da094f34ee70c3c80f71edcdb
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Jan 24 20:47:21 2024 +0000
Further improvements to asynchronous error handling.
Ensure that application threads cannot access the AsyncContext once the
call to AsyncListener.onError() has returned to the container. This
protects against various race conditions.
---
java/org/apache/catalina/core/AsyncContextImpl.java | 19 ++++++++++++++++---
java/org/apache/catalina/core/LocalStrings.properties | 1 +
webapps/docs/changelog.xml | 7 +++++++
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java
b/java/org/apache/catalina/core/AsyncContextImpl.java
index e77203ad21..b713f71f10 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -74,7 +74,8 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
private long timeout = -1;
private AsyncEvent event = null;
private volatile Request request;
- private final AtomicBoolean hasProcessedError = new AtomicBoolean(false);
+ private final AtomicBoolean hasErrorProcessingStarted = new
AtomicBoolean(false);
+ private final AtomicBoolean hasOnErrorReturned = new AtomicBoolean(false);
public AsyncContextImpl(Request request) {
if (log.isDebugEnabled()) {
@@ -281,7 +282,8 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
request = null;
clearServletRequestResponse();
timeout = -1;
- hasProcessedError.set(false);
+ hasErrorProcessingStarted.set(false);
+ hasOnErrorReturned.set(false);
}
private void clearServletRequestResponse() {
@@ -380,7 +382,7 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
public void setErrorState(Throwable t, boolean fireOnError) {
- if (!hasProcessedError.compareAndSet(false, true)) {
+ if (!hasErrorProcessingStarted.compareAndSet(false, true)) {
// Skip duplicate error processing
return;
}
@@ -402,6 +404,8 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
} catch (Throwable t2) {
ExceptionUtils.handleThrowable(t2);
log.warn(sm.getString("asyncContextImpl.onErrorError",
listener.getClass().getName()), t2);
+ } finally {
+ hasOnErrorReturned.set(true);
}
}
}
@@ -501,10 +505,19 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
}
private void check() {
+ Request request = this.request;
if (request == null) {
// AsyncContext has been recycled and should not be being used
throw new
IllegalStateException(sm.getString("asyncContextImpl.requestEnded"));
}
+ if (hasOnErrorReturned.get() &&
!request.getCoyoteRequest().isRequestThread()) {
+ /*
+ * Non-container thread is trying to use the AsyncContext after an
error has occurred and the call to
+ * AsyncListener.onError() has returned. At this point, the
non-container thread should not be trying to use
+ * the AsyncContext due to possible race conditions.
+ */
+ throw new
IllegalStateException(sm.getString("asyncContextImpl.afterOnError"));
+ }
}
private static class DebugException extends Exception {
diff --git a/java/org/apache/catalina/core/LocalStrings.properties
b/java/org/apache/catalina/core/LocalStrings.properties
index a43dfd9f25..ec1126d4bf 100644
--- a/java/org/apache/catalina/core/LocalStrings.properties
+++ b/java/org/apache/catalina/core/LocalStrings.properties
@@ -93,6 +93,7 @@ aprListener.tooLateForSSLRandomSeed=Cannot setSSLRandomSeed:
SSL has already bee
aprListener.usingFIPSProvider=Using OpenSSL with the FIPS provider as the
default provider
aprListener.wrongFIPSMode=Unexpected value of FIPSMode option of
AprLifecycleListener: [{0}]
+asyncContextImpl.afterOnError=A non-container (application) thread attempted
to use the AsyncContext after an error had occurred and the call to
AsyncListener.onError() had returned. This is not allowed to avoid race
conditions.
asyncContextImpl.asyncDispatchError=Error during asynchronous dispatch
asyncContextImpl.asyncRunnableError=Error during processing of asynchronous
Runnable via AsyncContext.start()
asyncContextImpl.dispatchingStarted=Asynchronous dispatch operation has
already been called. Additional asynchronous dispatch operation within the same
asynchronous cycle is not allowed.
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 44140d341f..8d17927e31 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -131,6 +131,13 @@
connection is marked to be closed, further asynchronous processing
cannot change that. (markt)
</fix>
+ <fix>
+ Make asynchronous error handling more robust. Ensure that once the call
+ to <code>AsyncListener.onError()</code> has returned to the container,
+ only container threads can access the <code>AsyncContext</code>. This
+ protects against various race conditions that woudl otherwise occur if
+ application threads continued to access the <code>AsyncContext</code>.
+ </fix>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
