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 2013/08/19 16:54:16 UTC
svn commit: r1515454 - in /tomcat/trunk:
java/org/apache/catalina/connector/InputBuffer.java
java/org/apache/catalina/connector/Request.java
java/org/apache/coyote/Request.java
test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
Author: markt
Date: Mon Aug 19 14:54:15 2013
New Revision: 1515454
URL: http://svn.apache.org/r1515454
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55438
Don't call onAllDataRead() twice for an empty request body.
Make the non-blocking test more robust as on client disconnect the error may occur in the read or the write listener.
Modified:
tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
tomcat/trunk/java/org/apache/catalina/connector/Request.java
tomcat/trunk/java/org/apache/coyote/Request.java
tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
Modified: tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java?rev=1515454&r1=1515453&r2=1515454&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java Mon Aug 19 14:54:15 2013
@@ -258,14 +258,14 @@ public class InputBuffer extends Reader
// Must call isFinished() first as a call to isReady() if the request
// has been finished will register the socket for read interest and that
// is not required.
- if (isFinished() || isReady()) {
+ if (!coyoteRequest.isFinished() && isReady()) {
coyoteRequest.action(ActionCode.DISPATCH_READ, null);
}
}
public boolean isFinished() {
- return available() == 0;
+ return coyoteRequest.isFinished();
}
Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1515454&r1=1515453&r2=1515454&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Mon Aug 19 14:54:15 2013
@@ -2461,9 +2461,7 @@ public class Request
* of the request body has been read
*/
public boolean isFinished() {
- AtomicBoolean result = new AtomicBoolean(false);
- coyoteRequest.action(ActionCode.REQUEST_BODY_FULLY_READ, result);
- return result.get();
+ return coyoteRequest.isFinished();
}
Modified: tomcat/trunk/java/org/apache/coyote/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Request.java?rev=1515454&r1=1515453&r2=1515454&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/Request.java (original)
+++ tomcat/trunk/java/org/apache/coyote/Request.java Mon Aug 19 14:54:15 2013
@@ -425,9 +425,15 @@ public final class Request {
this.available = available;
}
- // -------------------- Input Buffer --------------------
+ public boolean isFinished() {
+ AtomicBoolean result = new AtomicBoolean(false);
+ action(ActionCode.REQUEST_BODY_FULLY_READ, result);
+ return result.get();
+ }
+ // -------------------- Input Buffer --------------------
+
public InputBuffer getInputBuffer() {
return inputBuffer;
}
Modified: tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java?rev=1515454&r1=1515453&r2=1515454&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java (original)
+++ tomcat/trunk/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java Mon Aug 19 14:54:15 2013
@@ -316,7 +316,7 @@ public class TestNonBlockingAPI extends
} catch (Exception e) {
}
Assert.assertTrue("Error listener should have been invoked.",
- servlet.wlistener.onErrorInvoked);
+ servlet.wlistener.onErrorInvoked || servlet.rlistener.onErrorInvoked);
// TODO Figure out why non-blocking writes with the NIO connector appear
// to be slower on Linux
@@ -325,6 +325,43 @@ public class TestNonBlockingAPI extends
}
+ @Test
+ public void testBug55438NonBlockingReadWriteEmptyRead() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+
+ // Must have a real docBase - just use temp
+ StandardContext ctx = (StandardContext) tomcat.addContext("",
+ System.getProperty("java.io.tmpdir"));
+
+ NBReadWriteServlet servlet = new NBReadWriteServlet();
+ String servletName = NBReadWriteServlet.class.getName();
+ Tomcat.addServlet(ctx, servletName, servlet);
+ ctx.addServletMapping("/", servletName);
+
+ tomcat.start();
+
+ Map<String, List<String>> resHeaders = new HashMap<>();
+ int rc = postUrl(false, new BytesStreamer() {
+ @Override
+ public byte[] next() {
+ return new byte[] {};
+ }
+
+ @Override
+ public int getLength() {
+ return 0;
+ }
+
+ @Override
+ public int available() {
+ return 0;
+ }
+ }, "http://localhost:" +
+ getPort() + "/", new ByteChunk(), resHeaders, null);
+ Assert.assertEquals(HttpServletResponse.SC_OK, rc);
+ }
+
+
public static class DataWriter implements BytesStreamer {
final int max = 5;
int count = 0;
@@ -462,10 +499,31 @@ public class TestNonBlockingAPI extends
}
+
+ @WebServlet(asyncSupported = true)
+ public class NBReadWriteServlet extends TesterServlet {
+ private static final long serialVersionUID = 1L;
+ public volatile TestReadWriteListener rwlistener;
+
+ @Override
+ protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+ // step 1 - start async
+ AsyncContext actx = req.startAsync();
+ actx.setTimeout(Long.MAX_VALUE);
+
+ // step 2 - notify on read
+ ServletInputStream in = req.getInputStream();
+ rwlistener = new TestReadWriteListener(actx);
+ in.setReadListener(rwlistener);
+ }
+ }
+
private class TestReadListener implements ReadListener {
private final AsyncContext ctx;
private final StringBuilder body = new StringBuilder();
private final boolean usingNonBlockingWrite;
+ public volatile boolean onErrorInvoked = false;
+
public TestReadListener(AsyncContext ctx,
boolean usingNonBlockingWrite) {
@@ -515,6 +573,7 @@ public class TestNonBlockingAPI extends
public void onError(Throwable throwable) {
log.info("ReadListener.onError");
throwable.printStackTrace();
+ onErrorInvoked = true;
}
}
@@ -563,6 +622,62 @@ public class TestNonBlockingAPI extends
}
+ private class TestReadWriteListener implements ReadListener {
+ AsyncContext ctx;
+ private final StringBuilder body = new StringBuilder();
+
+ public TestReadWriteListener(AsyncContext ctx) {
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void onDataAvailable() throws IOException {
+ ServletInputStream in = ctx.getRequest().getInputStream();
+ String s = "";
+ byte[] b = new byte[8192];
+ int read = 0;
+ do {
+ read = in.read(b);
+ if (read == -1) {
+ break;
+ }
+ s += new String(b, 0, read);
+ } while (in.isReady());
+ log.info("Read [" + s + "]");
+ body.append(s);
+ }
+
+ @Override
+ public void onAllDataRead() throws IOException {
+ log.info("onAllDataRead");
+ ServletOutputStream output = ctx.getResponse().getOutputStream();
+ output.setWriteListener(new WriteListener() {
+ @Override
+ public void onWritePossible() throws IOException {
+ ServletOutputStream output = ctx.getResponse().getOutputStream();
+ if (output.isReady()) {
+ log.info("Writing [" + body.toString() + "]");
+ output.write(body.toString().getBytes("utf-8"));
+ }
+ ctx.complete();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ log.info("ReadWriteListener.onError");
+ throwable.printStackTrace();
+ }
+ });
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ log.info("ReadListener.onError");
+ throwable.printStackTrace();
+ }
+
+ }
+
public static int postUrlWithDisconnect(boolean stream, BytesStreamer streamer, String path,
Map<String, List<String>> reqHead, Map<String, List<String>> resHead) throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org