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 f07875c1fb Further improvements to asynchronous error handling.
f07875c1fb is described below
commit f07875c1fbf63b7130f6bf3775f6b51d568c8d78
Author: Mark Thomas <ma...@apache.org>
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 0f1b3694de..aea694a57d 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -73,7 +73,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()) {
@@ -280,7 +281,8 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
request = null;
clearServletRequestResponse();
timeout = -1;
- hasProcessedError.set(false);
+ hasErrorProcessingStarted.set(false);
+ hasOnErrorReturned.set(false);
}
private void clearServletRequestResponse() {
@@ -379,7 +381,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;
}
@@ -401,6 +403,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);
}
}
}
@@ -500,10 +504,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 af25085a27..9448e44378 100644
--- a/java/org/apache/catalina/core/LocalStrings.properties
+++ b/java/org/apache/catalina/core/LocalStrings.properties
@@ -96,6 +96,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 d64abaed8c..9057ad335e 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -121,6 +121,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: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
