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 2011/09/06 17:00:02 UTC

svn commit: r1165693 - in /tomcat/trunk/java/org/apache: coyote/http11/AbstractHttp11Processor.java coyote/http11/Http11AprProcessor.java coyote/http11/Http11NioProcessor.java coyote/http11/Http11Processor.java tomcat/util/net/AprEndpoint.java

Author: markt
Date: Tue Sep  6 15:00:02 2011
New Revision: 1165693

URL: http://svn.apache.org/viewvc?rev=1165693&view=rev
Log:
Aligning the HTTP connectors.
Handle request line timeouts consistently
Handle upload timeouts more consistently

Modified:
    tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
    tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java

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=1165693&r1=1165692&r2=1165693&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Tue Sep  6 15:00:02 2011
@@ -96,6 +96,7 @@ public abstract class AbstractHttp11Proc
      */
     protected boolean keptAlive;
 
+
     /**
      * Flag that indicates that send file processing is in progress and that the
      * socket should not be returned to the poller (where a poller is used).
@@ -816,6 +817,12 @@ public abstract class AbstractHttp11Proc
 
 
     /**
+     * Configures the timeout to be used for reading the request line.
+     */
+    protected abstract void setRequestLineReadTimeout() throws IOException;
+
+
+    /**
      * After reading the request headers, we have to setup the request filters.
      */
     protected void prepareRequest() {

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1165693&r1=1165692&r2=1165693&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Tue Sep  6 15:00:02 2011
@@ -190,8 +190,6 @@ public class Http11AprProcessor extends 
             keptAlive = socketWrapper.isKeptAlive();
         }
 
-        int soTimeout = endpoint.getSoTimeout();
-
         if (disableKeepAlive()) {
             socketWrapper.setKeepAliveLeft(0);
         }
