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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org