You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/02/25 13:46:12 UTC
[tomcat] branch 8.5.x updated (c371676 -> 5ba8065)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from c371676 Make sure all errors from OpenSSL are fully cleared
new 0ff1ed3 Avoid attempting to write an error report if I/O is not allowed
new c7d8223 Ensure AsyncListener.onError() is triggered for a non-blocking IO error
new 0a11905 Improve error handling if non-blocking IO code swallows IOExceptions
new 5ba8065 Avoid possible NPE during shutdown for some error conditions
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/catalina/connector/CoyoteAdapter.java | 25 ++-
.../org/apache/catalina/connector/InputBuffer.java | 57 +++---
.../apache/catalina/connector/OutputBuffer.java | 1 +
.../apache/catalina/valves/ErrorReportValve.java | 28 +--
java/org/apache/coyote/Request.java | 33 ++++
java/org/apache/coyote/Response.java | 11 +-
.../catalina/nonblocking/TestNonBlockingAPI.java | 192 +++++++++++++++++++--
webapps/docs/changelog.xml | 11 ++
8 files changed, 289 insertions(+), 69 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/04: Ensure AsyncListener.onError() is triggered for a
non-blocking IO error
Posted by ma...@apache.org.
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
commit c7d8223a056afa20babecf013fea34234d7e4659
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 141fde7..d8359ec 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -199,6 +199,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);
}
@@ -226,6 +227,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 dc0fd04..bf1635d 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -317,7 +317,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
@@ -387,8 +387,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 ++;
}
@@ -398,8 +397,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
@@ -550,7 +548,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);
@@ -590,9 +587,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);
@@ -625,7 +619,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,
@@ -678,7 +671,6 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
public void onError(Throwable throwable) {
log.info("ReadListener.onError totalData=" + body.toString().length());
throwable.printStackTrace();
- onErrorInvoked = true;
}
}
@@ -1185,7 +1177,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
tomcat.start();
- PostClient client = new PostClient();
+ ResponseOKClient client = new ResponseOKClient();
client.setPort(getPort());
client.setRequest(request);
client.connect();
@@ -1202,7 +1194,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
- private static final class PostClient extends SimpleHttpClient {
+ private static final class ResponseOKClient extends SimpleHttpClient {
@Override
public boolean isResponseBodyOK() {
@@ -1323,4 +1315,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 8724241..1859a3d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -110,6 +110,11 @@
Revert an incorrect fix for a potential resource leak that broke
deployment via the Ant deploy task. (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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 04/04: Avoid possible NPE during shutdown for some error
conditions
Posted by ma...@apache.org.
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
commit 5ba8065198178668bc17f2edd6068b7b8d03132e
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Feb 25 10:01:51 2021 +0000
Avoid possible NPE during shutdown for some error conditions
---
java/org/apache/catalina/connector/CoyoteAdapter.java | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java
index 325f2cd..55dfefb 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -160,9 +160,10 @@ public class CoyoteAdapter implements Adapter {
success = false;
Throwable t = (Throwable)req.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
req.getAttributes().remove(RequestDispatcher.ERROR_EXCEPTION);
+ Context context = request.getContext();
ClassLoader oldCL = null;
try {
- oldCL = request.getContext().bind(false, null);
+ oldCL = context.bind(false, null);
if (req.getReadListener() != null) {
req.getReadListener().onError(t);
}
@@ -170,7 +171,7 @@ public class CoyoteAdapter implements Adapter {
res.getWriteListener().onError(t);
}
} finally {
- request.getContext().unbind(false, oldCL);
+ context.unbind(false, oldCL);
}
if (t != null) {
asyncConImpl.setErrorState(t, true);
@@ -182,9 +183,10 @@ public class CoyoteAdapter implements Adapter {
WriteListener writeListener = res.getWriteListener();
ReadListener readListener = req.getReadListener();
if (writeListener != null && status == SocketEvent.OPEN_WRITE) {
+ Context context = request.getContext();
ClassLoader oldCL = null;
try {
- oldCL = request.getContext().bind(false, null);
+ oldCL = context.bind(false, null);
res.onWritePossible();
if (request.isFinished() && req.sendAllDataReadEvent() &&
readListener != null) {
@@ -205,12 +207,13 @@ public class CoyoteAdapter implements Adapter {
writeListener.onError(t);
asyncConImpl.setErrorState(t, true);
} finally {
- request.getContext().unbind(false, oldCL);
+ context.unbind(false, oldCL);
}
} else if (readListener != null && status == SocketEvent.OPEN_READ) {
+ Context context = request.getContext();
ClassLoader oldCL = null;
try {
- oldCL = request.getContext().bind(false, null);
+ oldCL = context.bind(false, null);
// If data is being read on a non-container thread a
// dispatch with status OPEN_READ will be used to get
// execution back on a container thread for the
@@ -237,7 +240,7 @@ public class CoyoteAdapter implements Adapter {
readListener.onError(t);
asyncConImpl.setErrorState(t, true);
} finally {
- request.getContext().unbind(false, oldCL);
+ context.unbind(false, oldCL);
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 03/04: Improve error handling if non-blocking IO code
swallows IOExceptions
Posted by ma...@apache.org.
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
commit 0a11905b67fdff58f4efa77fb099ca9967a709b3
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Feb 24 18:43:22 2021 +0000
Improve error handling if non-blocking IO code swallows IOExceptions
Tomcat previously expected IOExceptions in onWritePossible() and
onDataAvailable() to not be swallowed. These additions allow correct
error handling even if the application code swallows the exception.
---
.../apache/catalina/connector/CoyoteAdapter.java | 8 +++
.../org/apache/catalina/connector/InputBuffer.java | 57 +++++++----------
.../apache/catalina/connector/OutputBuffer.java | 1 +
java/org/apache/coyote/Request.java | 33 ++++++++++
java/org/apache/coyote/Response.java | 11 ++--
.../catalina/nonblocking/TestNonBlockingAPI.java | 73 ++++++++++++++--------
webapps/docs/changelog.xml | 6 ++
7 files changed, 124 insertions(+), 65 deletions(-)
diff --git a/java/org/apache/catalina/connector/CoyoteAdapter.java b/java/org/apache/catalina/connector/CoyoteAdapter.java
index d8359ec..325f2cd 100644
--- a/java/org/apache/catalina/connector/CoyoteAdapter.java
+++ b/java/org/apache/catalina/connector/CoyoteAdapter.java
@@ -190,6 +190,10 @@ public class CoyoteAdapter implements Adapter {
readListener != null) {
readListener.onAllDataRead();
}
+ // User code may have swallowed an IOException
+ if (response.getCoyoteResponse().isExceptionPresent()) {
+ throw response.getCoyoteResponse().getErrorException();
+ }
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
// Need to trigger the call to AbstractProcessor.setErrorState()
@@ -218,6 +222,10 @@ public class CoyoteAdapter implements Adapter {
if (request.isFinished() && req.sendAllDataReadEvent()) {
readListener.onAllDataRead();
}
+ // User code may have swallowed an IOException
+ if (request.getCoyoteRequest().isExceptionPresent()) {
+ throw request.getCoyoteRequest().getErrorException();
+ }
} catch (Throwable t) {
ExceptionUtils.handleThrowable(t);
// Need to trigger the call to AbstractProcessor.setErrorState()
diff --git a/java/org/apache/catalina/connector/InputBuffer.java b/java/org/apache/catalina/connector/InputBuffer.java
index 2da5fca..8393e09 100644
--- a/java/org/apache/catalina/connector/InputBuffer.java
+++ b/java/org/apache/catalina/connector/InputBuffer.java
@@ -343,6 +343,7 @@ public class InputBuffer extends Reader
try {
return coyoteRequest.doRead(this);
} catch (IOException ioe) {
+ coyoteRequest.setErrorException(ioe);
// An IOException on a read is almost always due to
// the remote client aborting the request.
throw new ClientAbortException(ioe);
@@ -351,9 +352,7 @@ public class InputBuffer extends Reader
public int readByte() throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (checkByteBufferEof()) {
return -1;
@@ -363,9 +362,7 @@ public class InputBuffer extends Reader
public int read(byte[] b, int off, int len) throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (checkByteBufferEof()) {
return -1;
@@ -388,9 +385,7 @@ public class InputBuffer extends Reader
* @throws IOException if an input or output exception has occurred
*/
public int read(ByteBuffer to) throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (checkByteBufferEof()) {
return -1;
@@ -456,10 +451,7 @@ public class InputBuffer extends Reader
@Override
public int read() throws IOException {
-
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (checkCharBufferEof()) {
return -1;
@@ -470,21 +462,14 @@ public class InputBuffer extends Reader
@Override
public int read(char[] cbuf) throws IOException {
-
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
-
+ throwIfClosed();
return read(cbuf, 0, cbuf.length);
}
@Override
public int read(char[] cbuf, int off, int len) throws IOException {
-
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (checkCharBufferEof()) {
return -1;
@@ -497,9 +482,7 @@ public class InputBuffer extends Reader
@Override
public long skip(long n) throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (n < 0) {
throw new IllegalArgumentException();
@@ -525,9 +508,7 @@ public class InputBuffer extends Reader
@Override
public boolean ready() throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (state == INITIAL_STATE) {
state = CHAR_STATE;
}
@@ -544,9 +525,7 @@ public class InputBuffer extends Reader
@Override
public void mark(int readAheadLimit) throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (cb.remaining() <= 0) {
clear(cb);
@@ -564,15 +543,15 @@ public class InputBuffer extends Reader
@Override
public void reset() throws IOException {
- if (closed) {
- throw new IOException(sm.getString("inputBuffer.streamClosed"));
- }
+ throwIfClosed();
if (state == CHAR_STATE) {
if (markPos < 0) {
clear(cb);
markPos = -1;
- throw new IOException();
+ IOException ioe = new IOException();
+ coyoteRequest.setErrorException(ioe);
+ throw ioe;
} else {
cb.position(markPos);
}
@@ -582,6 +561,14 @@ public class InputBuffer extends Reader
}
+ private void throwIfClosed() throws IOException {
+ if (closed) {
+ IOException ioe = new IOException(sm.getString("inputBuffer.streamClosed"));
+ coyoteRequest.setErrorException(ioe);
+ throw ioe;
+ }
+ }
+
public void checkConverter() throws IOException {
if (conv != null) {
return;
diff --git a/java/org/apache/catalina/connector/OutputBuffer.java b/java/org/apache/catalina/connector/OutputBuffer.java
index 6e0e951..df5db81 100644
--- a/java/org/apache/catalina/connector/OutputBuffer.java
+++ b/java/org/apache/catalina/connector/OutputBuffer.java
@@ -369,6 +369,7 @@ public class OutputBuffer extends Writer {
// An IOException on a write is almost always due to
// the remote client aborting the request. Wrap this
// so that it can be handled better by the error dispatcher.
+ coyoteResponse.setErrorException(e);
throw new ClientAbortException(e);
}
}
diff --git a/java/org/apache/coyote/Request.java b/java/org/apache/coyote/Request.java
index e7c943b..35189e8 100644
--- a/java/org/apache/coyote/Request.java
+++ b/java/org/apache/coyote/Request.java
@@ -161,6 +161,11 @@ public final class Request {
private boolean sendfile = true;
+ /**
+ * Holds request body reading error exception.
+ */
+ private Exception errorException = null;
+
volatile ReadListener listener;
public ReadListener getReadListener() {
@@ -595,6 +600,34 @@ public final class Request {
}
+ // -------------------- Error tracking --------------------
+
+ /**
+ * Set the error Exception that occurred during the writing of the response
+ * processing.
+ *
+ * @param ex The exception that occurred
+ */
+ public void setErrorException(Exception ex) {
+ errorException = ex;
+ }
+
+
+ /**
+ * Get the Exception that occurred during the writing of the response.
+ *
+ * @return The exception that occurred
+ */
+ public Exception getErrorException() {
+ return errorException;
+ }
+
+
+ public boolean isExceptionPresent() {
+ return errorException != null;
+ }
+
+
// -------------------- debug --------------------
@Override
diff --git a/java/org/apache/coyote/Response.java b/java/org/apache/coyote/Response.java
index 536f50f..1f965bb 100644
--- a/java/org/apache/coyote/Response.java
+++ b/java/org/apache/coyote/Response.java
@@ -122,9 +122,9 @@ public final class Response {
private long commitTime = -1;
/**
- * Holds request error exception.
+ * Holds response writing error exception.
*/
- Exception errorException = null;
+ private Exception errorException = null;
/**
* With the introduction of async processing and the possibility of
@@ -272,7 +272,8 @@ public final class Response {
// -----------------Error State --------------------
/**
- * Set the error Exception that occurred during request processing.
+ * Set the error Exception that occurred during the writing of the response
+ * processing.
*
* @param ex The exception that occurred
*/
@@ -282,7 +283,7 @@ public final class Response {
/**
- * Get the Exception that occurred during request processing.
+ * Get the Exception that occurred during the writing of the response.
*
* @return The exception that occurred
*/
@@ -292,7 +293,7 @@ public final class Response {
public boolean isExceptionPresent() {
- return ( errorException != null );
+ return errorException != null;
}
diff --git a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
index bf1635d..c612c4a 100644
--- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
+++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
@@ -1317,6 +1317,18 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
}
+ @Test
+ public void testNonBlockingWriteError02NoSwallow() throws Exception {
+ doTestNonBlockingWriteError02(false);
+ }
+
+
+ @Test
+ public void testNonBlockingWriteError02Swallow() throws Exception {
+ doTestNonBlockingWriteError02(true);
+ }
+
+
/*
* Tests client disconnect in the following scenario:
* - async with non-blocking IO
@@ -1325,8 +1337,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
* - client disconnects
* - server attempts a write
*/
- @Test
- public void testNonBlockingWriteError02() throws Exception {
+ private void doTestNonBlockingWriteError02(boolean swallowIoException) throws Exception {
CountDownLatch responseCommitLatch = new CountDownLatch(1);
CountDownLatch clientCloseLatch = new CountDownLatch(1);
CountDownLatch asyncCompleteLatch = new CountDownLatch(1);
@@ -1337,7 +1348,8 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
// No file system docBase required
Context ctx = tomcat.addContext("", null);
- NBWriteServlet02 writeServlet = new NBWriteServlet02(responseCommitLatch, clientCloseLatch, asyncCompleteLatch);
+ NBWriteServlet02 writeServlet =
+ new NBWriteServlet02(responseCommitLatch, clientCloseLatch, asyncCompleteLatch, swallowIoException);
Wrapper wrapper = Tomcat.addServlet(ctx, "writeServlet", writeServlet);
wrapper.setAsyncSupported(true);
ctx.addServletMappingDecoded("/*", "writeServlet");
@@ -1359,7 +1371,7 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
client.disconnect();
clientCloseLatch.countDown();
- Assert.assertTrue("Failed to complete async processing", asyncCompleteLatch.await(10, TimeUnit.SECONDS));
+ Assert.assertTrue("Failed to complete async processing", asyncCompleteLatch.await(60, TimeUnit.SECONDS));
}
@@ -1370,12 +1382,14 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
private final CountDownLatch responseCommitLatch;
private final CountDownLatch clientCloseLatch;
private final CountDownLatch asyncCompleteLatch;
+ private final boolean swallowIoException;
public NBWriteServlet02(CountDownLatch responseCommitLatch, CountDownLatch clientCloseLatch,
- CountDownLatch asyncCompleteLatch) {
+ CountDownLatch asyncCompleteLatch, boolean swallowIoException) {
this.responseCommitLatch = responseCommitLatch;
this.clientCloseLatch = clientCloseLatch;
this.asyncCompleteLatch = asyncCompleteLatch;
+ this.swallowIoException = swallowIoException;
}
@Override
@@ -1387,7 +1401,8 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
ac.addListener(new TestAsyncListener02(asyncCompleteLatch));
ac.setTimeout(5000);
- WriteListener writeListener = new TestWriteListener02(ac, responseCommitLatch, clientCloseLatch);
+ WriteListener writeListener =
+ new TestWriteListener02(ac, responseCommitLatch, clientCloseLatch, swallowIoException);
resp.getOutputStream().setWriteListener(writeListener);
}
}
@@ -1428,37 +1443,45 @@ public class TestNonBlockingAPI extends TomcatBaseTest {
private final AsyncContext ac;
private final CountDownLatch responseCommitLatch;
private final CountDownLatch clientCloseLatch;
+ private final boolean swallowIoException;
private volatile int stage = 0;
public TestWriteListener02(AsyncContext ac, CountDownLatch responseCommitLatch,
- CountDownLatch clientCloseLatch) {
+ CountDownLatch clientCloseLatch, boolean swallowIoException) {
this.ac = ac;
this.responseCommitLatch = responseCommitLatch;
this.clientCloseLatch = clientCloseLatch;
+ this.swallowIoException = swallowIoException;
}
@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
+ try {
+ 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();
}
- sos.print("TEST");
- stage++;
- } else if (stage == 2) {
- sos.flush();
+ } while (sos.isReady());
+ } catch (IOException ioe) {
+ if (!swallowIoException) {
+ throw ioe;
}
- } while (sos.isReady());
+ }
}
@Override
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1859a3d..c000da3 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -115,6 +115,12 @@
when a I/O error occurs during non-blocking I/O. There were some cases
discovered where this was not happening. (markt)
</fix>
+ <add>
+ Make the non-blocking I/O error handling more robust by handling the
+ case where the application code swallows an <code>IOException</code> in
+ <code>WriteListener.onWritePossible()</code> and
+ <code>ReadListener.onDataAvailable()</code>. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/04: Avoid attempting to write an error report if I/O is
not allowed
Posted by ma...@apache.org.
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
commit 0ff1ed325d2a35c64a12c36f2bfd1959f66413f2
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Feb 24 15:41:25 2021 +0000
Avoid attempting to write an error report if I/O is not allowed
---
.../apache/catalina/valves/ErrorReportValve.java | 28 ++++++++++++++--------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/java/org/apache/catalina/valves/ErrorReportValve.java b/java/org/apache/catalina/valves/ErrorReportValve.java
index c3fb116..74ccc1f 100644
--- a/java/org/apache/catalina/valves/ErrorReportValve.java
+++ b/java/org/apache/catalina/valves/ErrorReportValve.java
@@ -83,17 +83,25 @@ public class ErrorReportValve extends ValveBase {
if (response.isCommitted()) {
if (response.setErrorReported()) {
// Error wasn't previously reported but we can't write an error
- // page because the response has already been committed. Attempt
- // to flush any data that is still to be written to the client.
- try {
- response.flushBuffer();
- } catch (Throwable t) {
- ExceptionUtils.handleThrowable(t);
+ // page because the response has already been committed.
+
+ // See if IO is allowed
+ AtomicBoolean ioAllowed = new AtomicBoolean(true);
+ response.getCoyoteResponse().action(ActionCode.IS_IO_ALLOWED, ioAllowed);
+
+ if (ioAllowed.get()) {
+ // I/O is currently still allowed. Flush any data that is
+ // still to be written to the client.
+ try {
+ response.flushBuffer();
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ }
+ // Now close immediately to signal to the client that
+ // something went wrong
+ response.getCoyoteResponse().action(ActionCode.CLOSE_NOW,
+ request.getAttribute(RequestDispatcher.ERROR_EXCEPTION));
}
- // Close immediately to signal to the client that something went
- // wrong
- response.getCoyoteResponse().action(ActionCode.CLOSE_NOW,
- request.getAttribute(RequestDispatcher.ERROR_EXCEPTION));
}
return;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org