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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org