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/12 09:59:54 UTC

svn commit: r1651045 - in /tomcat/trunk/java/org/apache/tomcat/util/net: AprEndpoint.java Nio2Endpoint.java NioEndpoint.java SocketWrapperBase.java

Author: markt
Date: Mon Jan 12 08:59:53 2015
New Revision: 1651045

URL: http://svn.apache.org/r1651045
Log:
Complete a TODO
- Ensure a blocking call to doWrite() always empties the buffer or
  times out
- Remove flip parameter from doWrite() as it is no longer required

Modified:
    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/NioEndpoint.java
    tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java

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=1651045&r1=1651044&r2=1651045&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Mon Jan 12 08:59:53 2015
@@ -2509,7 +2509,7 @@ public class AprEndpoint extends Abstrac
 
 
         @Override
-        protected int doWrite(boolean block, boolean flip) throws IOException {
+        protected int doWrite(boolean block) throws IOException {
             if (closed) {
                 throw new IOException(sm.getString("apr.closed", getSocket()));
             }
@@ -2520,7 +2520,7 @@ public class AprEndpoint extends Abstrac
             readLock.lock();
             try {
                 if (getBlockingStatus() == block) {
-                    return doWriteInternal(flip);
+                    return doWriteInternal();
                 }
             } finally {
                 readLock.unlock();
@@ -2540,7 +2540,7 @@ public class AprEndpoint extends Abstrac
                 readLock.lock();
                 try {
                     writeLock.unlock();
-                    return doWriteInternal(flip);
+                    return doWriteInternal();
                 } finally {
                     readLock.unlock();
                 }
@@ -2554,8 +2554,8 @@ public class AprEndpoint extends Abstrac
         }
 
 
-        private int doWriteInternal(boolean flip) throws IOException {
-            if (flip) {
+        private int doWriteInternal() throws IOException {
+            if (!writeBufferFlipped) {
                 socketWriteBuffer.flip();
                 writeBufferFlipped = true;
             }
@@ -2601,7 +2601,7 @@ public class AprEndpoint extends Abstrac
                 }
                 written += thisTime;
                 socketWriteBuffer.position(socketWriteBuffer.position() + thisTime);
-            } while (thisTime > 0 && socketWriteBuffer.hasRemaining());
+            } while ((thisTime > 0 || getBlockingStatus()) && socketWriteBuffer.hasRemaining());
 
             if (socketWriteBuffer.remaining() == 0) {
                 socketWriteBuffer.clear();

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=1651045&r1=1651044&r2=1651045&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Mon Jan 12 08:59:53 2015
@@ -1148,12 +1148,12 @@ public class Nio2Endpoint extends Abstra
         }
 
 
+        /**
+         * @param block Ignored since this method is only called in the
+         *              blocking case
+         */
         @Override
-        protected int doWrite(boolean block, boolean flip) throws IOException {
-            // Only called in the non-blocking case since
-            // writeNonBlocking(byte[], int, int) and flush(boolean, boolean)
-            // are over-ridden.
-
+        protected int doWrite(boolean block) throws IOException {
             int result = -1;
             try {
                 socketWriteBuffer.flip();

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1651045&r1=1651044&r2=1651045&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Mon Jan 12 08:59:53 2015
@@ -1512,9 +1512,9 @@ public class NioEndpoint extends Abstrac
 
 
         @Override
-        protected synchronized int doWrite(boolean block, boolean flip)
+        protected synchronized int doWrite(boolean block)
                 throws IOException {
-            if (flip) {
+            if (!writeBufferFlipped) {
                 socketWriteBuffer.flip();
                 writeBufferFlipped = true;
             }

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=1651045&r1=1651044&r2=1651045&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/SocketWrapperBase.java Mon Jan 12 08:59:53 2015
@@ -348,13 +348,7 @@ public abstract class SocketWrapperBase<
         while (socketWriteBuffer.remaining() == 0) {
             len = len - thisTime;
             off = off + thisTime;
-            // TODO: There is an assumption here that the blocking write will
-            //       block until all the data is written or the write times out.
-            //       Document this assumption in the Javadoc for doWrite(),
-            //       ensure it is valid for all implementations of doWrite() and
-            //       then review all callers of doWrite() and review what
-            //       simplifications this offers.
-            doWrite(true, true);
+            doWrite(true);
             thisTime = transfer(buf, off, len, socketWriteBuffer);
         }
     }
@@ -378,7 +372,7 @@ public abstract class SocketWrapperBase<
             len = len - thisTime;
             while (socketWriteBuffer.remaining() == 0) {
                 off = off + thisTime;
-                if (doWrite(false, !writeBufferFlipped) == 0) {
+                if (doWrite(false) == 0) {
                     break;
                 }
                 if (writeBufferFlipped) {
@@ -434,7 +428,7 @@ public abstract class SocketWrapperBase<
 
 
     protected void flushBlocking() throws IOException {
-        doWrite(true, !writeBufferFlipped);
+        doWrite(true);
 
         if (bufferedWrites.size() > 0) {
             Iterator<ByteBufferHolder> bufIter = bufferedWrites.iterator();
@@ -446,7 +440,7 @@ public abstract class SocketWrapperBase<
                     if (buffer.getBuf().remaining() == 0) {
                         bufIter.remove();
                     }
-                    doWrite(true, !writeBufferFlipped);
+                    doWrite(true);
                 }
             }
         }
@@ -459,7 +453,7 @@ public abstract class SocketWrapperBase<
 
         // Write to the socket, if there is anything to write
         if (dataLeft) {
-            doWrite(false, !writeBufferFlipped);
+            doWrite(false);
         }
 
         dataLeft = hasMoreDataToFlush();
@@ -474,7 +468,7 @@ public abstract class SocketWrapperBase<
                     if (buffer.getBuf().remaining() == 0) {
                         bufIter.remove();
                     }
-                    doWrite(false, !writeBufferFlipped);
+                    doWrite(false);
                 }
             }
         }
@@ -483,7 +477,19 @@ public abstract class SocketWrapperBase<
     }
 
 
-    protected abstract int doWrite(boolean block, boolean flip) throws IOException;
+    /**
+     * Write the contents of the socketWriteBuffer to the socket. For blocking
+     * writes either then entire contents of the buffer will be written or an
+     * IOException will be thrown. Partial blocking writes will not occur.
+     *
+     * @param block Should the write be blocking or not?
+     *
+     * @return The number of bytes written
+     *
+     * @throws IOException If an I/O error such as a timeout occurs during the
+     *                     write
+     */
+    protected abstract int doWrite(boolean block) throws IOException;
 
 
     protected void addToBuffers(byte[] buf, int offset, int length) {



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org