Skip to content

Commit 864aa11

Browse files
markt-asfmarkt-asf
committed
Improve sendfile handling when requests are pipelined.
git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1788890 13f79535-47bb-0310-9956-ffa450edef68
1 parent 9a3f0fb commit 864aa11

8 files changed

Lines changed: 118 additions & 34 deletions

File tree

‎java/org/apache/coyote/AbstractProtocol.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -868,10 +868,9 @@ public SocketState process(SocketWrapperBase<S> wrapper, SocketEvent status) {
868868
wrapper.registerReadInterest();
869869
} else if (state == SocketState.SENDFILE) {
870870
// Sendfile in progress. If it fails, the socket will be
871-
// closed. If it works, the socket will be re-added to the
872-
// poller
873-
connections.remove(socket);
874-
release(processor);
871+
// closed. If it works, the socket either be added to the
872+
// poller (or equivalent) to await more data or processed
873+
// if there are any pipe-lined requests remaining.
875874
} else if (state == SocketState.UPGRADED) {
876875
// Don't add sockets back to the poller if this was a
877876
// non-blocking write otherwise the poller may trigger

‎java/org/apache/coyote/http11/Http11Processor.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@
5555
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
5656
import org.apache.tomcat.util.net.SSLSupport;
5757
import org.apache.tomcat.util.net.SendfileDataBase;
58+
import org.apache.tomcat.util.net.SendfileKeepAliveState;
5859
import org.apache.tomcat.util.net.SendfileState;
5960
import org.apache.tomcat.util.net.SocketWrapperBase;
6061
import org.apache.tomcat.util.res.StringManager;
@@ -1312,7 +1313,15 @@ private SendfileState processSendfile(SocketWrapperBase<?> socketWrapper) {
13121313
SendfileState result = SendfileState.DONE;
13131314
// Do sendfile as needed: add socket to sendfile and end
13141315
if (sendfileData != null && !getErrorState().isError()) {
1315-
sendfileData.keepAlive = keepAlive;
1316+
if (keepAlive) {
1317+
if (available(false) == 0) {
1318+
sendfileData.keepAliveState = SendfileKeepAliveState.OPEN;
1319+
} else {
1320+
sendfileData.keepAliveState = SendfileKeepAliveState.PIPELINED;
1321+
}
1322+
} else {
1323+
sendfileData.keepAliveState = SendfileKeepAliveState.NONE;
1324+
}
13161325
result = socketWrapper.processSendfile(sendfileData);
13171326
switch (result) {
13181327
case ERROR:

‎java/org/apache/tomcat/util/net/AprEndpoint.java‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2064,20 +2064,33 @@ public void run() {
20642064
state.length -= nw;
20652065
if (state.length == 0) {
20662066
remove(state);
2067-
if (state.keepAlive) {
2067+
switch (state.keepAliveState) {
2068+
case NONE: {
2069+
// Close the socket since this is
2070+
// the end of the not keep-alive request.
2071+
closeSocket(state.socket);
2072+
break;
2073+
}
2074+
case PIPELINED: {
20682075
// Destroy file descriptor pool, which should close the file
20692076
Pool.destroy(state.fdpool);
2070-
Socket.timeoutSet(state.socket,
2071-
getConnectionTimeout() * 1000);
2072-
// If all done put the socket back in the
2073-
// poller for processing of further requests
2074-
getPoller().add(
2075-
state.socket, getKeepAliveTimeout(),
2077+
Socket.timeoutSet(state.socket, getConnectionTimeout() * 1000);
2078+
// Process the pipelined request data
2079+
if (!processSocket(state.socket, SocketEvent.OPEN_READ)) {
2080+
closeSocket(state.socket);
2081+
}
2082+
break;
2083+
}
2084+
case OPEN: {
2085+
// Destroy file descriptor pool, which should close the file
2086+
Pool.destroy(state.fdpool);
2087+
Socket.timeoutSet(state.socket, getConnectionTimeout() * 1000);
2088+
// Put the socket back in the poller for
2089+
// processing of further requests
2090+
getPoller().add(state.socket, getKeepAliveTimeout(),
20762091
Poll.APR_POLLIN);
2077-
} else {
2078-
// Close the socket since this is
2079-
// the end of not keep-alive request.
2080-
closeSocket(state.socket);
2092+
break;
2093+
}
20812094
}
20822095
}
20832096
}

‎java/org/apache/tomcat/util/net/Nio2Endpoint.java‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -447,17 +447,24 @@ public void completed(Integer nWrite, SendfileData attachment) {
447447
} catch (IOException e) {
448448
// Ignore
449449
}
450-
if (attachment.keepAlive) {
451-
if (!isInline()) {
450+
if (isInline()) {
451+
attachment.doneInline = true;
452+
} else {
453+
switch (attachment.keepAliveState) {
454+
case NONE: {
455+
getEndpoint().processSocket(Nio2SocketWrapper.this,
456+
SocketEvent.DISCONNECT, false);
457+
break;
458+
}
459+
case PIPELINED: {
460+
getEndpoint().processSocket(Nio2SocketWrapper.this,
461+
SocketEvent.OPEN_READ, true);
462+
break;
463+
}
464+
case OPEN: {
452465
awaitBytes();
453-
} else {
454-
attachment.doneInline = true;
466+
break;
455467
}
456-
} else {
457-
if (!isInline()) {
458-
getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.DISCONNECT, false);
459-
} else {
460-
attachment.doneInline = true;
461468
}
462469
}
463470
return;

‎java/org/apache/tomcat/util/net/NioEndpoint.java‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -837,16 +837,30 @@ public SendfileState processSendfile(SelectionKey sk, NioSocketWrapper socketWra
837837
// responsible for registering the socket for the
838838
// appropriate event(s) if sendfile completes.
839839
if (!calledByProcessor) {
840-
if (sd.keepAlive) {
841-
if (log.isDebugEnabled()) {
842-
log.debug("Connection is keep alive, registering back for OP_READ");
843-
}
844-
reg(sk,socketWrapper,SelectionKey.OP_READ);
845-
} else {
840+
switch (sd.keepAliveState) {
841+
case NONE: {
846842
if (log.isDebugEnabled()) {
847843
log.debug("Send file connection is being closed");
848844
}
849845
close(sc, sk);
846+
break;
847+
}
848+
case PIPELINED: {
849+
if (log.isDebugEnabled()) {
850+
log.debug("Connection is keep alive, processing pipe-lined data");
851+
}
852+
if (!processSocket(socketWrapper, SocketEvent.OPEN_READ, true)) {
853+
close(sc, sk);
854+
}
855+
break;
856+
}
857+
case OPEN: {
858+
if (log.isDebugEnabled()) {
859+
log.debug("Connection is keep alive, registering back for OP_READ");
860+
}
861+
reg(sk,socketWrapper,SelectionKey.OP_READ);
862+
break;
863+
}
850864
}
851865
}
852866
return SendfileState.DONE;

‎java/org/apache/tomcat/util/net/SendfileDataBase.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ public abstract class SendfileDataBase {
2121
/**
2222
* Is the current request being processed on a keep-alive connection? This
2323
* determines if the socket is closed once the send file completes or if
24-
* processing continues with the next request on the connection (or waiting
25-
* for that next request to arrive).
24+
* processing continues with the next request on the connection or waiting
25+
* for that next request to arrive.
2626
*/
27-
public boolean keepAlive;
27+
public SendfileKeepAliveState keepAliveState = SendfileKeepAliveState.NONE;
2828

2929
/**
3030
* The full path to the file that contains the data to be written to the

‎java/org/apache/tomcat/util/net/SendfileKeepAliveState.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.tomcat.util.net;
18+
19+
public enum SendfileKeepAliveState {
20+
21+
/**
22+
* Keep-alive is not in use. The socket can be closed when the response has
23+
* been written.
24+
*/
25+
NONE,
26+
27+
/**
28+
* Keep-alive is in use and there is pipelined data in the input buffer to
29+
* be read as soon as the current response has been written.
30+
*/
31+
PIPELINED,
32+
33+
/**
34+
* Keep-alive is in use. The socket should be added to the poller (or
35+
* equivalent) to await more data as soon as the current response has been
36+
* written.
37+
*/
38+
OPEN
39+
}

‎webapps/docs/changelog.xml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,9 @@
145145
subsequent requests experiencing an <code>IllegalStateException</code>.
146146
(markt)
147147
</fix>
148+
<fix>
149+
Improve sendfile handling when requests are pipelined. (markt)
150+
</fix>
148151
</changelog>
149152
</subsection>
150153
<subsection name="Jasper">

0 commit comments

Comments
 (0)
⚡