This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new 582ff39 Improve handling of I/O errors during non-blocking writes
582ff39 is described below
commit 582ff396768c911ddacc35bc7b5bdaad537d8fea
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Mar 3 15:12:51 2021 +0000
Improve handling of I/O errors during non-blocking writes
---
.../apache/catalina/connector/CoyoteAdapter.java | 9 +++--
.../catalina/nonblocking/TestNonBlockingAPI.java | 42 ++++++++++++++++++----
webapps/docs/changelog.xml | 5 +++
3 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java
b/java/org/apache/catalina/connector/CoyoteAdapter.java
index 1c4d694..9d06af9 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -169,12 +169,11 @@ public class CoyoteAdapter implements Adapter {
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
@@ -202,8 +201,8 @@ public class CoyoteAdapter implements Adapter {
// 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);
@@ -235,8 +234,8 @@ public class CoyoteAdapter implements Adapter {
// 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 cd8321e..6217083 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -153,11 +153,13 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
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);
@@ -313,11 +315,25 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
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
@@ -329,7 +345,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
// 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);
@@ -392,12 +408,18 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
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
@@ -546,16 +568,20 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
@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;
}
@@ -579,11 +605,15 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
@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 eeca5be..880dad9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -163,6 +163,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">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
