Merge. Refresh from viewer-drano-http-4 after switch to 7.38.0+ and handle cache work.

4 files changed, 177 insertions, 8 deletions

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