From 7820587863140c915a76ead1724056228df7baaf Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Wed, 28 Feb 2018 13:39:45 -0600 Subject: [PATCH] Close file handle for simple HTTP reads. 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. --- src/XrdHttp/XrdHttpReq.cc | 15 ++++++++++++++- src/XrdHttp/XrdHttpReq.hh | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/XrdHttp/XrdHttpReq.cc b/src/XrdHttp/XrdHttpReq.cc index 0aaa55bbd29..233a6d4cde7 100644 --- a/src/XrdHttp/XrdHttpReq.cc +++ b/src/XrdHttp/XrdHttpReq.cc @@ -302,6 +302,7 @@ int XrdHttpReq::parseRWOp(char *str) { len_ok += len; rwOps_split.push_back(nfo); } + length += len_ok; } @@ -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 @@ -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?"); @@ -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?"); diff --git a/src/XrdHttp/XrdHttpReq.hh b/src/XrdHttp/XrdHttpReq.hh index 7c9ee80913f..c364eaa6663 100644 --- a/src/XrdHttp/XrdHttpReq.hh +++ b/src/XrdHttp/XrdHttpReq.hh @@ -187,7 +187,7 @@ public: std::vector 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;