Author: markt Date: Thu Jan 8 13:10:34 2015 New Revision: 1650276 URL: http://svn.apache.org/r1650276 Log: Fix failing NIO2 unit tests with a largish plaster until the writes are fully moved to the NIO2 SocketWrapper.
Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java tomcat/trunk/test/org/apache/coyote/http11/filters/TesterOutputBuffer.java Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java Thu Jan 8 13:10:34 2015 @@ -20,7 +20,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.security.AccessController; import java.security.PrivilegedAction; -import java.util.concurrent.LinkedBlockingDeque; import org.apache.coyote.ActionCode; import org.apache.coyote.OutputBuffer; @@ -28,7 +27,6 @@ import org.apache.coyote.Response; import org.apache.coyote.http11.filters.GzipOutputFilter; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; -import org.apache.tomcat.util.buf.ByteBufferHolder; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.MessageBytes; import org.apache.tomcat.util.http.HttpMessages; @@ -104,23 +102,6 @@ public abstract class AbstractOutputBuff */ protected long byteCount = 0; - protected ByteBuffer socketWriteBuffer; - protected volatile boolean writeBufferFlipped; - - /** - * For "non-blocking" writes use an external set of buffers. Although the - * API only allows one non-blocking write at a time, due to buffering and - * the possible need to write HTTP headers, there may be more than one write - * to the OutputBuffer. - */ - protected final LinkedBlockingDeque<ByteBufferHolder> bufferedWrites = - new LinkedBlockingDeque<>(); - - /** - * The max size of the buffered write buffer - */ - protected int bufferedWriteSize = 64*1024; //64k default write buffer - protected AbstractOutputBuffer(Response response, int headerBufferSize) { @@ -208,16 +189,6 @@ public abstract class AbstractOutputBuff } - public void setBufferedWriteSize(int bufferedWriteSize) { - this.bufferedWriteSize = bufferedWriteSize; - } - - - public int getBufferedWriteSize() { - return bufferedWriteSize; - } - - // --------------------------------------------------- OutputBuffer Methods /** @@ -321,8 +292,6 @@ public abstract class AbstractOutputBuff // Sub-classes may wish to do more than this. nextRequest(); socketWrapper = null; - bufferedWrites.clear(); - writeBufferFlipped = false; } /** @@ -625,12 +594,6 @@ public abstract class AbstractOutputBuff protected abstract void registerWriteInterest() throws IOException; - protected boolean hasMoreDataToFlush() { - return (writeBufferFlipped && socketWriteBuffer.remaining() > 0) || - (!writeBufferFlipped && socketWriteBuffer.position() > 0); - } - - /** * Writes any remaining buffered data. * Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java Thu Jan 8 13:10:34 2015 @@ -17,10 +17,8 @@ package org.apache.coyote.http11; import java.io.IOException; -import java.nio.ByteBuffer; import org.apache.coyote.Response; -import org.apache.tomcat.util.net.SocketWrapperBase; /** * Output buffer. @@ -35,34 +33,7 @@ public class InternalAprOutputBuffer ext * Default constructor. */ public InternalAprOutputBuffer(Response response, int headerBufferSize) { - super(response, headerBufferSize); - - if (headerBufferSize < (8 * 1024)) { - socketWriteBuffer = ByteBuffer.allocateDirect(6 * 1500); - } else { - socketWriteBuffer = ByteBuffer.allocateDirect((headerBufferSize / 1500 + 1) * 1500); - } - } - - - // --------------------------------------------------------- Public Methods - - @Override - public void init(SocketWrapperBase<Long> socketWrapper) { - super.init(socketWrapper); - socketWrapper.socketWriteBuffer = socketWriteBuffer; - } - - - /** - * Recycle the output buffer. This should be called when closing the - * connection. - */ - @Override - public void recycle() { - super.recycle(); - socketWriteBuffer.clear(); } 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=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalNio2OutputBuffer.java Thu Jan 8 13:10:34 2015 @@ -90,7 +90,6 @@ public class InternalNio2OutputBuffer ex public void init(SocketWrapperBase<Nio2Channel> socketWrapper) { super.init(socketWrapper); this.endpoint = socketWrapper.getEndpoint(); - this.socketWriteBuffer = socketWrapper.getSocket().getBufHandler().getWriteBuffer(); this.completionHandler = new CompletionHandler<Integer, ByteBuffer>() { @Override @@ -99,17 +98,17 @@ public class InternalNio2OutputBuffer ex synchronized (completionHandler) { if (nBytes.intValue() < 0) { failed(new EOFException(sm.getString("iob.failedwrite")), attachment); - } else if (bufferedWrites.size() > 0) { + } else if (socketWrapper.bufferedWrites.size() > 0) { // Continue writing data using a gathering write ArrayList<ByteBuffer> arrayList = new ArrayList<>(); if (attachment.hasRemaining()) { arrayList.add(attachment); } - for (ByteBufferHolder buffer : bufferedWrites) { + for (ByteBufferHolder buffer : socketWrapper.bufferedWrites) { buffer.flip(); arrayList.add(buffer.getBuf()); } - bufferedWrites.clear(); + socketWrapper.bufferedWrites.clear(); ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY); socketWrapper.getSocket().write(array, 0, array.length, socketWrapper.getTimeout(), TimeUnit.MILLISECONDS, @@ -152,7 +151,7 @@ public class InternalNio2OutputBuffer ex synchronized (completionHandler) { if (nBytes.longValue() < 0) { failed(new EOFException(sm.getString("iob.failedwrite")), attachment); - } else if (bufferedWrites.size() > 0 || arrayHasData(attachment)) { + } else if (socketWrapper.bufferedWrites.size() > 0 || arrayHasData(attachment)) { // Continue writing data ArrayList<ByteBuffer> arrayList = new ArrayList<>(); for (ByteBuffer buffer : attachment) { @@ -160,11 +159,11 @@ public class InternalNio2OutputBuffer ex arrayList.add(buffer); } } - for (ByteBufferHolder buffer : bufferedWrites) { + for (ByteBufferHolder buffer : socketWrapper.bufferedWrites) { buffer.flip(); arrayList.add(buffer.getBuf()); } - bufferedWrites.clear(); + socketWrapper.bufferedWrites.clear(); ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY); socketWrapper.getSocket().write(array, 0, array.length, socketWrapper.getTimeout(), TimeUnit.MILLISECONDS, @@ -212,7 +211,6 @@ public class InternalNio2OutputBuffer ex writePending.drainPermits(); writePending.release(); } - bufferedWrites.clear(); } @@ -220,7 +218,6 @@ public class InternalNio2OutputBuffer ex public void nextRequest() { super.nextRequest(); interest = false; - writeBufferFlipped = false; } @@ -246,10 +243,10 @@ public class InternalNio2OutputBuffer ex if (isBlocking()) { while (length > 0) { - int thisTime = transfer(buf, offset, length, socketWriteBuffer); + int thisTime = transfer(buf, offset, length, socketWrapper.socketWriteBuffer); length = length - thisTime; offset = offset + thisTime; - if (socketWriteBuffer.remaining() == 0) { + if (socketWrapper.socketWriteBuffer.remaining() == 0) { flushBuffer(true); } } @@ -266,7 +263,7 @@ public class InternalNio2OutputBuffer ex synchronized (completionHandler) { // No pending completion handler, so writing to the main buffer // is possible - int thisTime = transfer(buf, offset, length, socketWriteBuffer); + int thisTime = transfer(buf, offset, length, socketWrapper.socketWriteBuffer); length = length - thisTime; offset = offset + thisTime; if (length > 0) { @@ -287,7 +284,7 @@ public class InternalNio2OutputBuffer ex private void addToBuffers(byte[] buf, int offset, int length) { ByteBuffer buffer = ByteBuffer.allocate(length); buffer.put(buf, offset, length); - bufferedWrites.add(new ByteBufferHolder(buffer, false)); + socketWrapper.bufferedWrites.add(new ByteBufferHolder(buffer, false)); } @@ -319,8 +316,8 @@ public class InternalNio2OutputBuffer ex } } try { - if (bufferedWrites.size() > 0) { - for (ByteBufferHolder holder : bufferedWrites) { + if (socketWrapper.bufferedWrites.size() > 0) { + for (ByteBufferHolder holder : socketWrapper.bufferedWrites) { holder.flip(); ByteBuffer buffer = holder.getBuf(); while (buffer.hasRemaining()) { @@ -329,14 +326,14 @@ public class InternalNio2OutputBuffer ex } } } - bufferedWrites.clear(); + socketWrapper.bufferedWrites.clear(); } - if (!writeBufferFlipped) { - socketWriteBuffer.flip(); - writeBufferFlipped = true; + if (!socketWrapper.writeBufferFlipped) { + socketWrapper.socketWriteBuffer.flip(); + socketWrapper.writeBufferFlipped = true; } - while (socketWriteBuffer.hasRemaining()) { - if (socketWrapper.getSocket().write(socketWriteBuffer).get(socketWrapper.getTimeout(), TimeUnit.MILLISECONDS).intValue() < 0) { + while (socketWrapper.socketWriteBuffer.hasRemaining()) { + if (socketWrapper.getSocket().write(socketWrapper.socketWriteBuffer).get(socketWrapper.getTimeout(), TimeUnit.MILLISECONDS).intValue() < 0) { throw new EOFException(sm.getString("iob.failedwrite")); } } @@ -351,48 +348,48 @@ public class InternalNio2OutputBuffer ex } catch (TimeoutException e) { throw new SocketTimeoutException(); } - socketWriteBuffer.clear(); - writeBufferFlipped = false; + socketWrapper.socketWriteBuffer.clear(); + socketWrapper.writeBufferFlipped = false; return false; } else { synchronized (completionHandler) { if (hasPermit || writePending.tryAcquire()) { - if (!writeBufferFlipped) { - socketWriteBuffer.flip(); - writeBufferFlipped = true; + if (!socketWrapper.writeBufferFlipped) { + socketWrapper.socketWriteBuffer.flip(); + socketWrapper.writeBufferFlipped = true; } Nio2Endpoint.startInline(); - if (bufferedWrites.size() > 0) { + if (socketWrapper.bufferedWrites.size() > 0) { // Gathering write of the main buffer plus all leftovers ArrayList<ByteBuffer> arrayList = new ArrayList<>(); - if (socketWriteBuffer.hasRemaining()) { - arrayList.add(socketWriteBuffer); + if (socketWrapper.socketWriteBuffer.hasRemaining()) { + arrayList.add(socketWrapper.socketWriteBuffer); } - for (ByteBufferHolder buffer : bufferedWrites) { + for (ByteBufferHolder buffer : socketWrapper.bufferedWrites) { buffer.flip(); arrayList.add(buffer.getBuf()); } - bufferedWrites.clear(); + socketWrapper.bufferedWrites.clear(); ByteBuffer[] array = arrayList.toArray(EMPTY_BUF_ARRAY); socketWrapper.getSocket().write(array, 0, array.length, socketWrapper.getTimeout(), TimeUnit.MILLISECONDS, array, gatherCompletionHandler); - } else if (socketWriteBuffer.hasRemaining()) { + } else if (socketWrapper.socketWriteBuffer.hasRemaining()) { // Regular write - socketWrapper.getSocket().write(socketWriteBuffer, socketWrapper.getTimeout(), - TimeUnit.MILLISECONDS, socketWriteBuffer, completionHandler); + socketWrapper.getSocket().write(socketWrapper.socketWriteBuffer, socketWrapper.getTimeout(), + TimeUnit.MILLISECONDS, socketWrapper.socketWriteBuffer, completionHandler); } else { // Nothing was written writePending.release(); } Nio2Endpoint.endInline(); if (writePending.availablePermits() > 0) { - if (socketWriteBuffer.remaining() == 0) { - socketWriteBuffer.clear(); - writeBufferFlipped = false; + if (socketWrapper.socketWriteBuffer.remaining() == 0) { + socketWrapper.socketWriteBuffer.clear(); + socketWrapper.writeBufferFlipped = false; } } } - return hasMoreDataToFlush() || hasBufferedData() || e != null; + return socketWrapper.hasMoreDataToFlush() || hasBufferedData() || e != null; } } } @@ -401,12 +398,12 @@ public class InternalNio2OutputBuffer ex @Override public boolean hasDataToWrite() { synchronized (completionHandler) { - return hasMoreDataToFlush() || hasBufferedData() || e != null; + return socketWrapper.hasMoreDataToFlush() || hasBufferedData() || e != null; } } protected boolean hasBufferedData() { - return bufferedWrites.size() > 0; + return socketWrapper.bufferedWrites.size() > 0; } @Override Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java Thu Jan 8 13:10:34 2015 @@ -20,7 +20,6 @@ import java.io.IOException; import org.apache.coyote.Response; import org.apache.tomcat.util.net.NioChannel; -import org.apache.tomcat.util.net.SocketWrapperBase; /** * Output buffer. @@ -39,26 +38,6 @@ public class InternalNioOutputBuffer ext } - // --------------------------------------------------------- Public Methods - - @Override - public void init(SocketWrapperBase<NioChannel> socketWrapper) { - super.init(socketWrapper); - socketWriteBuffer = socketWrapper.getSocket().getBufHandler().getWriteBuffer(); - } - - - /** - * Recycle the output buffer. This should be called when closing the - * connection. - */ - @Override - public void recycle() { - super.recycle(); - socketWriteBuffer.clear(); - } - - // ------------------------------------------------------ Protected Methods @Override Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Thu Jan 8 13:10:34 2015 @@ -2383,6 +2383,9 @@ public class AprEndpoint extends Abstrac } else { sslOutputBuffer = null; } + + // TODO: This needs to be expandable to the header buffer size + socketWriteBuffer = ByteBuffer.allocateDirect(6 * 1500); } Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Jan 8 13:10:34 2015 @@ -797,6 +797,7 @@ public class Nio2Endpoint extends Abstra super.reset(channel, soTimeout); upgradeInit = false; sendfileData = null; + socketWriteBuffer = channel.getBufHandler().getWriteBuffer(); } @Override @@ -1040,6 +1041,7 @@ public class Nio2Endpoint extends Abstra writeBuffer.clear(); writeBuffer.put(b, off, len); writeBuffer.flip(); + writeBufferFlipped = true; try { written = getSocket().write(writeBuffer).get(getTimeout(), TimeUnit.MILLISECONDS).intValue(); } catch (ExecutionException e) { @@ -1059,6 +1061,7 @@ public class Nio2Endpoint extends Abstra writeBuffer.clear(); writeBuffer.put(b, off, len); writeBuffer.flip(); + writeBufferFlipped = true; Nio2Endpoint.startInline(); getSocket().write(writeBuffer, getTimeout(), TimeUnit.MILLISECONDS, writeBuffer, writeCompletionHandler); Nio2Endpoint.endInline(); Modified: tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Thu Jan 8 13:10:34 2015 @@ -70,9 +70,9 @@ public abstract class SocketWrapperBase< */ private final Object writeThreadLock = new Object(); - // TODO This being public is a temporary hack to simplify refactoring + // TODO These being public is a temporary hack to simplify refactoring public volatile ByteBuffer socketWriteBuffer; - protected volatile boolean writeBufferFlipped; + public volatile boolean writeBufferFlipped; /** * For "non-blocking" writes use an external set of buffers. Although the @@ -80,7 +80,8 @@ public abstract class SocketWrapperBase< * the possible need to write HTTP headers, there may be more than one write * to the OutputBuffer. */ - protected final LinkedBlockingDeque<ByteBufferHolder> bufferedWrites = + // TODO This being public is a temporary hack to simplify refactoring + public final LinkedBlockingDeque<ByteBufferHolder> bufferedWrites = new LinkedBlockingDeque<>(); /** @@ -179,7 +180,8 @@ public abstract class SocketWrapperBase< } public Object getWriteThreadLock() { return writeThreadLock; } - protected boolean hasMoreDataToFlush() { + // TODO This being public is a temporary hack to simplify refactoring + public boolean hasMoreDataToFlush() { return (writeBufferFlipped && socketWriteBuffer.remaining() > 0) || (!writeBufferFlipped && socketWriteBuffer.position() > 0); } Modified: tomcat/trunk/test/org/apache/coyote/http11/filters/TesterOutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/coyote/http11/filters/TesterOutputBuffer.java?rev=1650276&r1=1650275&r2=1650276&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/coyote/http11/filters/TesterOutputBuffer.java (original) +++ tomcat/trunk/test/org/apache/coyote/http11/filters/TesterOutputBuffer.java Thu Jan 8 13:10:34 2015 @@ -80,13 +80,6 @@ public class TesterOutputBuffer extends @Override - protected boolean hasMoreDataToFlush() { - // Unused - return false; - } - - - @Override protected void registerWriteInterest() { // NO-OP: Unused } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org