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 465d6d8cd6 Fix BZ 69121. Ensure onComplete() is always triggered 465d6d8cd6 is described below commit 465d6d8cd6b31329d5055d10e178d1749868ba81 Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Jun 21 10:43:48 2024 +0100 Fix BZ 69121. Ensure onComplete() is always triggered The call to onComplete() could be skipped if the dispatch target from AsyncListener.onError() threw an exception --- .../org/apache/catalina/core/AsyncContextImpl.java | 11 +++++++++ .../TestAsyncContextImplListenerOnComplete.java | 27 +++++++++++++++++++--- webapps/docs/changelog.xml | 5 ++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java b/java/org/apache/catalina/core/AsyncContextImpl.java index 5fe09c6fb1..50834aa7cf 100644 --- a/java/org/apache/catalina/core/AsyncContextImpl.java +++ b/java/org/apache/catalina/core/AsyncContextImpl.java @@ -346,6 +346,17 @@ public class AsyncContextImpl implements AsyncContext, AsyncContextCallback { fireOnComplete(); } } catch (RuntimeException x) { + AtomicBoolean result = new AtomicBoolean(); + request.getCoyoteRequest().action(ActionCode.IS_IO_ALLOWED, result); + /* + * If IO is allowed then onComplete() will be called from AbstractProcessorLight.process() when + * AbstractProcessorLight.postProcess() is called. If IO is not allowed then that call will not happen so + * call onComplete() here. This can't be handled in AbstractProcessorLight.process() as it does not have the + * information required to determine that onComplete() needs to be called. + */ + if (!result.get()) { + fireOnComplete(); + } if (x.getCause() instanceof ServletException) { throw (ServletException) x.getCause(); } diff --git a/test/org/apache/catalina/core/TestAsyncContextImplListenerOnComplete.java b/test/org/apache/catalina/core/TestAsyncContextImplListenerOnComplete.java index a316e95c25..8c6151244c 100644 --- a/test/org/apache/catalina/core/TestAsyncContextImplListenerOnComplete.java +++ b/test/org/apache/catalina/core/TestAsyncContextImplListenerOnComplete.java @@ -49,7 +49,21 @@ public class TestAsyncContextImplListenerOnComplete extends TomcatBaseTest { * https://bz.apache.org/bugzilla/show_bug.cgi?id=68227 */ @Test - public void testAfterNetworkErrorThenDispatch() throws Exception { + public void testAfterNetworkErrorThenDispatchWithoutRuntimeException() throws Exception { + doTestAfterNetworkErrorThenDispatch(false); + } + + + /* + * https://bz.apache.org/bugzilla/show_bug.cgi?id=69121 + */ + @Test + public void testAfterNetworkErrorThenDispatchWithRuntimeException() throws Exception { + doTestAfterNetworkErrorThenDispatch(true); + } + + + private void doTestAfterNetworkErrorThenDispatch(boolean dispatchThrowsRuntimeException) throws Exception { // Setup Tomcat instance Tomcat tomcat = getTomcatInstance(); @@ -59,7 +73,8 @@ public class TestAsyncContextImplListenerOnComplete extends TomcatBaseTest { // Latch to track that complete has been called CountDownLatch completeLatch = new CountDownLatch(1); - Wrapper servletWrapper = tomcat.addServlet("", "repro-servlet", new ReproServlet(completeLatch)); + Wrapper servletWrapper = + tomcat.addServlet("", "repro-servlet", new ReproServlet(completeLatch, dispatchThrowsRuntimeException)); servletWrapper.addMapping("/repro"); servletWrapper.setAsyncSupported(true); servletWrapper.setLoadOnStartup(1); @@ -97,9 +112,11 @@ public class TestAsyncContextImplListenerOnComplete extends TomcatBaseTest { private final EventSource eventSource = new EventSource(); private final CountDownLatch completeLatch; + private final boolean throwRuntimeExceptionOnErrorDispatch; - ReproServlet(CountDownLatch completeLatch) { + ReproServlet(CountDownLatch completeLatch, boolean throwRuntimeExceptionOnErrorDispatch) { this.completeLatch = completeLatch; + this.throwRuntimeExceptionOnErrorDispatch = throwRuntimeExceptionOnErrorDispatch; } @Override @@ -115,6 +132,10 @@ public class TestAsyncContextImplListenerOnComplete extends TomcatBaseTest { AsyncContext context = req.startAsync(); context.addListener(new ReproAsyncListener()); eventSource.add(context); + } else if (req.getDispatcherType() == DispatcherType.ASYNC && throwRuntimeExceptionOnErrorDispatch) { + // The only async dispatch to this servlet is on an error + // Spring will throw a RuntimeException here if it detects that the response is no longer usable + throw new RuntimeException(); } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 330378c443..a092d2b553 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -119,6 +119,11 @@ Clean and log OpenSSL errors before processing of OpenSSL conf commands in the FFM code. (remm) </fix> + <fix> + <bug>69121</bug>: Ensure that the <code>onComplete()</code> event is + triggered if <code>AsyncListener.onError()</code> dispatches to a target + that throws an exception. (markt) + </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