diff options
| -rwxr-xr-x | autobuild.xml | 12 | ||||
| -rwxr-xr-x | indra/llcorehttp/_httplibcurl.cpp | 83 | ||||
| -rwxr-xr-x | indra/llcorehttp/_httplibcurl.h | 78 | ||||
| -rwxr-xr-x | indra/llcorehttp/_httpoprequest.cpp | 20 | ||||
| -rwxr-xr-x | indra/llcorehttp/httpcommon.cpp | 4 |
5 files changed, 183 insertions, 14 deletions
diff --git a/autobuild.xml b/autobuild.xml index b6586b25b7..52f718716c 100755 --- a/autobuild.xml +++ b/autobuild.xml @@ -282,9 +282,9 @@ <key>archive</key> <map> <key>hash</key> - <string>f5a699c93beb1a854d0b51382b5cecc8</string> + <string>7e5385eedde808e51e602deca1879b22</string> <key>url</key> - <string>http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/290664/arch/Darwin/installer/curl-7.37.0-darwin-20140605.tar.bz2</string> + <string>http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/http_3p-curl-experimental/rev/294357/arch/Darwin/installer/curl-7.38.0-darwin-20140918.tar.bz2</string> </map> <key>name</key> <string>darwin</string> @@ -294,9 +294,9 @@ <key>archive</key> <map> <key>hash</key> - <string>2bc285edffd0e55e0cd6290f39854a89</string> + <string>c85406fb9cad19b5602b90c55a169a6a</string> <key>url</key> - <string>http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/290664/arch/Linux/installer/curl-7.37.0-linux-20140605.tar.bz2</string> + <string>http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/http_3p-curl-experimental/rev/294357/arch/Linux/installer/curl-7.38.0-linux-20140918.tar.bz2</string> </map> <key>name</key> <string>linux</string> @@ -306,9 +306,9 @@ <key>archive</key> <map> <key>hash</key> - <string>8d3b197d7a114d2b688d2831a0a59757</string> + <string>9fe81decac23f3179a2af58d23baf6ce</string> <key>url</key> - <string>http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/3pl_3p-curl-update/rev/290664/arch/CYGWIN/installer/curl-7.37.0-windows-20140605.tar.bz2</string> + <string>http://automated-builds-secondlife-com.s3.amazonaws.com/hg/repo/http_3p-curl-experimental/rev/294357/arch/CYGWIN/installer/curl-7.38.0-windows-20140918.tar.bz2</string> </map> <key>name</key> <string>windows</string> diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp index cfbe0fd2bb..81b44ab90b 100755 --- a/indra/llcorehttp/_httplibcurl.cpp +++ b/indra/llcorehttp/_httplibcurl.cpp @@ -51,6 +51,7 @@ namespace LLCore HttpLibcurl::HttpLibcurl(HttpService * service) : mService(service), + mHandleCache(), mPolicyCount(0), mMultiHandles(NULL), mActiveHandles(NULL), @@ -61,7 +62,7 @@ HttpLibcurl::HttpLibcurl(HttpService * service) HttpLibcurl::~HttpLibcurl() { shutdown(); - + mService = NULL; } @@ -279,7 +280,7 @@ void HttpLibcurl::cancelRequest(HttpOpRequest * op) // Detach from multi and recycle handle curl_multi_remove_handle(mMultiHandles[op->mReqPolicy], op->mCurlHandle); - curl_easy_cleanup(op->mCurlHandle); + mHandleCache.freeHandle(op->mCurlHandle); op->mCurlHandle = NULL; // Tracing @@ -356,7 +357,7 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode // Detach from multi and recycle handle curl_multi_remove_handle(multi_handle, handle); - curl_easy_cleanup(handle); + mHandleCache.freeHandle(op->mCurlHandle); op->mCurlHandle = NULL; // Tracing @@ -471,6 +472,82 @@ void HttpLibcurl::policyUpdated(int policy_class) } } +// --------------------------------------- +// HttpLibcurl::HandleCache +// --------------------------------------- + +HttpLibcurl::HandleCache::HandleCache() + : mHandleTemplate(NULL) +{ + mCache.reserve(50); +} + + +HttpLibcurl::HandleCache::~HandleCache() +{ + if (mHandleTemplate) + { + curl_easy_cleanup(mHandleTemplate); + mHandleTemplate = NULL; + } + + for (handle_cache_t::iterator it(mCache.begin()); mCache.end() != it; ++it) + { + curl_easy_cleanup(*it); + } + mCache.clear(); +} + + +CURL * HttpLibcurl::HandleCache::getHandle() +{ + CURL * ret(NULL); + + if (! mCache.empty()) + { + // Fastest path to handle + ret = mCache.back(); + mCache.pop_back(); + } + else if (mHandleTemplate) + { + // Still fast path + ret = curl_easy_duphandle(mHandleTemplate); + } + else + { + // When all else fails + ret = curl_easy_init(); + } + + return ret; +} + + +void HttpLibcurl::HandleCache::freeHandle(CURL * handle) +{ + if (! handle) + { + return; + } + + curl_easy_reset(handle); + if (! mHandleTemplate) + { + // Save the first freed handle as a template. + mHandleTemplate = handle; + } + else + { + // Otherwise add it to the cache + if (mCache.size() >= mCache.capacity()) + { + mCache.reserve(mCache.capacity() + 50); + } + mCache.push_back(handle); + } +} + // --------------------------------------- // Free functions diff --git a/indra/llcorehttp/_httplibcurl.h b/indra/llcorehttp/_httplibcurl.h index 2c7ad1fa8e..ffc24c63a8 100755 --- a/indra/llcorehttp/_httplibcurl.h +++ b/indra/llcorehttp/_httplibcurl.h @@ -124,6 +124,23 @@ public: /// Threading: called by worker thread. void policyUpdated(int policy_class); + /// Allocate a curl handle for caller. May be freed using + /// either the freeHandle() method or calling curl_easy_cleanup() + /// directly. + /// + /// @return Libcurl handle (CURL *) or NULL on allocation + /// problem. Handle will be in curl_easy_reset() + /// condition. + /// + /// Threading: callable by worker thread. + /// + /// Deprecation: Expect this to go away after _httpoprequest is + /// refactored bringing code into this class. + CURL * getHandle() + { + return mHandleCache.getHandle(); + } + protected: /// Invoked when libcurl has indicated a request has been processed /// to completion and we need to move the request to a new state. @@ -135,14 +152,67 @@ protected: protected: typedef std::set<HttpOpRequest *> active_set_t; + + /// Simple request handle cache for libcurl. + /// + /// Handle creation is somewhat slow and chunky in libcurl and there's + /// a pretty good speedup to be had from handle re-use. So, a simple + /// vector is kept of 'freed' handles to be reused as needed. When + /// that is empty, the first freed handle is kept as a template for + /// handle duplication. This is still faster than creation from nothing. + /// And when that fails, we init fresh from curl_easy_init(). + /// + /// Handles allocated with getHandle() may be freed with either + /// freeHandle() or curl_easy_cleanup(). Choice may be dictated + /// by thread constraints. + /// + /// Threading: Single-threaded. May only be used by a single thread, + /// typically the worker thread. If freeing requests' handles in an + /// unknown threading context, use curl_easy_cleanup() for safety. + + class HandleCache + { + public: + HandleCache(); + ~HandleCache(); + + private: + HandleCache(const HandleCache &); // Not defined + void operator=(const HandleCache &); // Not defined + + public: + /// Allocate a curl handle for caller. May be freed using + /// either the freeHandle() method or calling curl_easy_cleanup() + /// directly. + /// + /// @return Libcurl handle (CURL *) or NULL on allocation + /// problem. + /// + /// Threading: Single-thread (worker) only. + CURL * getHandle(); + + /// Free a libcurl handle acquired by whatever means. Thread + /// safety is left to the caller. + /// + /// Threading: Single-thread (worker) only. + void freeHandle(CURL * handle); + + protected: + typedef std::vector<CURL *> handle_cache_t; + + protected: + CURL * mHandleTemplate; // Template for duplicating new handles + handle_cache_t mCache; // Cache of old handles + }; // end class HandleCache protected: - HttpService * mService; // Simple reference, not owner + HttpService * mService; // Simple reference, not owner + HandleCache mHandleCache; // Handle allocator, owner active_set_t mActiveOps; int mPolicyCount; - CURLM ** mMultiHandles; // One handle per policy class - int * mActiveHandles; // Active count per policy class - bool * mDirtyPolicy; // Dirty policy update waiting for stall (per pc) + CURLM ** mMultiHandles; // One handle per policy class + int * mActiveHandles; // Active count per policy class + bool * mDirtyPolicy; // Dirty policy update waiting for stall (per pc) }; // end class HttpLibcurl diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp index 38c1f1e78a..bbda0b82fd 100755 --- a/indra/llcorehttp/_httpoprequest.cpp +++ b/indra/llcorehttp/_httpoprequest.cpp @@ -47,6 +47,19 @@ #include "llhttpconstants.h" #include "llproxy.h" +// *DEBUG: "[curl:bugs] #1420" problem and testing. +// +// A pipelining problem, https://sourceforge.net/p/curl/bugs/1420/, +// was a source of Core_9 failures. Code related to this can be +// identified and tested by: +// * Looking for '[curl:bugs]' strings in source and following +// instructions there. +// * Set 'QAModeHttpTrace' to 2 or 3 in settings.xml and look for +// 'timed out' events in the log. +// * Enable the HttpRangeRequestsDisable debug setting which causes +// full asset fetches. These slow the pipelines down a bit. +// + namespace { @@ -157,6 +170,8 @@ HttpOpRequest::~HttpOpRequest() if (mCurlHandle) { + // Uncertain of thread context so free using + // safest method. curl_easy_cleanup(mCurlHandle); mCurlHandle = NULL; } @@ -416,7 +431,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) HttpPolicyGlobal & gpolicy(service->getPolicy().getGlobalOptions()); HttpPolicyClass & cpolicy(service->getPolicy().getClassOptions(mReqPolicy)); - mCurlHandle = LLCurl::createStandardCurlHandle(); + mCurlHandle = service->getTransport().getHandle(); if (! mCurlHandle) { // We're in trouble. We'll continue but it won't go well. @@ -424,6 +439,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) << LL_ENDL; return HttpStatus(HttpStatus::LLCORE, HE_BAD_ALLOC); } + code = curl_easy_setopt(mCurlHandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4); check_curl_easy_code(code, CURLOPT_IPRESOLVE); code = curl_easy_setopt(mCurlHandle, CURLOPT_NOSIGNAL, 1); @@ -610,6 +626,8 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service) // *TODO: Find a better scheme than timeouts to guarantee liveness. xfer_timeout *= cpolicy.mPipelining; } + // *DEBUG: Useful for timeout handling and "[curl:bugs] #1420" tests + // xfer_timeout = 3L; code = curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, xfer_timeout); check_curl_easy_code(code, CURLOPT_TIMEOUT); code = curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, timeout); diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp index 9bcf7ac5e3..8714915fa2 100755 --- a/indra/llcorehttp/httpcommon.cpp +++ b/indra/llcorehttp/httpcommon.cpp @@ -240,6 +240,10 @@ bool HttpStatus::isRetryable() const static const HttpStatus inv_cont_range(HttpStatus::LLCORE, HE_INV_CONTENT_RANGE_HDR); static const HttpStatus inv_status(HttpStatus::LLCORE, HE_INVALID_HTTP_STATUS); + // *DEBUG: For "[curl:bugs] #1420" tests. + // Disable the '*this == inv_status' test and look for 'Core_9' + // failures in log files. + return ((isHttpStatus() && mType >= 499 && mType <= 599) || // Include special 499 in retryables *this == cant_connect || // Connection reset/endpoint problems *this == cant_res_proxy || // DNS problems |