Skip to content

Commit

Permalink
Close file handle for simple HTTP reads.
Browse files Browse the repository at this point in the history
If a client requested a single byte range which was less than
the filesize, then the file handle was never explicitly closed.

When the TCP connection was closed, all open file handles on the
bridge are automatically closed too.  Hence, it seems like this is
a moot point - a transient "leak".  However, we have observed some
clients (particularly: CVMFS) that hold open TCP connections for
hours or days, resulting in the "leak" of a file handle per read
operation.  In some cases, the number of file handles leaked was in
the tens-of-thousands.

In addition to the obvious memory issues, this was observable in the
f-stream monitoring as the near-simultaneous closing of 10k file handles
caused UDP buffer overflows and dropped monitoring packets.
  • Loading branch information
bbockelm committed Mar 5, 2018
1 parent ee3fb8e commit 7820587
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
15 changes: 14 additions & 1 deletion src/XrdHttp/XrdHttpReq.cc
Expand Up @@ -302,6 +302,7 @@ int XrdHttpReq::parseRWOp(char *str) {
len_ok += len;
rwOps_split.push_back(nfo);
}
length += len_ok;


}
Expand Down Expand Up @@ -984,7 +985,7 @@ int XrdHttpReq::ProcessHTTPReq() {
{

if ( ((reqstate == 3) && (rwOps.size() > 1)) ||
(writtenbytes >= filesize) ) {
(writtenbytes >= length) ) {

// Close() if this was a readv or we have finished, otherwise read the next chunk

Expand Down Expand Up @@ -1677,6 +1678,12 @@ int XrdHttpReq::PostProcessHTTPReq(bool final_) {
&filesize,
&fileflags,
&filemodtime);

// We will default the response size specified by the headers; if that
// wasn't given, use the file size.
if (!length) {
length = filesize;
}
}
else
TRACEI(REQ, "Can't find the stat information for '" << resource << "' Internal error?");
Expand Down Expand Up @@ -1716,6 +1723,12 @@ int XrdHttpReq::PostProcessHTTPReq(bool final_) {
&filesize,
&fileflags,
&filemodtime);

// As above: if the client specified a response size, we use that.
// Otherwise, utilize the filesize
if (!length) {
length = filesize;
}
}
else
TRACEI(ALL, "GET returned no STAT information. Internal error?");
Expand Down
2 changes: 1 addition & 1 deletion src/XrdHttp/XrdHttpReq.hh
Expand Up @@ -187,7 +187,7 @@ public:
std::vector<ReadWriteOp> rwOps_split;

bool keepalive;
long long length;
long long length; // Total size from client for PUT; total length of response TO client for GET.
int depth;
bool sendcontinue;

Expand Down

0 comments on commit 7820587

Please sign in to comment.