@@ -203,9 +201,8 @@ public class Http11AprProcessor extends 
 
             // Parsing the request header
             try {
-                if( !disableUploadTimeout && keptAlive && soTimeout > 0 ) {
-                    Socket.timeoutSet(socketRef, soTimeout * 1000);
-                }
+                setRequestLineReadTimeout();
+                
                 if (!inputBuffer.parseRequestLine(keptAlive)) {
                     // This means that no data is available right now
                     // (long keepalive), so that the processor should be recycled
@@ -326,6 +323,10 @@ public class Http11AprProcessor extends 
                 outputBuffer.nextRequest();
             }
 
+            if (!disableUploadTimeout) {
+                Socket.timeoutSet(socketRef, endpoint.getSoTimeout() * 1000);
+            }
+
             rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE);
 
             if (breakKeepAliveLoop(socketWrapper)) {
@@ -358,6 +359,36 @@ public class Http11AprProcessor extends 
 
 
     @Override
+    protected boolean disableKeepAlive() {
+        return false;
+    }
+
+
+    @Override
+    protected void setRequestLineReadTimeout() throws IOException {
+        // Timeouts while in the poller are handled entirely by the poller
+        // Only need to be concerned with socket timeouts
+
+        // APR uses simulated blocking so if some request line data is present
+        // then it must all be presented (with the normal socket timeout).
+        
+        // When entering the processing loop for the first time there will
+        // always be some data to read so the keep-alive timeout is not required
+        
+        // For the second and subsequent executions of the processing loop, if
+        // there is no request line data present then no further data will be
+        // read from the socket. If there is request line data present then it
+        // must all be presented (with the normal socket timeout)
+
+        // When the socket is created it is given the correct timeout.
+        // sendfile may change the timeout but will restore it
+        // This processor may change the timeout for uploads but will restore it
+        
+        // NO-OP
+    }
+
+
+    @Override
     protected void setCometTimeouts(SocketWrapper<Long> socketWrapper) {
         // NO-OP for APR/native
         return;
@@ -393,12 +424,6 @@ public class Http11AprProcessor extends 
 
 
     @Override
-    protected boolean disableKeepAlive() {
-        return false;
-    }
-
-
-    @Override
     protected void resetTimeouts() {
         // NOOP for APR
     }

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1165693&r1=1165692&r2=1165693&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Tue Sep  6 15:00:02 2011
@@ -219,8 +219,6 @@ public class Http11NioProcessor extends 
             keptAlive = socketWrapper.isKeptAlive();
         }
         
-        int soTimeout = endpoint.getSoTimeout();
-
         if (disableKeepAlive()) {
             socketWrapper.setKeepAliveLeft(0);
         }
@@ -228,13 +226,10 @@ public class Http11NioProcessor extends 
         while (!error && keepAlive && !comet && !isAsync() &&
                 !endpoint.isPaused()) {
 
-            //always default to our soTimeout
-            socketWrapper.setTimeout(soTimeout);
             // Parsing the request header
             try {
-                if( !disableUploadTimeout && keptAlive && soTimeout > 0 ) {
-                    socketWrapper.getSocket().getIOChannel().socket().setSoTimeout(soTimeout);
-                }
+                setRequestLineReadTimeout();
+                
                 if (!inputBuffer.parseRequestLine(keptAlive)) {
                     // Haven't finished reading the request so keep the socket
                     // open
@@ -372,6 +367,11 @@ public class Http11NioProcessor extends 
                 outputBuffer.nextRequest();
             }
 
+            if (!disableUploadTimeout) { //only for body, not for request headers
+                socketWrapper.getSocket().getIOChannel().socket().setSoTimeout(
+                        endpoint.getSoTimeout());
+            }
+
             rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE);
 
             if (breakKeepAliveLoop(socketWrapper)) {
@@ -404,6 +404,33 @@ public class Http11NioProcessor extends 
 
 
     @Override
+    protected boolean disableKeepAlive() {
+        return false;
+    }
+
+
+    @Override
+    protected void setRequestLineReadTimeout() throws IOException {
+        // socket.setTimeout()
+        //     - timeout used by poller
+        // socket.getSocket().getIOChannel().socket().setSoTimeout()
+        //     - timeout used for blocking reads
+
+        // When entering the processing loop there will always be data to read
+        // so no point changing timeouts at this point
+        
+        // For the second and subsequent executions of the processing loop, a
+        // non-blocking read is used so again no need to set the timeouts
+        
+        // Because NIO supports non-blocking reading of the request line and
+        // headers the timeouts need to be set when returning the socket to
+        // the poller rather than here.
+        
+        // NO-OP
+    }
+
+
+    @Override
     protected void setCometTimeouts(SocketWrapper<NioChannel> socketWrapper) {
         // Comet support
         SelectionKey key = socketWrapper.getSocket().getIOChannel().keyFor(
@@ -440,12 +467,6 @@ public class Http11NioProcessor extends 
 
 
     @Override
-    protected boolean disableKeepAlive() {
-        return false;
-    }
-
-
-    @Override
     public void recycleInternal() {
         socket = null;
         cometClose = false;

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1165693&r1=1165692&r2=1165693&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Tue Sep  6 15:00:02 2011
@@ -165,48 +165,8 @@ public class Http11Processor extends Abs
 
             // Parsing the request header
             try {
-                int standardTimeout = 0;
-                if (keptAlive) {
-                    if (keepAliveTimeout > 0) {
-                        standardTimeout = keepAliveTimeout;
-                    } else if (soTimeout > 0) {
-                        standardTimeout = soTimeout;
-                    }
-                }
-                /*
-                 * When there is no data in the buffer and this is not the first
-                 * request on this connection and timeouts are being used the
-                 * first read for this request may need a different timeout to
-                 * take account of time spent waiting for a processing thread.
-                 * 
-                 * This is a little hacky but better than exposing the socket
-                 * and the timeout info to the InputBuffer
-                 */
-                if (inputBuffer.lastValid == 0 &&
-                        socketWrapper.getLastAccess() > -1 &&
-                        standardTimeout > 0) {
-
-                    long queueTime = System.currentTimeMillis() -
-                            socketWrapper.getLastAccess();
-                    int firstReadTimeout;
-                    if (queueTime >= standardTimeout) {
-                        // Queued for longer than timeout but there might be
-                        // data so use shortest possible timeout
-                        firstReadTimeout = 1;
-                    } else {
-                        // Cast is safe since queueTime must be less than
-                        // standardTimeout which is an int
-                        firstReadTimeout = standardTimeout - (int) queueTime;
-                    }
-                    socket.getSocket().setSoTimeout(firstReadTimeout);
-                    if (!inputBuffer.fill()) {
-                        throw new EOFException(sm.getString("iib.eof.error"));
-                    }
-                }
-                if (standardTimeout > 0) {
-                    socket.getSocket().setSoTimeout(standardTimeout);
-                }
-
+                setRequestLineReadTimeout();
+                
                 inputBuffer.parseRequestLine(false);
                 if (endpoint.isPaused()) {
                     // 503 - Service unavailable
@@ -320,6 +280,10 @@ public class Http11Processor extends Abs
                 outputBuffer.nextRequest();
             }
 
+            if (!disableUploadTimeout) {
+                socket.getSocket().setSoTimeout(endpoint.getSoTimeout());
+            }
+
             rp.setStage(org.apache.coyote.Constants.STAGE_KEEPALIVE);
 
             if (breakKeepAliveLoop(socketWrapper)) {
@@ -352,24 +316,6 @@ public class Http11Processor extends Abs
 
 
     @Override
-    protected void setCometTimeouts(SocketWrapper<Socket> socketWrapper) {
-        // NO-OP for BIO
-        return;
-    }
-
-
-    @Override
-    protected boolean breakKeepAliveLoop(SocketWrapper<Socket> socketWrapper) {
-        // If we don't have a pipe-lined request allow this thread to be
-        // used by another connection
-        if (inputBuffer.lastValid == 0) {
-            return true;
-        }
-        return false;
-    }
-
-    
-    @Override
     protected boolean disableKeepAlive() {
         int threadRatio = -1;   
         // These may return zero or negative values     
@@ -390,6 +336,70 @@ public class Http11Processor extends Abs
 
 
     @Override
+    protected void setRequestLineReadTimeout() throws IOException {
+        
+        int standardTimeout = 0;
+        
+        if (keptAlive) {
+            if (keepAliveTimeout > 0) {
+                standardTimeout = keepAliveTimeout;
+            } else if (endpoint.getSoTimeout() > 0) {
+                standardTimeout = endpoint.getSoTimeout();
+            }
+        }
+        /*
+         * When there is no data in the buffer and this is not the first
+         * request on this connection and timeouts are being used the
+         * first read for this request may need a different timeout to
+         * take account of time spent waiting for a processing thread.
+         * 
+         * This is a little hacky but better than exposing the socket
+         * and the timeout info to the InputBuffer
+         */
+        if (inputBuffer.lastValid == 0 && socket.getLastAccess() > -1 &&
+                standardTimeout > 0) {
+
+            long queueTime = System.currentTimeMillis() - socket.getLastAccess();
+            int firstReadTimeout;
+            if (queueTime >= standardTimeout) {
+                // Queued for longer than timeout but there might be
+                // data so use shortest possible timeout
+                firstReadTimeout = 1;
+            } else {
+                // Cast is safe since queueTime must be less than
+                // standardTimeout which is an int
+                firstReadTimeout = standardTimeout - (int) queueTime;
+            }
+            socket.getSocket().setSoTimeout(firstReadTimeout);
+            if (!inputBuffer.fill()) {
+                throw new EOFException(sm.getString("iib.eof.error"));
+            }
+            // Once the first byte has been read, the standard timeout should be
+            // used so restore it here.
+            socket.getSocket().setSoTimeout(endpoint.getSoTimeout());
+        }
+    }
+
+
+    @Override
+    protected void setCometTimeouts(SocketWrapper<Socket> socketWrapper) {
+        // NO-OP for BIO
+        return;
+    }
+
+
+    @Override
+    protected boolean breakKeepAliveLoop(SocketWrapper<Socket> socketWrapper) {
+        // If we don't have a pipe-lined request allow this thread to be
+        // used by another connection
+        if (inputBuffer.lastValid == 0) {
+            return true;
+        }
+        return false;
+    }
+
+    
+    @Override
     protected void resetTimeouts() {
         // NOOP for BIO
     }

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=1165693&r1=1165692&r2=1165693&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Tue Sep  6 15:00:02 2011
@@ -752,8 +752,7 @@ public class AprEndpoint extends Abstrac
                 Socket.optSet(socket, Socket.APR_SO_LINGER, socketProperties.getSoLingerTime());
             if (socketProperties.getTcpNoDelay())
                 Socket.optSet(socket, Socket.APR_TCP_NODELAY, (socketProperties.getTcpNoDelay() ? 1 : 0));
-            if (socketProperties.getSoTimeout() > 0)
-                Socket.timeoutSet(socket, socketProperties.getSoTimeout() * 1000);
+            Socket.timeoutSet(socket, socketProperties.getSoTimeout() * 1000);
 
             // 2: SSL handshake
             step = 2;



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