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 29c6e7cb7e Fix BZ 69121. Ensure onComplete() is always triggered
29c6e7cb7e is described below
commit 29c6e7cb7ed2994e095ce1378acebcdf76289bd0
Author: Mark Thomas <[email protected]>
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
s
---
.../org/apache/catalina/core/AsyncContextImpl.java | 11 ++++++++
.../TestAsyncContextImplListenerOnComplete.java | 32 ++++++++++++++++++----
webapps/docs/changelog.xml | 5 ++++
3 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java
b/java/org/apache/catalina/core/AsyncContextImpl.java
index 7b93e1114c..ae62a3dd8b 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -345,6 +345,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 950ace2dc8..c796bb458b 100644
--- a/test/org/apache/catalina/core/TestAsyncContextImplListenerOnComplete.java
+++ b/test/org/apache/catalina/core/TestAsyncContextImplListenerOnComplete.java
@@ -50,7 +50,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();
@@ -60,7 +74,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);
@@ -82,9 +97,8 @@ public class TestAsyncContextImplListenerOnComplete extends
TomcatBaseTest {
socket.connect(new InetSocketAddress("localhost", port));
try (Writer writer = new
OutputStreamWriter(socket.getOutputStream())) {
- writer.write("GET /repro" + SimpleHttpClient.CRLF +
- "Accept: text/event-stream" + SimpleHttpClient.CRLF +
- SimpleHttpClient.CRLF);
+ writer.write("GET /repro" + SimpleHttpClient.CRLF + "Accept:
text/event-stream" +
+ SimpleHttpClient.CRLF + SimpleHttpClient.CRLF);
writer.flush();
}
Thread.sleep(1_000);
@@ -98,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
@@ -116,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 d9f83d8786..8248679a4d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -121,6 +121,11 @@
addresses. Interfaces that are configured for point to point
connections
or are not currently up are now skipped. (markt)
</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: [email protected]
For additional commands, e-mail: [email protected]
