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 2015/01/08 14:10:34 UTC
svn commit: r1650276 - in /tomcat/trunk: java/org/apache/coyote/http11/
java/org/apache/tomcat/util/net/ test/org/apache/coyote/http11/filters/
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org