This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 1e95c5a414bba99d0f7693dea65b84af01e8fe26
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Feb 24 17:11:16 2021 +0000
Ensure AsyncListener.onError() is triggered for a non-blocking IO error
If the non-blocking I/O error occurred in onWritePossible() or
onDataAvailable() (rather than when buffered data was being written in
the background) the error was not propogated to the AsyncListener.
---
.../apache/catalina/connector/CoyoteAdapter.java | 2 +
.../catalina/nonblocking/TestNonBlockingAPI.java | 169 +++++++++++++++++++--
webapps/docs/changelog.xml | 5 +
3 files changed, 163 insertions(+), 13 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java
b/java/org/apache/catalina/connector/CoyoteAdapter.java
index e470ed9..e9100e1 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -188,6 +188,7 @@ public class CoyoteAdapter implements Adapter {
// https://bz.apache.org/bugzilla/show_bug.cgi?id=65001
res.action(ActionCode.CLOSE_NOW, t);
writeListener.onError(t);
+ asyncConImpl.setErrorState(t, true);
} finally {
request.getContext().unbind(false, oldCL);
}
@@ -215,6 +216,7 @@ public class CoyoteAdapter implements Adapter {
// https://bz.apache.org/bugzilla/show_bug.cgi?id=65001
res.action(ActionCode.CLOSE_NOW, t);
readListener.onError(t);
+ asyncConImpl.setErrorState(t, true);
} finally {
request.getContext().unbind(false, oldCL);
}
diff --git a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
index 6bb20ee..7f4f930 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -318,7 +318,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
@Test
- public void testNonBlockingWriteError() throws Exception {
+ public void testNonBlockingWriteError01() throws Exception {
Tomcat tomcat = getTomcatInstance();
// No file system docBase required
@@ -388,8 +388,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
// Listeners are invoked and access valve entries created on a
different
// thread so give that thread a chance to complete its work.
int count = 0;
- while (count < 100 &&
- !(servlet.wlistener.onErrorInvoked ||
servlet.rlistener.onErrorInvoked)) {
+ while (count < 100 && !servlet.wlistener.onErrorInvoked) {
Thread.sleep(100);
count ++;
}
@@ -399,8 +398,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
count ++;
}
- Assert.assertTrue("Error listener should have been invoked.",
- servlet.wlistener.onErrorInvoked ||
servlet.rlistener.onErrorInvoked);
+ Assert.assertTrue("Error listener should have been invoked.",
servlet.wlistener.onErrorInvoked);
// TODO Figure out why non-blocking writes with the NIO connector
appear
// to be slower on Linux
@@ -551,7 +549,6 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
private static final long serialVersionUID = 1L;
private final boolean unlimited;
public transient volatile TestWriteListener wlistener;
- public transient volatile TestReadListener rlistener;
public NBWriteServlet() {
this(false);
@@ -591,9 +588,6 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
});
// step 2 - notify on read
- ServletInputStream in = req.getInputStream();
- rlistener = new TestReadListener(actx, true, false);
- in.setReadListener(rlistener);
ServletOutputStream out = resp.getOutputStream();
resp.setBufferSize(200 * 1024);
wlistener = new TestWriteListener(actx, unlimited);
@@ -626,7 +620,6 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
protected final boolean usingNonBlockingWrite;
protected final boolean ignoreIsReady;
protected final StringBuilder body = new StringBuilder();
- public volatile boolean onErrorInvoked = false;
public TestReadListener(AsyncContext ctx,
@@ -679,7 +672,6 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
public void onError(Throwable throwable) {
log.info("ReadListener.onError totalData=" +
body.toString().length());
throwable.printStackTrace();
- onErrorInvoked = true;
}
}
@@ -1182,7 +1174,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
tomcat.start();
- PostClient client = new PostClient();
+ ResponseOKClient client = new ResponseOKClient();
client.setPort(getPort());
client.setRequest(request);
client.connect();
@@ -1199,7 +1191,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
- private static final class PostClient extends SimpleHttpClient {
+ private static final class ResponseOKClient extends SimpleHttpClient {
@Override
public boolean isResponseBodyOK() {
@@ -1320,4 +1312,155 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
ac.complete();
}
}
+
+
+ /*
+ * Tests client disconnect in the following scenario:
+ * - async with non-blocking IO
+ * - response has been committed
+ * - no data in buffers
+ * - client disconnects
+ * - server attempts a write
+ */
+ @Test
+ public void testNonBlockingWriteError02() throws Exception {
+ CountDownLatch responseCommitLatch = new CountDownLatch(1);
+ CountDownLatch clientCloseLatch = new CountDownLatch(1);
+ CountDownLatch asyncCompleteLatch = new CountDownLatch(1);
+
+ // Setup Tomcat instance
+ Tomcat tomcat = getTomcatInstance();
+
+ // No file system docBase required
+ Context ctx = tomcat.addContext("", null);
+
+ NBWriteServlet02 writeServlet = new
NBWriteServlet02(responseCommitLatch, clientCloseLatch, asyncCompleteLatch);
+ Wrapper wrapper = Tomcat.addServlet(ctx, "writeServlet", writeServlet);
+ wrapper.setAsyncSupported(true);
+ ctx.addServletMappingDecoded("/*", "writeServlet");
+
+ tomcat.start();
+
+ ResponseOKClient client = new ResponseOKClient();
+ client.setPort(getPort());
+ client.setRequest(new String[] {
+ "GET / HTTP/1.1" + SimpleHttpClient.CRLF +
+ "Host: localhost:" + SimpleHttpClient.CRLF +
+ SimpleHttpClient.CRLF
+ });
+ client.connect();
+ client.sendRequest();
+
+ responseCommitLatch.await();
+
+ client.disconnect();
+ clientCloseLatch.countDown();
+
+ Assert.assertTrue("Failed to complete async processing",
asyncCompleteLatch.await(10, TimeUnit.SECONDS));
+ }
+
+
+ private static class NBWriteServlet02 extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private final CountDownLatch responseCommitLatch;
+ private final CountDownLatch clientCloseLatch;
+ private final CountDownLatch asyncCompleteLatch;
+
+ public NBWriteServlet02(CountDownLatch responseCommitLatch,
CountDownLatch clientCloseLatch,
+ CountDownLatch asyncCompleteLatch) {
+ this.responseCommitLatch = responseCommitLatch;
+ this.clientCloseLatch = clientCloseLatch;
+ this.asyncCompleteLatch = asyncCompleteLatch;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException {
+ resp.setContentType("text/plain");
+ resp.setCharacterEncoding("UTF-8");
+
+ AsyncContext ac = req.startAsync();
+ ac.addListener(new TestAsyncListener02(asyncCompleteLatch));
+ ac.setTimeout(5000);
+
+ WriteListener writeListener = new TestWriteListener02(ac,
responseCommitLatch, clientCloseLatch);
+ resp.getOutputStream().setWriteListener(writeListener);
+ }
+ }
+
+
+ private static class TestAsyncListener02 implements AsyncListener {
+
+ private final CountDownLatch asyncCompleteLatch;
+
+ public TestAsyncListener02(CountDownLatch asyncCompleteLatch) {
+ this.asyncCompleteLatch = asyncCompleteLatch;
+ }
+
+ @Override
+ public void onComplete(AsyncEvent event) throws IOException {
+ asyncCompleteLatch.countDown();
+ }
+
+ @Override
+ public void onTimeout(AsyncEvent event) throws IOException {
+ // NO-OP
+ }
+
+ @Override
+ public void onError(AsyncEvent event) throws IOException {
+ // NO-OP
+ }
+
+ @Override
+ public void onStartAsync(AsyncEvent event) throws IOException {
+ // NO-OP
+ }
+
+ }
+
+ private static class TestWriteListener02 implements WriteListener {
+
+ private final AsyncContext ac;
+ private final CountDownLatch responseCommitLatch;
+ private final CountDownLatch clientCloseLatch;
+ private volatile int stage = 0;
+
+ public TestWriteListener02(AsyncContext ac, CountDownLatch
responseCommitLatch,
+ CountDownLatch clientCloseLatch) {
+ this.ac = ac;
+ this.responseCommitLatch = responseCommitLatch;
+ this.clientCloseLatch = clientCloseLatch;
+ }
+
+ @Override
+ public void onWritePossible() throws IOException {
+ ServletOutputStream sos = ac.getResponse().getOutputStream();
+ do {
+ if (stage == 0) {
+ // Commit the response
+ ac.getResponse().flushBuffer();
+ responseCommitLatch.countDown();
+ stage++;
+ } else if (stage == 1) {
+ // Wait for the client to drop the connection
+ try {
+ clientCloseLatch.await();
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ sos.print("TEST");
+ stage++;
+ } else if (stage == 2) {
+ sos.flush();
+ }
+ } while (sos.isReady());
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ // NO-OP
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4a8cd19..fd5b4e6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -141,6 +141,11 @@
<code>setLocale()</code> with the recent clarification from the Jakarta
Servlet project of the expected behaviour in these cases. (markt)
</fix>
+ <fix>
+ Ensure that the <code>AsyncListener.onError()</code> event is triggered
+ when a I/O error occurs during non-blocking I/O. There were some cases
+ discovered where this was not happening. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
