Author: markt Date: Wed May 29 15:55:28 2013 New Revision: 1487523 URL: http://svn.apache.org/r1487523 Log: Align onWritePossible behaviour with the spec. This passes the unit tests on Windows. Still some TODOs to resolve once I have checked this passes the tests on other platforms (including the CI system)
Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java tomcat/trunk/java/org/apache/coyote/ActionCode.java tomcat/trunk/java/org/apache/coyote/Response.java tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java 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/InternalNioOutputBuffer.java tomcat/trunk/java/org/apache/coyote/http11/InternalOutputBuffer.java Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Wed May 29 15:55:28 2013 @@ -356,7 +356,7 @@ public class CoyoteAdapter implements Ad request.getContext().getLoader().getClassLoader(); try { Thread.currentThread().setContextClassLoader(newCL); - res.getWriteListener().onWritePossible(); + res.onWritePossible(); } finally { Thread.currentThread().setContextClassLoader(oldCL); } Modified: tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java?rev=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/catalina/connector/OutputBuffer.java Wed May 29 15:55:28 2013 @@ -22,7 +22,6 @@ import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; import java.util.HashMap; -import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.WriteListener; @@ -647,13 +646,7 @@ public class OutputBuffer extends Writer public boolean isReady() { - if (coyoteResponse.getWriteListener() == null) { - throw new IllegalStateException("not in non blocking mode."); - } - // Assume write is not possible - AtomicBoolean isReady = new AtomicBoolean(false); - coyoteResponse.action(ActionCode.NB_WRITE_INTEREST, isReady); - return isReady.get(); + return coyoteResponse.isReady(); } Modified: tomcat/trunk/java/org/apache/coyote/ActionCode.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original) +++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Wed May 29 15:55:28 2013 @@ -213,6 +213,12 @@ public enum ActionCode { NB_WRITE_INTEREST, /** + * Flush the lower level buffers and re-register the socket with the poller + * if the buffers cannot be completely flushed. + */ + NB_WRITE_FLUSH, + + /** * Indicates if the request body has been fully read. */ REQUEST_BODY_FULLY_READ Modified: tomcat/trunk/java/org/apache/coyote/Response.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/Response.java?rev=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/Response.java (original) +++ tomcat/trunk/java/org/apache/coyote/Response.java Wed May 29 15:55:28 2013 @@ -548,6 +548,8 @@ public final class Response { } protected volatile WriteListener listener; + private boolean fireListener = false; + private final Object fireListenerLock = new Object(); public WriteListener getWriteListener() { return listener; @@ -573,4 +575,45 @@ public final class Response { this.listener = listener; } + + public boolean isReady() { + if (listener == null) { + // TODO i18n + throw new IllegalStateException("not in non blocking mode."); + } + // Assume write is not possible + AtomicBoolean isReady = new AtomicBoolean(false); + synchronized (fireListenerLock) { + if (fireListener) { + // isReady() has already returned false + return true; + } + action(ActionCode.NB_WRITE_INTEREST, isReady); + fireListener = !isReady.get(); + } + return isReady.get(); + } + + public void onWritePossible() throws IOException { + // Flush the lower level buffers + // If data left in buffers wait for next onWritePossible. Socket will + // have been placed in poller if buffers weren't emptied. + AtomicBoolean isDataLeftInBuffers = new AtomicBoolean(true); + action(ActionCode.NB_WRITE_FLUSH, isDataLeftInBuffers); + if (isDataLeftInBuffers.get()) { + return; + } + + // No data in lower level buffers. Ready for app to write more data. + boolean fire = false; + synchronized (fireListenerLock) { + if (fireListener) { + fireListener = false; + fire = true; + } + } + if (fire) { + listener.onWritePossible(); + } + } } Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Wed May 29 15:55:28 2013 @@ -815,7 +815,20 @@ public abstract class AbstractHttp11Proc request.setAvailable(inputBuffer.available()); } else if (actionCode == ActionCode.NB_WRITE_INTEREST) { AtomicBoolean isReady = (AtomicBoolean)param; - isReady.set(getOutputBuffer().isReady()); + try { + isReady.set(getOutputBuffer().isReady()); + } catch (IOException e) { + // TODO + throw new IllegalStateException(); + } + } else if (actionCode == ActionCode.NB_WRITE_FLUSH) { + AtomicBoolean isDataLeftInBuffers = (AtomicBoolean)param; + try { + isDataLeftInBuffers.set(getOutputBuffer().flushBuffer(false)); + } catch (IOException e) { + // TODO + throw new IllegalStateException(); + } } else if (actionCode == ActionCode.NB_READ_INTEREST) { registerForEvent(true, false); } else if (actionCode == ActionCode.UPGRADE) { 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=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java Wed May 29 15:55:28 2013 @@ -607,6 +607,7 @@ public abstract class AbstractOutputBuff //------------------------------------------------------ Non-blocking writes protected abstract boolean hasMoreDataToFlush(); + protected abstract void registerWriteInterest() throws IOException; /** @@ -628,8 +629,12 @@ public abstract class AbstractOutputBuff } - protected final boolean isReady() { - return !hasDataToWrite(); + protected final boolean isReady() throws IOException { + boolean result = !hasDataToWrite(); + if (!result) { + registerWriteInterest(); + } + return result; } 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=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalAprOutputBuffer.java Wed May 29 15:55:28 2013 @@ -316,7 +316,7 @@ public class InternalAprOutputBuffer ext bbuf.clear(); flipped = false; } else { - ((AprEndpoint) endpoint).getPoller().add(socket, -1, false, true); + registerWriteInterest(); } } @@ -339,6 +339,12 @@ public class InternalAprOutputBuffer ext } + @Override + protected void registerWriteInterest() { + ((AprEndpoint) endpoint).getPoller().add(socket, -1, false, true); + } + + // ----------------------------------- OutputStreamOutputBuffer Inner Class /** 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=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalNioOutputBuffer.java Wed May 29 15:55:28 2013 @@ -155,7 +155,7 @@ public class InternalNioOutputBuffer ext } if (flipped) { // Still have data to write - att.getPoller().add(socket, SelectionKey.OP_WRITE); + registerWriteInterest(); } return written; } @@ -274,18 +274,31 @@ public class InternalNioOutputBuffer ext return hasMoreDataToFlush(); } + @Override protected boolean hasMoreDataToFlush() { return (flipped && socket.getBufHandler().getWriteBuffer().remaining()>0) || (!flipped && socket.getBufHandler().getWriteBuffer().position() > 0); } + + @Override + protected void registerWriteInterest() throws IOException { + NioEndpoint.KeyAttachment att = (NioEndpoint.KeyAttachment)socket.getAttachment(false); + if (att == null) { + throw new IOException("Key must be cancelled"); + } + att.getPoller().add(socket, SelectionKey.OP_WRITE); + } + + private int transfer(byte[] from, int offset, int length, ByteBuffer to) { int max = Math.min(length, to.remaining()); to.put(from, offset, max); return max; } + private void transfer(ByteBuffer from, ByteBuffer to) { int max = Math.min(from.remaining(), to.remaining()); ByteBuffer tmp = from.duplicate (); Modified: tomcat/trunk/java/org/apache/coyote/http11/InternalOutputBuffer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/InternalOutputBuffer.java?rev=1487523&r1=1487522&r2=1487523&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/coyote/http11/InternalOutputBuffer.java (original) +++ tomcat/trunk/java/org/apache/coyote/http11/InternalOutputBuffer.java Wed May 29 15:55:28 2013 @@ -195,6 +195,12 @@ public class InternalOutputBuffer extend @Override + protected void registerWriteInterest() throws IOException { + // NO-OP for non-blocking connector + } + + + @Override protected boolean flushBuffer(boolean block) throws IOException { // Blocking connector so ignore block parameter as this will always use // blocking IO. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org