Improve handling of I/O errors during non-blocking writes
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java
index 8af8cd6..b39f07b 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -158,12 +158,11 @@
if (res.getWriteListener() != null) {
res.getWriteListener().onError(t);
}
+ res.action(ActionCode.CLOSE_NOW, t);
+ asyncConImpl.setErrorState(t, true);
} finally {
context.unbind(false, oldCL);
}
- if (t != null) {
- asyncConImpl.setErrorState(t, true);
- }
}
// Check to see if non-blocking writes or reads are being used
@@ -191,8 +190,8 @@
// Therefore no need to set success=false as that would trigger a
// second call to AbstractProcessor.setErrorState()
// https://bz.apache.org/bugzilla/show_bug.cgi?id=65001
- res.action(ActionCode.CLOSE_NOW, t);
writeListener.onError(t);
+ res.action(ActionCode.CLOSE_NOW, t);
asyncConImpl.setErrorState(t, true);
} finally {
context.unbind(false, oldCL);
@@ -224,8 +223,8 @@
// Therefore no need to set success=false as that would trigger a
// second call to AbstractProcessor.setErrorState()
// https://bz.apache.org/bugzilla/show_bug.cgi?id=65001
- res.action(ActionCode.CLOSE_NOW, t);
readListener.onError(t);
+ res.action(ActionCode.CLOSE_NOW, t);
asyncConImpl.setErrorState(t, true);
} finally {
context.unbind(false, oldCL);
diff --git a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
index 97c6a22..738b2aa 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -154,11 +154,13 @@
}
private void testNonBlockingWriteInternal(boolean keepAlive) throws Exception {
+ AtomicBoolean asyncContextIsComplete = new AtomicBoolean(false);
+
Tomcat tomcat = getTomcatInstance();
// No file system docBase required
Context ctx = tomcat.addContext("", null);
- NBWriteServlet servlet = new NBWriteServlet();
+ NBWriteServlet servlet = new NBWriteServlet(asyncContextIsComplete);
String servletName = NBWriteServlet.class.getName();
Tomcat.addServlet(ctx, servletName, servlet);
ctx.addServletMappingDecoded("/", servletName);
@@ -314,11 +316,25 @@
}
Assert.assertEquals(WRITE_SIZE, totalBodyRead);
+ Assert.assertTrue("AsyncContext should have been completed.", asyncContextIsComplete.get());
}
@Test
- public void testNonBlockingWriteError01() throws Exception {
+ public void testNonBlockingWriteError01ListenerComplete() throws Exception {
+ doTestNonBlockingWriteError01NoListenerComplete(true);
+ }
+
+
+ @Test
+ public void testNonBlockingWriteError01NoListenerComplete() throws Exception {
+ doTestNonBlockingWriteError01NoListenerComplete(false);
+ }
+
+
+ private void doTestNonBlockingWriteError01NoListenerComplete(boolean listenerCompletesOnError) throws Exception {
+ AtomicBoolean asyncContextIsComplete = new AtomicBoolean(false);
+
Tomcat tomcat = getTomcatInstance();
// No file system docBase required
@@ -330,7 +346,7 @@
// Some CI platforms appear to have particularly large write buffers
// and appear to ignore the socket.txBufSize below. Therefore, configure
// configure the Servlet to keep writing until an error is encountered.
- NBWriteServlet servlet = new NBWriteServlet(true);
+ NBWriteServlet servlet = new NBWriteServlet(asyncContextIsComplete, true, listenerCompletesOnError);
String servletName = NBWriteServlet.class.getName();
Tomcat.addServlet(ctx, servletName, servlet);
ctx.addServletMappingDecoded("/", servletName);
@@ -393,12 +409,18 @@
count ++;
}
+ while (count < 100 && !asyncContextIsComplete.get()) {
+ Thread.sleep(100);
+ count ++;
+ }
+
while (count < 100 && alv.getEntryCount() < 1) {
Thread.sleep(100);
count ++;
}
Assert.assertTrue("Error listener should have been invoked.", servlet.wlistener.onErrorInvoked);
+ Assert.assertTrue("Async context should have been completed.", asyncContextIsComplete.get());
// TODO Figure out why non-blocking writes with the NIO connector appear
// to be slower on Linux
@@ -547,16 +569,20 @@
@WebServlet(asyncSupported = true)
public static class NBWriteServlet extends TesterServlet {
private static final long serialVersionUID = 1L;
+ private final AtomicBoolean asyncContextIsComplete;
private final boolean unlimited;
+ private final boolean listenerCompletesOnError;
public transient volatile TestWriteListener wlistener;
- public NBWriteServlet() {
- this(false);
+ public NBWriteServlet(AtomicBoolean asyncContextIsComplete) {
+ this(asyncContextIsComplete, false, true);
}
- public NBWriteServlet(boolean unlimited) {
+ public NBWriteServlet(AtomicBoolean asyncContextIsComplete, boolean unlimited, boolean listenerCompletesOnError) {
+ this.asyncContextIsComplete = asyncContextIsComplete;
this.unlimited = unlimited;
+ this.listenerCompletesOnError = listenerCompletesOnError;
}
@@ -580,11 +606,15 @@
@Override
public void onError(AsyncEvent event) throws IOException {
log.info("AsyncListener.onError");
+ if (listenerCompletesOnError) {
+ event.getAsyncContext().complete();
+ }
}
@Override
public void onComplete(AsyncEvent event) throws IOException {
log.info("onComplete");
+ asyncContextIsComplete.set(true);
}
});
// step 2 - notify on read
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 370572e..826b326 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -189,6 +189,11 @@
Make handling of OpenSSL read errors more robust when plain text data is
reported to be available to read. (markt)
</fix>
+ <fix>
+ Correct handling of write errors during non-blocking I/O to ensure that
+ the associated <code>AsyncContext</code> was closed down correctly.
+ (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
