Skip to content

Commit f5e06b8

Browse files
markt-asfmarkt-asf
committed
Improve sendfile handling when requests are pipelined.
git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc8.5.x/trunk@1788932 13f79535-47bb-0310-9956-ffa450edef68
1 parent 4fef4b2 commit f5e06b8

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
@@ -933,10 +933,9 @@ public SocketState process(SocketWrapperBase<S> wrapper, SocketEvent status) {
933933
wrapper.registerReadInterest();
934934
} else if (state == SocketState.SENDFILE) {
935935
// Sendfile in progress. If it fails, the socket will be
936-
// closed. If it works, the socket will be re-added to the
937-
// poller
938-
connections.remove(socket);
939-
release(processor);
936+
// closed. If it works, the socket either be added to the
937+
// poller (or equivalent) to await more data or processed
938+
// if there are any pipe-lined requests remaining.
940939
} else if (state == SocketState.UPGRADED) {
941940
// Don't add sockets back to the poller if this was a
942941
// 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
@@ -58,6 +58,7 @@
5858
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
5959
import org.apache.tomcat.util.net.SSLSupport;
6060
import org.apache.tomcat.util.net.SendfileDataBase;
61+
import org.apache.tomcat.util.net.SendfileKeepAliveState;
6162
import org.apache.tomcat.util.net.SendfileState;
6263
import org.apache.tomcat.util.net.SocketWrapperBase;
6364
import org.apache.tomcat.util.res.StringManager;
@@ -1612,7 +1613,15 @@ private SendfileState processSendfile(SocketWrapperBase<?> socketWrapper) {
16121613
SendfileState result = SendfileState.DONE;
16131614
// Do sendfile as needed: add socket to sendfile and end
16141615
if (sendfileData != null && !getErrorState().isError()) {
1615-
sendfileData.keepAlive = keepAlive;
1616+
if (keepAlive) {
1617+
if (available(false) == 0) {
1618+
sendfileData.keepAliveState = SendfileKeepAliveState.OPEN;
1619+
} else {
1620+
sendfileData.keepAliveState = SendfileKeepAliveState.PIPELINED;
1621+
}
1622+
} else {
1623+
sendfileData.keepAliveState = SendfileKeepAliveState.NONE;
1624+
}
16161625
result = socketWrapper.processSendfile(sendfileData);
16171626
switch (result) {
16181627
case ERROR:

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

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2144,20 +2144,33 @@ public void run() {
21442144
state.length -= nw;
21452145
if (state.length == 0) {
21462146
remove(state);
2147-
if (state.keepAlive) {
2147+
switch (state.keepAliveState) {
2148+
case NONE: {
2149+
// Close the socket since this is
2150+
// the end of the not keep-alive request.
2151+
closeSocket(state.socket);
2152+
break;
2153+
}
2154+
case PIPELINED: {
21482155
// Destroy file descriptor pool, which should close the file
21492156
Pool.destroy(state.fdpool);
2150-
Socket.timeoutSet(state.socket,
2151-
getSoTimeout() * 1000);
2152-
// If all done put the socket back in the
2153-
// poller for processing of further requests
2154-
getPoller().add(
2155-
state.socket, getKeepAliveTimeout(),
2157+
Socket.timeoutSet(state.socket, getSoTimeout() * 1000);
2158+
// Process the pipelined request data
2159+
if (!processSocket(state.socket, SocketEvent.OPEN_READ)) {
2160+
closeSocket(state.socket);
2161+
}
2162+
break;
2163+
}
2164+
case OPEN: {
2165+
// Destroy file descriptor pool, which should close the file
2166+
Pool.destroy(state.fdpool);
2167+
Socket.timeoutSet(state.socket, getSoTimeout() * 1000);
2168+
// Put the socket back in the poller for
2169+
// processing of further requests
2170+
getPoller().add(state.socket, getKeepAliveTimeout(),
21562171
Poll.APR_POLLIN);
2157-
} else {
2158-
// Close the socket since this is
2159-
// the end of not keep-alive request.
2160-
closeSocket(state.socket);
2172+
break;
2173+
}
21612174
}
21622175
}
21632176
}

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -536,17 +536,24 @@ public void completed(Integer nWrite, SendfileData attachment) {
536536
} catch (IOException e) {
537537
// Ignore
538538
}
539-
if (attachment.keepAlive) {
540-
if (!isInline()) {
539+
if (isInline()) {
540+
attachment.doneInline = true;
541+
} else {
542+
switch (attachment.keepAliveState) {
543+
case NONE: {
544+
getEndpoint().processSocket(Nio2SocketWrapper.this,
545+
SocketEvent.DISCONNECT, false);
546+
break;
547+
}
548+
case PIPELINED: {
549+
getEndpoint().processSocket(Nio2SocketWrapper.this,
550+
SocketEvent.OPEN_READ, true);
551+
break;
552+
}
553+
case OPEN: {
541554
awaitBytes();
542-
} else {
543-
attachment.doneInline = true;
555+
break;
544556
}
545-
} else {
546-
if (!isInline()) {
547-
getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.DISCONNECT, false);
548-
} else {
549-
attachment.doneInline = true;
550557
}
551558
}
552559
return;

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

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -924,16 +924,30 @@ public SendfileState processSendfile(SelectionKey sk, NioSocketWrapper socketWra
924924
// responsible for registering the socket for the
925925
// appropriate event(s) if sendfile completes.
926926
if (!calledByProcessor) {
927-
if (sd.keepAlive) {
928-
if (log.isDebugEnabled()) {
929-
log.debug("Connection is keep alive, registering back for OP_READ");
930-
}
931-
reg(sk,socketWrapper,SelectionKey.OP_READ);
932-
} else {
927+
switch (sd.keepAliveState) {
928+
case NONE: {
933929
if (log.isDebugEnabled()) {
934930
log.debug("Send file connection is being closed");
935931
}
936932
close(sc, sk);
933+
break;
934+
}
935+
case PIPELINED: {
936+
if (log.isDebugEnabled()) {
937+
log.debug("Connection is keep alive, processing pipe-lined data");
938+
}
939+
if (!processSocket(socketWrapper, SocketEvent.OPEN_READ, true)) {
940+
close(sc, sk);
941+
}
942+
break;
943+
}
944+
case OPEN: {
945+
if (log.isDebugEnabled()) {
946+
log.debug("Connection is keep alive, registering back for OP_READ");
947+
}
948+
reg(sk,socketWrapper,SelectionKey.OP_READ);
949+
break;
950+
}
937951
}
938952
}
939953
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
@@ -127,6 +127,9 @@
127127
subsequent requests experiencing an <code>IllegalStateException</code>.
128128
(markt)
129129
</fix>
130+
<fix>
131+
Improve sendfile handling when requests are pipelined. (markt)
132+
</fix>
130133
</changelog>
131134
</subsection>
132135
<subsection name="Jasper">

0 commit comments

Comments
 (0)
⚡