Author: remm Date: Wed Apr 2 09:59:14 2014 New Revision: 1583950 URL: http://svn.apache.org/r1583950 Log: - Handle incomplete writes as it is very easy to do, just in case (it never happens for me though). - Try again that SSL test.
Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java tomcat/trunk/test/org/apache/tomcat/websocket/TestWebSocketFrameClientSSL.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java?rev=1583950&r1=1583949&r2=1583950&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java Wed Apr 2 09:59:14 2014 @@ -76,7 +76,7 @@ public class InternalNio2OutputBuffer ex /** * The completion handler used for asynchronous write operations */ - protected CompletionHandler<Integer, SocketWrapper<Nio2Channel>> completionHandler; + protected CompletionHandler<Integer, ByteBuffer> completionHandler; /** * The completion handler used for asynchronous write operations @@ -111,18 +111,20 @@ public class InternalNio2OutputBuffer ex this.socket = socketWrapper; this.endpoint = associatedEndpoint; - this.completionHandler = new CompletionHandler<Integer, SocketWrapper<Nio2Channel>>() { + this.completionHandler = new CompletionHandler<Integer, ByteBuffer>() { @Override - public void completed(Integer nBytes, SocketWrapper<Nio2Channel> attachment) { + public void completed(Integer nBytes, ByteBuffer attachment) { boolean notify = false; synchronized (completionHandler) { if (nBytes.intValue() < 0) { failed(new EOFException(sm.getString("iob.failedwrite")), attachment); return; - } - if (bufferedWrites.size() > 0) { - // Continue writing data + } else if (bufferedWrites.size() > 0) { + // Continue writing data using a gathering write ArrayList<ByteBuffer> arrayList = new ArrayList<>(); + if (attachment.hasRemaining()) { + arrayList.add(attachment); + } for (ByteBuffer buffer : bufferedWrites) { buffer.flip(); arrayList.add(buffer); @@ -130,9 +132,14 @@ public class InternalNio2OutputBuffer ex bufferedWrites.clear(); ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY); socket.getSocket().write(array, 0, array.length, - attachment.getTimeout(), TimeUnit.MILLISECONDS, + socket.getTimeout(), TimeUnit.MILLISECONDS, array, gatherCompletionHandler); + } else if (attachment.hasRemaining()) { + // Regular write + socket.getSocket().write(attachment, socket.getTimeout(), + TimeUnit.MILLISECONDS, attachment, completionHandler); } else { + // All data has been written if (interest && !Nio2Endpoint.isInline()) { interest = false; notify = true; @@ -141,13 +148,13 @@ public class InternalNio2OutputBuffer ex } } if (notify) { - endpoint.processSocket(attachment, SocketStatus.OPEN_WRITE, true); + endpoint.processSocket(socket, SocketStatus.OPEN_WRITE, true); } } @Override - public void failed(Throwable exc, SocketWrapper<Nio2Channel> attachment) { - attachment.setError(true); + public void failed(Throwable exc, ByteBuffer attachment) { + socket.setError(true); if (exc instanceof IOException) { e = (IOException) exc; } else { @@ -155,7 +162,7 @@ public class InternalNio2OutputBuffer ex } response.getRequest().setAttribute(RequestDispatcher.ERROR_EXCEPTION, e); writePending.release(); - endpoint.processSocket(attachment, SocketStatus.OPEN_WRITE, true); + endpoint.processSocket(socket, SocketStatus.OPEN_WRITE, true); } }; this.gatherCompletionHandler = new CompletionHandler<Long, ByteBuffer[]>() { @@ -166,8 +173,7 @@ public class InternalNio2OutputBuffer ex if (nBytes.longValue() < 0) { failed(new EOFException(sm.getString("iob.failedwrite")), attachment); return; - } - if (bufferedWrites.size() > 0 || arrayHasData(attachment)) { + } else if (bufferedWrites.size() > 0 || arrayHasData(attachment)) { // Continue writing data ArrayList<ByteBuffer> arrayList = new ArrayList<>(); for (ByteBuffer buffer : attachment) { @@ -185,6 +191,7 @@ public class InternalNio2OutputBuffer ex socket.getTimeout(), TimeUnit.MILLISECONDS, array, gatherCompletionHandler); } else { + // All data has been written if (interest && !Nio2Endpoint.isInline()) { interest = false; notify = true; @@ -384,13 +391,15 @@ public class InternalNio2OutputBuffer ex if (bufferedWrites.size() > 0) { for (ByteBuffer buffer : bufferedWrites) { buffer.flip(); - if (socket.getSocket().write(buffer).get(socket.getTimeout(), TimeUnit.MILLISECONDS).intValue() < 0) { - throw new EOFException(sm.getString("iob.failedwrite")); + while (buffer.hasRemaining()) { + if (socket.getSocket().write(buffer).get(socket.getTimeout(), TimeUnit.MILLISECONDS).intValue() < 0) { + throw new EOFException(sm.getString("iob.failedwrite")); + } } } bufferedWrites.clear(); } - if (byteBuffer.hasRemaining()) { + while (byteBuffer.hasRemaining()) { if (socket.getSocket().write(byteBuffer).get(socket.getTimeout(), TimeUnit.MILLISECONDS).intValue() < 0) { throw new EOFException(sm.getString("iob.failedwrite")); } @@ -437,7 +446,7 @@ public class InternalNio2OutputBuffer ex } else if (byteBuffer.hasRemaining()) { // Regular write socket.getSocket().write(byteBuffer, socket.getTimeout(), - TimeUnit.MILLISECONDS, socket, completionHandler); + TimeUnit.MILLISECONDS, byteBuffer, completionHandler); } else { // Nothing was written writePending.release(); Modified: tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java?rev=1583950&r1=1583949&r2=1583950&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/upgrade/Nio2ServletOutputStream.java Wed Apr 2 09:59:14 2014 @@ -38,42 +38,46 @@ public class Nio2ServletOutputStream ext private final AbstractEndpoint<Nio2Channel> endpoint; private final Nio2Channel channel; private final int maxWrite; - private final CompletionHandler<Integer, SocketWrapper<Nio2Channel>> completionHandler; + private final CompletionHandler<Integer, ByteBuffer> completionHandler; private final Semaphore writePending = new Semaphore(1); - public Nio2ServletOutputStream(AbstractEndpoint<Nio2Channel> endpoint0, SocketWrapper<Nio2Channel> socketWrapper) { - super(socketWrapper); + public Nio2ServletOutputStream(AbstractEndpoint<Nio2Channel> endpoint0, SocketWrapper<Nio2Channel> socketWrapper0) { + super(socketWrapper0); this.endpoint = endpoint0; - channel = socketWrapper.getSocket(); + channel = socketWrapper0.getSocket(); maxWrite = channel.getBufHandler().getWriteBuffer().capacity(); - this.completionHandler = new CompletionHandler<Integer, SocketWrapper<Nio2Channel>>() { + this.completionHandler = new CompletionHandler<Integer, ByteBuffer>() { @Override - public void completed(Integer nBytes, SocketWrapper<Nio2Channel> attachment) { + public void completed(Integer nBytes, ByteBuffer attachment) { if (nBytes.intValue() < 0) { failed(new EOFException(), attachment); return; - } - writePending.release(); - if (!Nio2Endpoint.isInline()) { - try { - onWritePossible(); - } catch (IOException e) { - attachment.setError(true); - onError(e); - endpoint.processSocket(attachment, SocketStatus.ERROR, true); + } else if (attachment.hasRemaining()) { + channel.write(attachment, socketWrapper.getTimeout(), + TimeUnit.MILLISECONDS, attachment, completionHandler); + } else { + writePending.release(); + if (!Nio2Endpoint.isInline()) { + try { + onWritePossible(); + } catch (IOException e) { + socketWrapper.setError(true); + onError(e); + endpoint.processSocket(socketWrapper, SocketStatus.ERROR, true); + } } } } @Override - public void failed(Throwable exc, SocketWrapper<Nio2Channel> attachment) { - attachment.setError(true); + public void failed(Throwable exc, ByteBuffer attachment) { + socketWrapper.setError(true); writePending.release(); if (exc instanceof AsynchronousCloseException) { // If already closed, don't call onError and close again return; } onError(exc); - endpoint.processSocket(attachment, SocketStatus.ERROR, true); + endpoint.processSocket(socketWrapper, SocketStatus.ERROR, true); } }; } @@ -140,7 +144,7 @@ public class Nio2ServletOutputStream ext buffer.put(b, off, len); buffer.flip(); Nio2Endpoint.startInline(); - channel.write(buffer, socketWrapper.getTimeout(), TimeUnit.MILLISECONDS, socketWrapper, completionHandler); + channel.write(buffer, socketWrapper.getTimeout(), TimeUnit.MILLISECONDS, buffer, completionHandler); Nio2Endpoint.endInline(); written = len; } Modified: tomcat/trunk/test/org/apache/tomcat/websocket/TestWebSocketFrameClientSSL.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/TestWebSocketFrameClientSSL.java?rev=1583950&r1=1583949&r2=1583950&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/websocket/TestWebSocketFrameClientSSL.java (original) +++ tomcat/trunk/test/org/apache/tomcat/websocket/TestWebSocketFrameClientSSL.java Wed Apr 2 09:59:14 2014 @@ -46,14 +46,6 @@ public class TestWebSocketFrameClientSSL @Test public void testConnectToServerEndpoint() throws Exception { - // FIXME Skip NIO2 since its CPU use on non blocking writes to - // do the encryption inline apparently messes up - // the websockets writes, which deadlock until timedout. - // Reenable later when investigated and fixed. - Assume.assumeFalse( - "Skip this test on NIO2. FIXME: investigate.", - getTomcatInstance().getConnector().getProtocol() - .equals("org.apache.coyote.http11.Http11Nio2Protocol")); Tomcat tomcat = getTomcatInstance(); // Must have a real docBase - just use temp Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1583950&r1=1583949&r2=1583950&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Wed Apr 2 09:59:14 2014 @@ -80,6 +80,9 @@ <fix> <bug>56336</bug>: AJP output corruption and errors. (remm) </fix> + <fix> + Handle incomplete writes in NIO2, just in case. (remm) + </fix> </changelog> </subsection> <subsection name="Cluster"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org