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 2017/03/27 19:52:31 UTC

svn commit: r1788999 - in /tomcat/tc8.0.x/trunk: java/org/apache/coyote/ java/org/apache/coyote/http11/ java/org/apache/tomcat/util/net/ webapps/docs/

Author: markt
Date: Mon Mar 27 19:52:30 2017
New Revision: 1788999

URL: http://svn.apache.org/viewvc?rev=1788999&view=rev
Log:
Improve sendfile handling when requests are pipelined.

Added:
    tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/SendfileKeepAliveState.java   (with props)
Modified:
    tomcat/tc8.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java
    tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
    tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
    tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
    tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
    tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
    tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
    tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml

Modified: tomcat/tc8.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/coyote/AbstractProtocol.java Mon Mar 27 19:52:30 2017
@@ -747,10 +747,9 @@ public abstract class AbstractProtocol<S
                     release(wrapper, processor, false, true);
                 } else if (state == SocketState.SENDFILE) {
                     // Sendfile in progress. If it fails, the socket will be
-                    // closed. If it works, the socket will be re-added to the
-                    // poller
-                    connections.remove(socket);
-                    release(wrapper, processor, false, false);
+                    // closed. If it works, the socket either be added to the
+                    // poller (or equivalent) to await more data or processed
+                    // if there are any pipe-lined requests remaining.
                 } else if (state == SocketState.UPGRADED) {
                     // Don't add sockets back to the poller if this was a
                     // non-blocking write otherwise the poller may trigger

Modified: tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Mon Mar 27 19:52:30 2017
@@ -38,6 +38,7 @@ import org.apache.tomcat.util.ExceptionU
 import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
 import org.apache.tomcat.util.net.AprEndpoint;
 import org.apache.tomcat.util.net.SSLSupport;
+import org.apache.tomcat.util.net.SendfileKeepAliveState;
 import org.apache.tomcat.util.net.SocketStatus;
 import org.apache.tomcat.util.net.SocketWrapper;
 
@@ -198,7 +199,15 @@ public class Http11AprProcessor extends
         // Do sendfile as needed: add socket to sendfile and end
         if (sendfileData != null && !getErrorState().isError()) {
             sendfileData.socket = socketWrapper.getSocket().longValue();
-            sendfileData.keepAlive = keepAlive;
+            if (keepAlive) {
+                if (getInputBuffer().available(false) == 0) {
+                    sendfileData.keepAliveState = SendfileKeepAliveState.OPEN;
+                } else {
+                    sendfileData.keepAliveState = SendfileKeepAliveState.PIPELINED;
+                }
+            } else {
+                sendfileData.keepAliveState = SendfileKeepAliveState.NONE;
+            }
             switch (((AprEndpoint)endpoint).getSendfile().add(sendfileData)) {
             case DONE:
                 return false;

Modified: tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11Nio2Processor.java Mon Mar 27 19:52:30 2017
@@ -37,6 +37,7 @@ import org.apache.tomcat.util.net.Nio2Ch
 import org.apache.tomcat.util.net.Nio2Endpoint;
 import org.apache.tomcat.util.net.SSLSupport;
 import org.apache.tomcat.util.net.SecureNio2Channel;
+import org.apache.tomcat.util.net.SendfileKeepAliveState;
 import org.apache.tomcat.util.net.SocketStatus;
 import org.apache.tomcat.util.net.SocketWrapper;
 
@@ -280,7 +281,15 @@ public class Http11Nio2Processor extends
         // Do sendfile as needed: add socket to sendfile and end
         if (sendfileData != null && !getErrorState().isError()) {
             ((Nio2Endpoint.Nio2SocketWrapper) socketWrapper).setSendfileData(sendfileData);
-            sendfileData.keepAlive = keepAlive;
+            if (keepAlive) {
+                if (getInputBuffer().available(false) == 0) {
+                    sendfileData.keepAliveState = SendfileKeepAliveState.OPEN;
+                } else {
+                    sendfileData.keepAliveState = SendfileKeepAliveState.PIPELINED;
+                }
+            } else {
+                sendfileData.keepAliveState = SendfileKeepAliveState.NONE;
+            }
             switch (((Nio2Endpoint) endpoint).processSendfile(
                     (Nio2Endpoint.Nio2SocketWrapper) socketWrapper)) {
             case DONE:

Modified: tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Mon Mar 27 19:52:30 2017
@@ -37,6 +37,7 @@ import org.apache.tomcat.util.net.NioEnd
 import org.apache.tomcat.util.net.NioEndpoint.KeyAttachment;
 import org.apache.tomcat.util.net.SSLSupport;
 import org.apache.tomcat.util.net.SecureNioChannel;
+import org.apache.tomcat.util.net.SendfileKeepAliveState;
 import org.apache.tomcat.util.net.SocketStatus;
 import org.apache.tomcat.util.net.SocketWrapper;
 
@@ -278,7 +279,15 @@ public class Http11NioProcessor extends
         // Do sendfile as needed: add socket to sendfile and end
         if (sendfileData != null && !getErrorState().isError()) {
             ((KeyAttachment) socketWrapper).setSendfileData(sendfileData);
-            sendfileData.keepAlive = keepAlive;
+            if (keepAlive) {
+                if (getInputBuffer().available(false) == 0) {
+                    sendfileData.keepAliveState = SendfileKeepAliveState.OPEN;
+                } else {
+                    sendfileData.keepAliveState = SendfileKeepAliveState.PIPELINED;
+                }
+            } else {
+                sendfileData.keepAliveState = SendfileKeepAliveState.NONE;
+            }
             SelectionKey key = socketWrapper.getSocket().getIOChannel().keyFor(
                     socketWrapper.getSocket().getPoller().getSelector());
             //do the first write on this thread, might as well

Modified: tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Mon Mar 27 19:52:30 2017
@@ -2078,7 +2078,7 @@ public class AprEndpoint extends Abstrac
         // Position
         public long pos;
         // KeepAlive flag
-        public boolean keepAlive;
+        public SendfileKeepAliveState keepAliveState = SendfileKeepAliveState.NONE;
     }
 
 
@@ -2321,20 +2321,33 @@ public class AprEndpoint extends Abstrac
                             state.pos = state.pos + nw;
                             if (state.pos >= state.end) {
                                 remove(state);
-                                if (state.keepAlive) {
+                                switch (state.keepAliveState) {
+                                case NONE: {
+                                    // Close the socket since this is
+                                    // the end of the not keep-alive request.
+                                    closeSocket(state.socket);
+                                    break;
+                                }
+                                case PIPELINED: {
+                                    // Destroy file descriptor pool, which should close the file
+                                    Pool.destroy(state.fdpool);
+                                    Socket.timeoutSet(state.socket, getSoTimeout() * 1000);
+                                    // Process the pipelined request data
+                                    if (!processSocket(state.socket, SocketStatus.OPEN_READ)) {
+                                        closeSocket(state.socket);
+                                    }
+                                    break;
+                                }
+                                case OPEN: {
                                     // Destroy file descriptor pool, which should close the file
                                     Pool.destroy(state.fdpool);
-                                    Socket.timeoutSet(state.socket,
-                                            getSoTimeout() * 1000);
-                                    // If all done put the socket back in the
-                                    // poller for processing of further requests
-                                    getPoller().add(
-                                            state.socket, getKeepAliveTimeout(),
+                                    Socket.timeoutSet(state.socket, getSoTimeout() * 1000);
+                                    // Put the socket back in the poller for
+                                    // processing of further requests
+                                    getPoller().add(state.socket, getKeepAliveTimeout(),
                                             Poll.APR_POLLIN);
-                                } else {
-                                    // Close the socket since this is
-                                    // the end of not keep-alive request.
-                                    closeSocket(state.socket);
+                                    break;
+                                }
                                 }
                             }
                         }

Modified: tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Mon Mar 27 19:52:30 2017
@@ -914,17 +914,22 @@ public class Nio2Endpoint extends Abstra
                     } catch (IOException e) {
                         // Ignore
                     }
-                    if (attachment.keepAlive) {
-                        if (!isInline()) {
-                            awaitBytes(attachment.socket);
-                        } else {
-                            attachment.doneInline = true;
-                        }
+                    if (isInline()) {
+                        attachment.doneInline = true;
                     } else {
-                        if (!isInline()) {
+                        switch (attachment.keepAliveState) {
+                        case NONE: {
                             processSocket(attachment.socket, SocketStatus.DISCONNECT, false);
-                        } else {
-                            attachment.doneInline = true;
+                            break;
+                        }
+                        case PIPELINED: {
+                            processSocket(attachment.socket, SocketStatus.OPEN_READ, true);
+                            break;
+                        }
+                        case OPEN: {
+                            awaitBytes(attachment.socket);
+                            break;
+                        }
                         }
                     }
                     return;
@@ -1162,7 +1167,7 @@ public class Nio2Endpoint extends Abstra
         public long pos;
         public long length;
         // KeepAlive flag
-        public boolean keepAlive;
+        public SendfileKeepAliveState keepAliveState = SendfileKeepAliveState.NONE;
         // Internal use only
         private Nio2SocketWrapper socket;
         private ByteBuffer buffer;

Modified: tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Mon Mar 27 19:52:30 2017
@@ -1214,16 +1214,30 @@ public class NioEndpoint extends Abstrac
                     // responsible for registering the socket for the
                     // appropriate event(s) if sendfile completes.
                     if (!calledByProcessor) {
-                        if (sd.keepAlive) {
-                            if (log.isDebugEnabled()) {
-                                log.debug("Connection is keep alive, registering back for OP_READ");
-                            }
-                            reg(sk,attachment,SelectionKey.OP_READ);
-                        } else {
+                        switch (sd.keepAliveState) {
+                        case NONE: {
                             if (log.isDebugEnabled()) {
                                 log.debug("Send file connection is being closed");
                             }
                             cancelledKey(sk,SocketStatus.STOP);
+                            break;
+                        }
+                        case PIPELINED: {
+                            if (log.isDebugEnabled()) {
+                                log.debug("Connection is keep alive, processing pipe-lined data");
+                            }
+                            if (!processSocket(attachment, SocketStatus.OPEN_READ, true)) {
+                                cancelledKey(sk, SocketStatus.DISCONNECT);
+                            }
+                            break;
+                        }
+                        case OPEN: {
+                            if (log.isDebugEnabled()) {
+                                log.debug("Connection is keep alive, registering back for OP_READ");
+                            }
+                            reg(sk, attachment, SelectionKey.OP_READ);
+                            break;
+                        }
                         }
                     }
                     return SendfileState.DONE;
@@ -1589,6 +1603,6 @@ public class NioEndpoint extends Abstrac
         public volatile long pos;
         public volatile long length;
         // KeepAlive flag
-        public volatile boolean keepAlive;
+        public SendfileKeepAliveState keepAliveState = SendfileKeepAliveState.NONE;
     }
 }

Added: tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/SendfileKeepAliveState.java
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/SendfileKeepAliveState.java?rev=1788999&view=auto
==============================================================================
--- tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/SendfileKeepAliveState.java (added)
+++ tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/SendfileKeepAliveState.java Mon Mar 27 19:52:30 2017
@@ -0,0 +1,39 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.tomcat.util.net;
+
+public enum SendfileKeepAliveState {
+
+    /**
+     * Keep-alive is not in use. The socket can be closed when the response has
+     * been written.
+     */
+    NONE,
+
+    /**
+     * Keep-alive is in use and there is pipelined data in the input buffer to
+     * be read as soon as the current response has been written.
+     */
+    PIPELINED,
+
+    /**
+     * Keep-alive is in use. The socket should be added to the poller (or
+     * equivalent) to await more data as soon as the current response has been
+     * written.
+     */
+    OPEN
+}

Propchange: tomcat/tc8.0.x/trunk/java/org/apache/tomcat/util/net/SendfileKeepAliveState.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml?rev=1788999&r1=1788998&r2=1788999&view=diff
==============================================================================
--- tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc8.0.x/trunk/webapps/docs/changelog.xml Mon Mar 27 19:52:30 2017
@@ -75,6 +75,9 @@
         configuration attributes and internal code. Based on a patch by Michael
         Osipov. (markt)
       </fix>
+      <fix>
+        Improve sendfile handling when requests are pipelined. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">



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