This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 591fe624d2 Further improvements to asynchronous error handling.
591fe624d2 is described below
commit 591fe624d2f5e26355475c2f32c2ad67d297c364
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 626fb36c2c..ae41532aca 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 d65120a9fa..e323b57b37 100644
--- a/java/org/apache/catalina/core/LocalStrings.properties
+++ b/java/org/apache/catalina/core/LocalStrings.properties
@@ -90,6 +90,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 756b40a1b7..86ddca1546 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -142,6 +142,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="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
