This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d205d9457f49598f6bc06a9563e04b6fe029e7a0 Author: Mark Thomas <[email protected]> AuthorDate: Fri Nov 6 19:00:52 2026 +0000 Write buffered output when async processing ends in an error state BZ70290. When async processing is ended early because the processor is in an error state, any output still held in the Catalina response buffer was discarded when the response was recycled, even if the error state (CLOSE_CLEAN) still permits I/O and the response could have been completed before the connection is closed. - CoyoteAdapter: when the error state permits I/O, finish the request and response before the early post-processing, as the normal completion path would have done. Applies to service() and asyncDispatch(). - AsyncStateMachine: when the error state permits I/O, flush buffered output before completing a pending dispatch, as the MUST_COMPLETE and DISPATCHING paths do. Add tests for the output being delivered and for the flush behaviour. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> --- .../apache/catalina/connector/CoyoteAdapter.java | 22 ++++ java/org/apache/coyote/AsyncStateMachine.java | 8 +- .../TestCoyoteAdapterAsyncErrorState.java | 114 +++++++++++++++++++-- test/org/apache/coyote/TestAsyncStateMachine.java | 66 +++++++++++- webapps/docs/changelog.xml | 4 +- 5 files changed, 204 insertions(+), 10 deletions(-) diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java index ded5606294..7f09798129 100644 --- a/java/org/apache/catalina/connector/CoyoteAdapter.java +++ b/java/org/apache/catalina/connector/CoyoteAdapter.java @@ -257,6 +257,7 @@ public class CoyoteAdapter implements Adapter { if (request.isAsyncCompleting() || request.isAsyncDispatching()) { // Connection will be forcibly closed which will prevent completion/dispatch happening at the usual // point. Trigger post-processing here. + finishResponseIfIoAllowed(request, response, res); res.action(ActionCode.ASYNC_POST_PROCESS, null); } success = false; @@ -388,6 +389,7 @@ public class CoyoteAdapter implements Adapter { // completion or the dispatch requested during error handling // from happening at the usual point. Need to trigger the call // to onComplete() here. + finishResponseIfIoAllowed(request, response, res); res.action(ActionCode.ASYNC_POST_PROCESS, null); async = false; } @@ -431,6 +433,26 @@ public class CoyoteAdapter implements Adapter { } + /* + * Called when the processor is in an error state and the connection will be closed before an async complete or + * dispatch can take place at the usual point. If the error state still permits IO (e.g. CLOSE_CLEAN) the current + * response can be completed before the connection is closed. Output that is still held in the response buffer + * would be discarded when the response is recycled so write it now, as the normal completion path would have done. + */ + private void finishResponseIfIoAllowed(Request request, Response response, org.apache.coyote.Response res) { + AtomicBoolean ioAllowed = new AtomicBoolean(false); + res.action(ActionCode.IS_IO_ALLOWED, ioAllowed); + if (ioAllowed.get()) { + try { + request.finishRequest(); + response.finishResponse(); + } catch (IOException ignore) { + // Issues that should be logged will have already been logged + } + } + } + + private void updateWrapperErrorCount(Request request, Response response) { if (response.isError()) { Wrapper wrapper = request.getWrapper(); diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java index 466e72aa18..e430df62a0 100644 --- a/java/org/apache/coyote/AsyncStateMachine.java +++ b/java/org/apache/coyote/AsyncStateMachine.java @@ -313,7 +313,13 @@ class AsyncStateMachine { // call to dispatch() from AsyncListener.onError()) can no longer be // performed. Complete the async processing here so that // onComplete() is called and the in-progress async count is - // balanced, rather than leaving the request in MUST_DISPATCH. + // balanced, rather than leaving the request in MUST_DISPATCH. If IO + // is still allowed (CLOSE_CLEAN) the current response can still be + // completed before the connection is closed so write any buffered + // output first. + if (processor.getErrorState().isIoAllowed() && processor.flushBufferedWrite()) { + return SocketState.LONG; + } asyncCtxt.fireOnComplete(); updateState(AsyncState.DISPATCHED); asyncCtxt.decrementInProgressAsyncCount(); diff --git a/test/org/apache/catalina/connector/TestCoyoteAdapterAsyncErrorState.java b/test/org/apache/catalina/connector/TestCoyoteAdapterAsyncErrorState.java index db3888665c..85ff3f1756 100644 --- a/test/org/apache/catalina/connector/TestCoyoteAdapterAsyncErrorState.java +++ b/test/org/apache/catalina/connector/TestCoyoteAdapterAsyncErrorState.java @@ -153,6 +153,71 @@ public class TestCoyoteAdapterAsyncErrorState extends TomcatBaseTest { } + /* + * BZ 70290: Output written to the response, but not yet flushed, before complete() was called must still be + * delivered to the client when the error state (CLOSE_CLEAN) permits IO. It must not be discarded when the request + * is recycled. + */ + @Test + public void testBz70290CompleteAfterCloseCleanDeliversBufferedOutput() throws Exception { + TestServlet servlet = new TestServlet(ActionCode.DISABLE_SWALLOW_INPUT, true).withOutput("BUFFERED-OUTPUT"); + startTomcat(servlet); + + String response = sendRequest(); + + Assert.assertTrue(response, response.contains("BUFFERED-OUTPUT")); + assertCompletedOnce(servlet); + } + + + /* + * BZ 70290: As above but the dispatch requested by the application can not be performed because the connection is + * going to be closed. The output already written must still be delivered. + */ + @Test + public void testBz70290DispatchAfterCloseCleanDeliversBufferedOutput() throws Exception { + TestServlet servlet = new TestServlet(ActionCode.DISABLE_SWALLOW_INPUT).withOutput("BUFFERED-OUTPUT"); + startTomcat(servlet); + + String response = sendRequest(); + + Assert.assertTrue(response, response.contains("BUFFERED-OUTPUT")); + assertCompletedOnce(servlet); + } + + + /* + * BZ 70290: Control. With no error the buffered output is delivered when complete() is called. + */ + @Test + public void testBz70290CompleteWithoutErrorDeliversBufferedOutput() throws Exception { + TestServlet servlet = new TestServlet(null, true).withOutput("BUFFERED-OUTPUT"); + startTomcat(servlet); + + String response = sendRequest(); + + Assert.assertTrue(response, response.contains("BUFFERED-OUTPUT")); + assertCompletedOnce(servlet); + } + + + /* + * BZ 70290: As above but complete() is called from AsyncListener.onTimeout() so the request is processed by + * CoyoteAdapter.asyncDispatch() rather than by the container thread that made the initial service call. + */ + @Test + public void testBz70290CompleteFromTimeoutAfterCloseCleanDeliversBufferedOutput() throws Exception { + TestServlet servlet = new TestServlet(ActionCode.DISABLE_SWALLOW_INPUT, true).withOutput("BUFFERED-OUTPUT") + .completeFromTimeout(); + startTomcat(servlet); + + String response = sendRequest(); + + Assert.assertTrue(response, response.contains("BUFFERED-OUTPUT")); + assertCompletedOnce(servlet); + } + + private void startTomcat(TestServlet servlet) throws Exception { Tomcat tomcat = getTomcatInstance(); Context ctx = getProgrammaticRootContext(); @@ -222,6 +287,8 @@ public class TestCoyoteAdapterAsyncErrorState extends TomcatBaseTest { private final ActionCode errorAction; private final boolean complete; + private volatile String output; + private volatile boolean completeFromTimeout; final AtomicInteger dispatches = new AtomicInteger(); final AtomicInteger completions = new AtomicInteger(); @@ -231,6 +298,24 @@ public class TestCoyoteAdapterAsyncErrorState extends TomcatBaseTest { this(errorAction, false); } + /* + * Write the given output to the response before the error occurs. It is deliberately not flushed so it is + * still held in the Catalina response buffer when the error occurs. + */ + TestServlet withOutput(String output) { + this.output = output; + return this; + } + + /* + * Defer the output, the error and the call to complete() to AsyncListener.onTimeout() so they happen after the + * initial service call has finished and are processed via CoyoteAdapter.asyncDispatch(). + */ + TestServlet completeFromTimeout() { + this.completeFromTimeout = true; + return this; + } + TestServlet(ActionCode errorAction, boolean complete) { this.errorAction = errorAction; this.complete = complete; @@ -254,8 +339,11 @@ public class TestCoyoteAdapterAsyncErrorState extends TomcatBaseTest { } @Override - public void onTimeout(AsyncEvent event) { - // NO-OP + public void onTimeout(AsyncEvent event) throws IOException { + if (completeFromTimeout) { + writeOutputAndFail(req, resp); + ac.complete(); + } } @Override @@ -269,12 +357,13 @@ public class TestCoyoteAdapterAsyncErrorState extends TomcatBaseTest { } }); - if (errorAction != null) { - // The connection fails - Request request = (Request) req.getAttribute(REQUEST_ATTRIBUTE); - request.getCoyoteRequest().action(errorAction, null); + if (completeFromTimeout) { + ac.setTimeout(100); + return; } + writeOutputAndFail(req, resp); + // Request the dispatch (or completion) on the container thread during the initial service call if (complete) { ac.complete(); @@ -282,5 +371,18 @@ public class TestCoyoteAdapterAsyncErrorState extends TomcatBaseTest { ac.dispatch(); } } + + private void writeOutputAndFail(HttpServletRequest req, HttpServletResponse resp) throws IOException { + if (output != null) { + resp.setContentType("text/plain"); + resp.getWriter().print(output); + } + + if (errorAction != null) { + // The connection fails + Request request = (Request) req.getAttribute(REQUEST_ATTRIBUTE); + request.getCoyoteRequest().action(errorAction, null); + } + } } } diff --git a/test/org/apache/coyote/TestAsyncStateMachine.java b/test/org/apache/coyote/TestAsyncStateMachine.java index 89cfa10124..423269457e 100644 --- a/test/org/apache/coyote/TestAsyncStateMachine.java +++ b/test/org/apache/coyote/TestAsyncStateMachine.java @@ -154,6 +154,52 @@ public class TestAsyncStateMachine { } + /* + * BZ 70290: CLOSE_CLEAN permits IO so the current response can be completed before the connection is closed. Any + * buffered output must be written before onComplete() is called, as it is for MUST_COMPLETE and DISPATCHING. While + * output remains the state machine must stay in the long poll without completing. + */ + @Test + public void testBz70290DispatchWithCloseCleanFlushesBufferedOutputBeforeCompleting() throws IOException { + processor.setErrorState(ErrorState.CLOSE_CLEAN, null); + processor.action(ActionCode.ASYNC_DISPATCH, null); + processor.setPendingFlushes(2); + + // Output remains so processing must not end yet + Assert.assertEquals(SocketState.LONG, processor.asyncPostProcess()); + Assert.assertTrue("Async processing must not have ended", isAsync()); + Assert.assertEquals(0, asyncCtxt.getCompleteCount()); + Assert.assertEquals(0, asyncCtxt.getDecrementCount()); + + // Still output remaining + Assert.assertEquals(SocketState.LONG, processor.asyncPostProcess()); + Assert.assertEquals(0, asyncCtxt.getCompleteCount()); + Assert.assertEquals(0, asyncCtxt.getDecrementCount()); + + // Output has been written in full so processing can now end + Assert.assertEquals(SocketState.ASYNC_END, processor.asyncPostProcess()); + assertCompleted(); + Assert.assertEquals(3, processor.getFlushCalls()); + } + + + /* + * BZ 70290: When IO is not allowed buffered output can not be written so it must not be attempted and processing + * must end immediately. + */ + @Test + public void testBz70290DispatchWithCloseNowDoesNotFlushBufferedOutput() throws IOException { + processor.setErrorState(ErrorState.CLOSE_NOW, new IOException()); + processor.action(ActionCode.ASYNC_DISPATCH, null); + processor.setPendingFlushes(2); + + Assert.assertEquals(SocketState.ASYNC_END, processor.asyncPostProcess()); + + assertCompleted(); + Assert.assertEquals("No flush must be attempted", 0, processor.getFlushCalls()); + } + + /* * BZ 70290: Control. The equivalent complete() handling (MUST_COMPLETE) in an error state already balanced the * count. Confirm the dispatch and complete cases behave consistently. @@ -246,10 +292,26 @@ public class TestAsyncStateMachine { // NO-OP. Tests drive the state machine directly rather than via a socket. } + private int pendingFlushes; + private int flushCalls; + + /** + * Simulate buffered output that needs the given number of calls to {@link #flushBufferedWrite()} before it has + * been written in full. + */ + void setPendingFlushes(int pendingFlushes) { + this.pendingFlushes = pendingFlushes; + } + + int getFlushCalls() { + return flushCalls; + } + @Override protected boolean flushBufferedWrite() throws IOException { - // There is no socket so there is never any buffered data to flush - return false; + // There is no socket so there is only buffered data if the test has asked for it + flushCalls++; + return pendingFlushes-- > 0; } @Override diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 034b326e1b..ffacb0131c 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -351,7 +351,9 @@ <code>AsyncContext.dispatch()</code>), the request could be recycled while still in the <code>MUST_DISPATCH</code> state so that <code>onComplete()</code> was not called and the in-progress async - count was not decremented. (markt) + count was not decremented. In addition, when the error state still + permits I/O, output buffered before the request was completed is now + written rather than being discarded. (markt) </fix> </changelog> </subsection> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
