This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push: new 1035ff1b7a Further improvements to asynchronous error handling. 1035ff1b7a is described below commit 1035ff1b7ae5709cd65f1f4ad00ff2878db58d10 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 ebf71f008d..621053a09c 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 0430811c3f..9eea586608 100644 --- a/java/org/apache/catalina/core/LocalStrings.properties +++ b/java/org/apache/catalina/core/LocalStrings.properties @@ -95,6 +95,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 2b7e17db88..1be6e6d385 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -127,6 +127,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