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

Reply via email to