From b8edacd0bb4feacc3ac1d61421e600c75ab87f7c Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Fri, 1 Jun 2012 14:07:34 -0400
Subject: Major steps towards implementing the policy component. Identified and
 reacted to the priority inversion problem we have in texturefetch.  Includes
 the introduction of a priority_queue for the requests that are ready.  Start
 some parameterization in anticipation of having policy_class everywhere. 
 Removed _assert.h which isn't really needed in indra codebase.  Implemented
 async setPriority request (which I hope I can get rid of eventually along
 with all priorities in this library).  Converted to using unsigned int for
 priority rather than float.  Implemented POST and did groundwork for PUT.

---
 indra/llcorehttp/_httpreadyqueue.h | 85 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 indra/llcorehttp/_httpreadyqueue.h

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
new file mode 100644
index 0000000000..283e868b4c
--- /dev/null
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -0,0 +1,85 @@
+/**
+ * @file _httpreadyqueue.h
+ * @brief Internal declaration for the operation ready queue
+ *
+ * $LicenseInfo:firstyear=2012&license=viewerlgpl$
+ * Second Life Viewer Source Code
+ * Copyright (C) 2012, Linden Research, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License only.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
+ * $/LicenseInfo$
+ */
+
+#ifndef	_LLCORE_HTTP_READY_QUEUE_H_
+#define	_LLCORE_HTTP_READY_QUEUE_H_
+
+
+#include <queue>
+
+#include "_httpoperation.h"
+
+
+namespace LLCore
+{
+
+class HttpOpRequest;
+
+/// HttpReadyQueue provides a simple priority queue for HttpOpRequest objects.
+///
+/// This uses the priority_queue adaptor class to provide the queue
+/// as well as the ordering scheme while allowing us access to the
+/// raw container if we follow a few simple rules.  One of the more
+/// important of those rules is that any iterator becomes invalid
+/// on element erasure.  So pay attention.
+///
+/// Threading:  not thread-safe.  Expected to be used entirely by
+/// a single thread, typically a worker thread of some sort.
+
+class HttpReadyQueue : public std::priority_queue<HttpOpRequest *,
+												  std::deque<HttpOpRequest *>,
+												  LLCore::HttpOpCompare>
+{
+public:
+	HttpReadyQueue()
+		: priority_queue()
+		{}
+	
+	~HttpReadyQueue()
+		{}
+	
+protected:
+	HttpReadyQueue(const HttpReadyQueue &);		// Not defined
+	void operator=(const HttpReadyQueue &);		// Not defined
+
+public:
+	const container_type & get_container() const
+		{
+			return c;
+		}
+
+	container_type & get_container()
+		{
+			return c;
+		}
+
+}; // end class HttpReadyQueue
+
+
+}  // end namespace LLCore
+
+
+#endif	// _LLCORE_HTTP_READY_QUEUE_H_
-- 
cgit v1.3


From 640798bb9951bc512bcbcffbe136d42372c99322 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Fri, 1 Jun 2012 18:18:53 -0400
Subject: Platform fixups:  typedef for priority_queue, more specific
 comparator functor.

---
 indra/llcorehttp/_httpoperation.h  | 13 -------------
 indra/llcorehttp/_httpoprequest.h  | 13 +++++++++++++
 indra/llcorehttp/_httpreadyqueue.h | 12 +++++++-----
 3 files changed, 20 insertions(+), 18 deletions(-)

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httpoperation.h b/indra/llcorehttp/_httpoperation.h
index 4d9298d801..01e26029d2 100644
--- a/indra/llcorehttp/_httpoperation.h
+++ b/indra/llcorehttp/_httpoperation.h
@@ -159,19 +159,6 @@ public:
 };  // end class HttpOpNull
 
 
-/// HttpOpCompare isn't an operation but a uniform comparison
-/// functor for STL containers that order by priority.  Mainly
-/// used for the ready queue container but defined here.
-class HttpOpCompare
-{
-public:
-	bool operator()(const HttpOperation * lhs, const HttpOperation * rhs)
-		{
-			return lhs->mReqPriority > rhs->mReqPriority;
-		}
-};  // end class HttpOpCompare
-
-	
 }   // end namespace LLCore
 
 #endif	// _LLCORE_HTTP_OPERATION_H_
diff --git a/indra/llcorehttp/_httpoprequest.h b/indra/llcorehttp/_httpoprequest.h
index e973f6ad26..0cad4e8459 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -131,6 +131,19 @@ public:
 };  // end class HttpOpRequest
 
 
+/// HttpOpRequestCompare isn't an operation but a uniform comparison
+/// functor for STL containers that order by priority.  Mainly
+/// used for the ready queue container but defined here.
+class HttpOpRequestCompare
+{
+public:
+	bool operator()(const HttpOpRequest * lhs, const HttpOpRequest * rhs)
+		{
+			return lhs->mReqPriority > rhs->mReqPriority;
+		}
+};  // end class HttpOpRequestCompare
+
+
 }   // end namespace LLCore
 
 #endif	// _LLCORE_HTTP_OPREQUEST_H_
diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
index 283e868b4c..2cd96aefe3 100644
--- a/indra/llcorehttp/_httpreadyqueue.h
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -30,7 +30,7 @@
 
 #include <queue>
 
-#include "_httpoperation.h"
+#include "_httpoprequest.h"
 
 
 namespace LLCore
@@ -49,13 +49,15 @@ class HttpOpRequest;
 /// Threading:  not thread-safe.  Expected to be used entirely by
 /// a single thread, typically a worker thread of some sort.
 
-class HttpReadyQueue : public std::priority_queue<HttpOpRequest *,
-												  std::deque<HttpOpRequest *>,
-												  LLCore::HttpOpCompare>
+typedef std::priority_queue<HttpOpRequest *,
+							std::deque<HttpOpRequest *>,
+							LLCore::HttpOpRequestCompare> HttpReadyQueueBase;
+
+class HttpReadyQueue : public HttpReadyQueueBase
 {
 public:
 	HttpReadyQueue()
-		: priority_queue()
+		: HttpReadyQueueBase()
 		{}
 	
 	~HttpReadyQueue()
-- 
cgit v1.3


From 28a04400b4160dd34166483ddcf0c12637bcc363 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Fri, 8 Jun 2012 20:21:54 -0400
Subject: Implemented HTTP retry for requests.  Went in rather easily which
 surprised me.  Added a retry queue similar to ready queue to the policy
 object which is sorted by retry time.  Currently do five retries (after the
 initial try) delayed by .25, .5, 1, 2 and 5 seconds.  Removed the retry logic
 from the lltexturefetch module. Upped the waiting time in the unit test for
 the retries.  People won't like this but tough, need tests.

---
 indra/llcorehttp/_httplibcurl.cpp           |  58 +++++++++--
 indra/llcorehttp/_httplibcurl.h             |   4 +-
 indra/llcorehttp/_httpoprequest.cpp         |  51 ++++++++--
 indra/llcorehttp/_httpoprequest.h           |   5 +
 indra/llcorehttp/_httppolicy.cpp            |  98 ++++++++++++++-----
 indra/llcorehttp/_httppolicy.h              |  19 +++-
 indra/llcorehttp/_httpreadyqueue.h          |   2 -
 indra/llcorehttp/_httpretryqueue.h          |  94 ++++++++++++++++++
 indra/llcorehttp/httpcommon.h               |  12 +++
 indra/llcorehttp/tests/test_httprequest.hpp |   2 +-
 indra/newview/lltexturefetch.cpp            | 145 ++++++++++++++++++++--------
 indra/newview/lltexturefetch.h              |   6 +-
 12 files changed, 402 insertions(+), 94 deletions(-)
 create mode 100644 indra/llcorehttp/_httpretryqueue.h

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp
index 5272c391e8..05b2c2be69 100644
--- a/indra/llcorehttp/_httplibcurl.cpp
+++ b/indra/llcorehttp/_httplibcurl.cpp
@@ -29,6 +29,7 @@
 #include "httpheaders.h"
 #include "bufferarray.h"
 #include "_httpoprequest.h"
+#include "_httppolicy.h"
 
 
 namespace LLCore
@@ -85,6 +86,8 @@ void HttpLibcurl::term()
 
 HttpService::ELoopSpeed HttpLibcurl::processTransport()
 {
+	HttpService::ELoopSpeed	ret(HttpService::REQUEST_SLEEP);
+
 	// Give libcurl some cycles to do I/O & callbacks
 	for (int policy_class(0); policy_class < HttpRequest::POLICY_CLASS_LIMIT; ++policy_class)
 	{
@@ -110,7 +113,8 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
 				CURL * handle(msg->easy_handle);
 				CURLcode result(msg->data.result);
 
-				completeRequest(mMultiHandles[policy_class], handle, result);
+				HttpService::ELoopSpeed	speed(completeRequest(mMultiHandles[policy_class], handle, result));
+				ret = (std::min)(ret, speed);
 				handle = NULL;			// No longer valid on return
 			}
 			else if (CURLMSG_NONE == msg->msg)
@@ -127,7 +131,11 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
 		}
 	}
 
-	return mActiveOps.empty() ? HttpService::REQUEST_SLEEP : HttpService::NORMAL;
+	if (! mActiveOps.empty())
+	{
+		ret = (std::min)(ret, HttpService::NORMAL);
+	}
+	return ret;
 }
 
 
@@ -153,8 +161,12 @@ void HttpLibcurl::addOp(HttpOpRequest * op)
 }
 
 
-void HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status)
+HttpService::ELoopSpeed HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status)
 {
+	static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
+	static const HttpStatus cant_res_proxy(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_PROXY);
+	static const HttpStatus cant_res_host(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_RESOLVE_HOST);
+
 	HttpOpRequest * op(NULL);
 	curl_easy_getinfo(handle, CURLINFO_PRIVATE, &op);
 	// *FIXME:  check the pointer
@@ -190,10 +202,7 @@ void HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode
 		int http_status(200);
 
 		curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &http_status);
-		op->mStatus = LLCore::HttpStatus(http_status,
-										 (http_status >= 200 && http_status <= 299
-										  ? HE_SUCCESS
-										  : HE_REPLY_ERROR));
+		op->mStatus = LLCore::HttpStatus(http_status);
 	}
 
 	// Detach from multi and recycle handle
@@ -201,9 +210,42 @@ void HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode
 	curl_easy_cleanup(handle);
 	op->mCurlHandle = NULL;
 	
-	// Deliver to reply queue and release
+	// Retry or finalize
+	if (! op->mStatus)
+	{
+		// If this failed, we might want to retry.  Have to inspect
+		// the status a little more deeply for those reasons worth retrying...
+		if (op->mPolicyRetries < op->mPolicyRetryLimit &&
+			((op->mStatus.isHttpStatus() && op->mStatus.mType >= 499 && op->mStatus.mType <= 599) ||
+			 cant_connect == op->mStatus ||
+			 cant_res_proxy == op->mStatus ||
+			 cant_res_host == op->mStatus))
+		{
+			// Okay, worth a retry.  We include 499 in this test as
+			// it's the old 'who knows?' error from many grid services...
+			HttpPolicy & policy(mService->getPolicy());
+		
+			policy.retryOp(op);
+			return HttpService::NORMAL;			// Having pushed to retry, keep things running
+		}
+	}
+
+	// This op is done, finalize it delivering it to the reply queue...
+	if (! op->mStatus)
+	{
+		LL_WARNS("CoreHttp") << "URL op failed after " << op->mPolicyRetries
+							 << " retries.  Reason:  " << op->mStatus.toString()
+							 << LL_ENDL;
+	}
+	else if (op->mPolicyRetries)
+	{
+		LL_WARNS("CoreHttp") << "URL op succeeded after " << op->mPolicyRetries << " retries."
+							 << LL_ENDL;
+	}
+	
 	op->stageFromActive(mService);
 	op->release();
+	return HttpService::REQUEST_SLEEP;
 }
 
 
diff --git a/indra/llcorehttp/_httplibcurl.h b/indra/llcorehttp/_httplibcurl.h
index ec325c1946..fe628b9ab0 100644
--- a/indra/llcorehttp/_httplibcurl.h
+++ b/indra/llcorehttp/_httplibcurl.h
@@ -83,7 +83,9 @@ public:
 protected:
 	/// Invoked when libcurl has indicated a request has been processed
 	/// to completion and we need to move the request to a new state.
-	void completeRequest(CURLM * multi_handle, CURL * handle, CURLcode status);
+	HttpService::ELoopSpeed completeRequest(CURLM * multi_handle,
+											CURL * handle,
+											CURLcode status);
 	
 protected:
 	typedef std::set<HttpOpRequest *> active_set_t;
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 4bdc4a5257..895629c514 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -99,8 +99,15 @@ HttpOpRequest::HttpOpRequest()
 	  mReplyBody(NULL),
 	  mReplyOffset(0),
 	  mReplyLength(0),
-	  mReplyHeaders(NULL)
-{}
+	  mReplyHeaders(NULL),
+	  mPolicyRetries(0),
+	  mPolicyRetryAt(HttpTime(0)),
+	  mPolicyRetryLimit(5)				// *FIXME:  Get from policy definitions
+{
+	// *NOTE:  As members are added, retry initialization/cleanup
+	// may need to be extended in @prepareRequest().
+}
+
 
 
 HttpOpRequest::~HttpOpRequest()
@@ -130,7 +137,6 @@ HttpOpRequest::~HttpOpRequest()
 	}
 
 	mCurlService = NULL;
-	
 
 	if (mCurlHeaders)
 	{
@@ -313,6 +319,30 @@ HttpStatus HttpOpRequest::setupPost(HttpRequest::policy_t policy_id,
 
 HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 {
+	// Scrub transport and result data for retried op case
+	mCurlActive = false;
+	mCurlHandle = NULL;
+	mCurlService = NULL;
+	if (mCurlHeaders)
+	{
+		curl_slist_free_all(mCurlHeaders);
+		mCurlHeaders = NULL;
+	}
+	mCurlBodyPos = 0;
+
+	if (mReplyBody)
+	{
+		mReplyBody->release();
+		mReplyBody = NULL;
+	}
+	mReplyOffset = 0;
+	mReplyLength = 0;
+	if (mReplyHeaders)
+	{
+		mReplyHeaders->release();
+		mReplyHeaders = NULL;
+	}
+	
 	// *FIXME:  better error handling later
 	HttpStatus status;
 
@@ -321,6 +351,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	
 	mCurlHandle = curl_easy_init();
 	// curl_easy_setopt(mCurlHandle, CURLOPT_VERBOSE, 1);
+	curl_easy_setopt(mCurlHandle, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4);
 	curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, 30);
 	curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, 30);
 	curl_easy_setopt(mCurlHandle, CURLOPT_NOSIGNAL, 1);
@@ -403,12 +434,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 		break;
 	}
 	
-	if (mReqHeaders)
-	{
-		mCurlHeaders = append_headers_to_slist(mReqHeaders, mCurlHeaders);
-	}
-	mCurlHeaders = curl_slist_append(mCurlHeaders, "Pragma:");
-	
+	// There's a CURLOPT for this now...
 	if ((mReqOffset || mReqLength) && HOR_GET == mReqMethod)
 	{
 		static const char * const fmt1("Range: bytes=%lu-%lu");
@@ -428,6 +454,13 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 		range_line[sizeof(range_line) - 1] = '\0';
 		mCurlHeaders = curl_slist_append(mCurlHeaders, range_line);
 	}
+
+	mCurlHeaders = curl_slist_append(mCurlHeaders, "Pragma:");
+	if (mReqHeaders)
+	{
+		// Caller's headers last to override
+		mCurlHeaders = append_headers_to_slist(mReqHeaders, mCurlHeaders);
+	}
 	curl_easy_setopt(mCurlHandle, CURLOPT_HTTPHEADER, mCurlHeaders);
 	
 	if (mProcFlags & (PF_SCAN_RANGE_HEADER | PF_SAVE_HEADERS))
diff --git a/indra/llcorehttp/_httpoprequest.h b/indra/llcorehttp/_httpoprequest.h
index 0cad4e8459..6dcf30ca0c 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -128,6 +128,11 @@ public:
 	off_t				mReplyOffset;
 	size_t				mReplyLength;
 	HttpHeaders *		mReplyHeaders;
+
+	// Policy data
+	int					mPolicyRetries;
+	HttpTime			mPolicyRetryAt;
+	const int			mPolicyRetryLimit;
 };  // end class HttpOpRequest
 
 
diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp
index 51f5e487dc..1f4cd34a4b 100644
--- a/indra/llcorehttp/_httppolicy.cpp
+++ b/indra/llcorehttp/_httppolicy.cpp
@@ -24,39 +24,46 @@
  * $/LicenseInfo$
  */
 
+#include "linden_common.h"
+
 #include "_httppolicy.h"
 
 #include "_httpoprequest.h"
 #include "_httpservice.h"
 #include "_httplibcurl.h"
 
+#include "lltimer.h"
+
 
 namespace LLCore
 {
 
 HttpPolicy::HttpPolicy(HttpService * service)
 	: mService(service)
-{
-	for (int policy_class(0); policy_class < HttpRequest::POLICY_CLASS_LIMIT; ++policy_class)
-	{
-		mReadyInClass[policy_class] = 0;
-	}
-}
+{}
 
 
 HttpPolicy::~HttpPolicy()
 {
-	for (int policy_class(0); policy_class < HttpRequest::POLICY_CLASS_LIMIT; ++policy_class)
+	for (int policy_class(0); policy_class < LL_ARRAY_SIZE(mState); ++policy_class)
 	{
-		HttpReadyQueue & readyq(mReadyQueue[policy_class]);
+		HttpRetryQueue & retryq(mState[policy_class].mRetryQueue);
+		while (! retryq.empty())
+		{
+			HttpOpRequest * op(retryq.top());
 		
+			op->cancel();
+			op->release();
+			retryq.pop();
+		}
+
+		HttpReadyQueue & readyq(mState[policy_class].mReadyQueue);
 		while (! readyq.empty())
 		{
 			HttpOpRequest * op(readyq.top());
 		
 			op->cancel();
 			op->release();
-			mReadyInClass[policy_class]--;
 			readyq.pop();
 		}
 	}
@@ -68,27 +75,69 @@ void HttpPolicy::addOp(HttpOpRequest * op)
 {
 	const int policy_class(op->mReqPolicy);
 	
-	mReadyQueue[policy_class].push(op);
-	++mReadyInClass[policy_class];
+	op->mPolicyRetries = 0;
+	mState[policy_class].mReadyQueue.push(op);
+}
+
+
+void HttpPolicy::retryOp(HttpOpRequest * op)
+{
+	static const HttpTime retry_deltas[] =
+		{
+			 250000,			// 1st retry in 0.25 S, etc...
+			 500000,
+			1000000,
+			2000000,
+			5000000				// ... to every 5.0 S.
+		};
+	static const int delta_max(int(LL_ARRAY_SIZE(retry_deltas)) - 1);
+	
+	const HttpTime now(totalTime());
+	const int policy_class(op->mReqPolicy);
+	
+	const HttpTime delta(retry_deltas[llclamp(op->mPolicyRetries, 0, delta_max)]);
+	op->mPolicyRetryAt = now + delta;
+	++op->mPolicyRetries;
+	LL_WARNS("CoreHttp") << "URL op retry #" << op->mPolicyRetries
+						 << " being scheduled for " << delta << " uSecs from now."
+						 << LL_ENDL;
+	mState[policy_class].mRetryQueue.push(op);
 }
 
 
 HttpService::ELoopSpeed HttpPolicy::processReadyQueue()
 {
+	const HttpTime now(totalTime());
 	HttpService::ELoopSpeed result(HttpService::REQUEST_SLEEP);
 	HttpLibcurl & transport(mService->getTransport());
 	
-	for (int policy_class(0); policy_class < HttpRequest::POLICY_CLASS_LIMIT; ++policy_class)
+	for (int policy_class(0); policy_class < LL_ARRAY_SIZE(mState); ++policy_class)
 	{
-		HttpReadyQueue & readyq(mReadyQueue[policy_class]);
 		int active(transport.getActiveCountInClass(policy_class));
 		int needed(8 - active);
 
-		if (needed > 0 && mReadyInClass[policy_class] > 0)
+		HttpRetryQueue & retryq(mState[policy_class].mRetryQueue);
+		HttpReadyQueue & readyq(mState[policy_class].mReadyQueue);
+		
+		if (needed > 0)
 		{
-			// Scan ready queue for requests that match policy
-
-			while (! readyq.empty() && needed > 0 && mReadyInClass[policy_class] > 0)
+			// First see if we have any retries...
+			while (needed > 0 && ! retryq.empty())
+			{
+				HttpOpRequest * op(retryq.top());
+				if (op->mPolicyRetryAt > now)
+					break;
+			
+				retryq.pop();
+				
+				op->stageFromReady(mService);
+				op->release();
+					
+				--needed;
+			}
+		
+			// Now go on to the new requests...
+			while (needed > 0 && ! readyq.empty())
 			{
 				HttpOpRequest * op(readyq.top());
 				readyq.pop();
@@ -96,17 +145,16 @@ HttpService::ELoopSpeed HttpPolicy::processReadyQueue()
 				op->stageFromReady(mService);
 				op->release();
 					
-				--mReadyInClass[policy_class];
 				--needed;
 			}
 		}
-
-		if (! readyq.empty())
+				
+		if (! readyq.empty() || ! retryq.empty())
 		{
 			// If anything is ready, continue looping...
 			result = (std::min)(result, HttpService::NORMAL);
 		}
-	}
+	} // end foreach policy_class
 
 	return result;
 }
@@ -114,9 +162,9 @@ HttpService::ELoopSpeed HttpPolicy::processReadyQueue()
 
 bool HttpPolicy::changePriority(HttpHandle handle, HttpRequest::priority_t priority)
 {
-	for (int policy_class(0); policy_class < HttpRequest::POLICY_CLASS_LIMIT; ++policy_class)
+	for (int policy_class(0); policy_class < LL_ARRAY_SIZE(mState); ++policy_class)
 	{
-		HttpReadyQueue::container_type & c(mReadyQueue[policy_class].get_container());
+		HttpReadyQueue::container_type & c(mState[policy_class].mReadyQueue.get_container());
 	
 		// Scan ready queue for requests that match policy
 		for (HttpReadyQueue::container_type::iterator iter(c.begin()); c.end() != iter;)
@@ -126,9 +174,9 @@ bool HttpPolicy::changePriority(HttpHandle handle, HttpRequest::priority_t prior
 			if (static_cast<HttpHandle>(*cur) == handle)
 			{
 				HttpOpRequest * op(*cur);
-				c.erase(cur);							// All iterators are now invalidated
+				c.erase(cur);									// All iterators are now invalidated
 				op->mReqPriority = priority;
-				mReadyQueue[policy_class].push(op);		// Re-insert using adapter class
+				mState[policy_class].mReadyQueue.push(op);		// Re-insert using adapter class
 				return true;
 			}
 		}
diff --git a/indra/llcorehttp/_httppolicy.h b/indra/llcorehttp/_httppolicy.h
index 425079ec63..6f18264f3d 100644
--- a/indra/llcorehttp/_httppolicy.h
+++ b/indra/llcorehttp/_httppolicy.h
@@ -31,6 +31,7 @@
 #include "httprequest.h"
 #include "_httpservice.h"
 #include "_httpreadyqueue.h"
+#include "_httpretryqueue.h"
 #include "_httppolicyglobal.h"
 
 
@@ -67,6 +68,14 @@ public:
 	/// additional references will be added.)
 	void addOp(HttpOpRequest *);
 
+	/// Similar to addOp, used when a caller wants to retry a
+	/// request that has failed.  It's placed on a special retry
+	/// queue but ordered by retry time not priority.  Otherwise,
+	/// handling is the same and retried operations are considered
+	/// before new ones but that doesn't guarantee completion
+	/// order.
+	void retryOp(HttpOpRequest *);
+
 	// Shadows HttpService's method
 	bool changePriority(HttpHandle handle, HttpRequest::priority_t priority);
 
@@ -77,10 +86,14 @@ public:
 			return mGlobalOptions;
 		}
 	
-	
 protected:
-	int					mReadyInClass[HttpRequest::POLICY_CLASS_LIMIT];
-	HttpReadyQueue		mReadyQueue[HttpRequest::POLICY_CLASS_LIMIT];
+	struct State
+	{
+		HttpReadyQueue		mReadyQueue;
+		HttpRetryQueue		mRetryQueue;
+	};
+
+	State				mState[HttpRequest::POLICY_CLASS_LIMIT];
 	HttpService *		mService;				// Naked pointer, not refcounted, not owner
 	HttpPolicyGlobal	mGlobalOptions;
 	
diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
index 2cd96aefe3..87828834dc 100644
--- a/indra/llcorehttp/_httpreadyqueue.h
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -36,8 +36,6 @@
 namespace LLCore
 {
 
-class HttpOpRequest;
-
 /// HttpReadyQueue provides a simple priority queue for HttpOpRequest objects.
 ///
 /// This uses the priority_queue adaptor class to provide the queue
diff --git a/indra/llcorehttp/_httpretryqueue.h b/indra/llcorehttp/_httpretryqueue.h
new file mode 100644
index 0000000000..745adec09d
--- /dev/null
+++ b/indra/llcorehttp/_httpretryqueue.h
@@ -0,0 +1,94 @@
+/**
+ * @file _httpretryqueue.h
+ * @brief Internal declaration for the operation retry queue
+ *
+ * $LicenseInfo:firstyear=2012&license=viewerlgpl$
+ * Second Life Viewer Source Code
+ * Copyright (C) 2012, Linden Research, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License only.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
+ * $/LicenseInfo$
+ */
+
+#ifndef	_LLCORE_HTTP_RETRY_QUEUE_H_
+#define	_LLCORE_HTTP_RETRY_QUEUE_H_
+
+
+#include <queue>
+
+#include "_httpoprequest.h"
+
+
+namespace LLCore
+{
+
+/// HttpRetryQueue provides a simple priority queue for HttpOpRequest objects.
+///
+/// This uses the priority_queue adaptor class to provide the queue
+/// as well as the ordering scheme while allowing us access to the
+/// raw container if we follow a few simple rules.  One of the more
+/// important of those rules is that any iterator becomes invalid
+/// on element erasure.  So pay attention.
+///
+/// Threading:  not thread-safe.  Expected to be used entirely by
+/// a single thread, typically a worker thread of some sort.
+
+struct HttpOpRetryCompare
+{
+	bool operator()(const HttpOpRequest * lhs, const HttpOpRequest * rhs)
+		{
+			return lhs->mPolicyRetryAt < rhs->mPolicyRetryAt;
+		}
+};
+
+	
+typedef std::priority_queue<HttpOpRequest *,
+							std::deque<HttpOpRequest *>,
+							LLCore::HttpOpRetryCompare> HttpRetryQueueBase;
+
+class HttpRetryQueue : public HttpRetryQueueBase
+{
+public:
+	HttpRetryQueue()
+		: HttpRetryQueueBase()
+		{}
+	
+	~HttpRetryQueue()
+		{}
+	
+protected:
+	HttpRetryQueue(const HttpRetryQueue &);		// Not defined
+	void operator=(const HttpRetryQueue &);		// Not defined
+
+public:
+	const container_type & get_container() const
+		{
+			return c;
+		}
+
+	container_type & get_container()
+		{
+			return c;
+		}
+
+}; // end class HttpRetryQueue
+
+
+}  // end namespace LLCore
+
+
+#endif	// _LLCORE_HTTP_RETRY_QUEUE_H_
diff --git a/indra/llcorehttp/httpcommon.h b/indra/llcorehttp/httpcommon.h
index fd2661b700..42b75edb41 100644
--- a/indra/llcorehttp/httpcommon.h
+++ b/indra/llcorehttp/httpcommon.h
@@ -114,6 +114,9 @@ namespace LLCore
 typedef void * HttpHandle;
 #define LLCORE_HTTP_HANDLE_INVALID		(NULL)
 
+/// For internal scheduling and metrics, we use a microsecond
+/// timebase compatible with the environment.
+typedef U64 HttpTime;
 
 /// Error codes defined by the library itself as distinct from
 /// libcurl (or any other transport provider).
@@ -180,6 +183,15 @@ struct HttpStatus
 		  mStatus(status)
 		{}
 	
+	HttpStatus(int http_status)
+		: mType(http_status),
+		  mStatus(http_status >= 200 && http_status <= 299
+				  ? HE_SUCCESS
+				  : HE_REPLY_ERROR)
+		{
+			llassert(http_status >= 100 && http_status <= 999);
+		}
+	
 	HttpStatus(const HttpStatus & rhs)
 		: mType(rhs.mType),
 		  mStatus(rhs.mStatus)
diff --git a/indra/llcorehttp/tests/test_httprequest.hpp b/indra/llcorehttp/tests/test_httprequest.hpp
index 0e9d7d8979..2d91b95347 100644
--- a/indra/llcorehttp/tests/test_httprequest.hpp
+++ b/indra/llcorehttp/tests/test_httprequest.hpp
@@ -381,7 +381,7 @@ void HttpRequestTestObjectType::test<5>()
 
 	// Run the notification pump.
 	int count(0);
-	int limit(20);
+	int limit(180);				// With retries, can take more than 10 seconds to give up
 	while (count++ < limit && mHandlerCalls < 1)
 	{
 		req->update(1000);
diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index 92b847345d..4a46ea0e97 100644
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -67,9 +67,53 @@
 // This is an attempt to document what's going on in here after-the-fact.
 // It's a sincere attempt to be accurate but there will be mistakes.
 //
+//
 // Purpose
 //
-// (What is this solving?)
+// What is this module trying to do?  It accepts requests to load textures
+// at a given priority and discard level and notifies the caller when done
+// (successfully or not).  Additional constraints are:
+//
+// * Support a local texture cache.  Don't hit network when possible
+//   to avoid it.
+// * Use UDP or HTTP as directed or as fallback.  HTTP is tried when
+//   not disabled and a URL is available.  UDP when a URL isn't
+//   available or HTTP attempts fail.
+// * Asynchronous (using threads).  Main thread is not to be blocked or
+//   burdened.
+// * High concurrency.  Many requests need to be in-flight and at various
+//   stages of completion.
+// * Tolerate frequent re-prioritizations of requests.  Priority is
+//   a reflection of a camera's viewpoint and as that viewpoint changes,
+//   objects and textures become more and less relevant and that is
+//   expressed at this level by priority changes and request cancelations.
+//
+// The caller interfaces that fall out of the above and shape the
+// implementation are:
+// * createRequest - Load j2c image via UDP or HTTP at given discard level and priority
+// * deleteRequest - Request removal of prior request
+// * getRequestFinished - Test if request is finished returning data to caller
+// * updateRequestPriority - Change priority of existing request
+// * getFetchState - Retrieve progress on existing request
+//
+// Everything else in here is mostly plumbing, metrics and debug.
+//
+//
+// The Work Queue
+//
+// The two central classes are LLTextureFetch and LLTextureFetchWorker.
+// LLTextureFetch combines threading with a priority queue of work
+// requests.  The priority queue is sorted by a U32 priority derived
+// from the F32 priority in the APIs.  The *only* work request that
+// receives service time by this thread is the highest priority
+// request.  All others wait until it is complete or a dynamic priority
+// change has re-ordered work.
+//
+// LLTextureFetchWorker implements the work request and is 1:1 with
+// texture fetch requests.  Embedded in each is a state machine that
+// walks it through the cache, HTTP, UDP, image decode and retry
+// steps of texture acquisition.
+//
 //
 // Threads
 //
@@ -83,6 +127,7 @@
 // 5.  Tid      Image decoder's worker thread
 // 6.  Thl      HTTP library's worker thread
 //
+//
 // Mutexes/Condition Variables
 //
 // 1.  Mt       Mutex defined for LLThread's condition variable (base class of
@@ -98,6 +143,7 @@
 //              LLTextureFetchWorker).  One per request.
 // 7.  Mw       LLTextureFetchWorker's mutex.  One per request.
 //
+//
 // Lock Ordering Rules
 //
 // Not an exhaustive list but shows the order of lock acquisition
@@ -105,6 +151,8 @@
 // acquiring 'B'.
 //
 // 1.    Mw < Mfnq
+// (there are many more...)
+//
 //
 // Method and Member Definitions
 //
@@ -124,7 +172,10 @@
 // comment can mean the member is unlocked or that I didn't bother
 // to do the archaeology.  In the case of LLTextureFetchWorker,
 // most data members added by the leaf class are actually covered
-// by the Mw lock.
+// by the Mw lock.  You may also see "// T<xxx>" which means that
+// the member's usage is restricted to one thread (except for
+// perhaps construction and destruction) and so explicit locking
+// isn't used.
 //
 // In code, a trailing comment like "// [-+]M<xxx>" indicates a
 // lock acquision or release point.
@@ -132,27 +183,54 @@
 //
 // Worker Lifecycle
 //
-// (Can't unilaterally delete, cleanup is two-phase, etc.)
+// The threading and responder model makes it very likely that
+// other components are holding on to a pointer to a worker request.
+// So, uncoordinated deletions of requests is a guarantee of memory
+// corruption in a short time.  So destroying a request involves
+// invocations's of LLQueuedThread/LLWorkerThread's abort/stop
+// logic that removes workers and puts them ona delete queue for
+// 2-phase destruction.  That second phase is deferrable by calls
+// to deleteOK() which only allow final destruction (via dtor)
+// once deleteOK has determined that the request is in a safe
+// state.
+//
 //
 // Worker State Machine
 //
 // (ASCII art needed)
 //
+//
 // Priority Scheme
 //
 // [PRIORITY_LOW, PRIORITY_NORMAL)   - for WAIT_HTTP_RESOURCE state
 // [PRIORITY_NORMAL, PRIORITY_HIGH)  - waiting for external event
-// [PRIORITY_HIGH, PRIORITY_URGENT)  - rapidly transitioning through states,
+// [PRIORITY_HIGH, PRIORITY_URGENT)  - External event delivered,
+//                                     rapidly transitioning through states,
 //                                     no waiting allowed
 //
+// By itself, the above work queue model would fail the concurrency
+// and liveness requirements of the interface.  A high priority
+// request could find itself on the head and stalled for external
+// reasons (see VWR-28996).  So a few additional constraints are
+// required to keep things running:
+// * Anything that can make forward progress must be kept at a
+//   higher priority than anything that can't.
+// * On completion of external events, the associated request
+//   needs to be elevated beyond the normal range to handle
+//   any data delivery and release any external resource.
+//
+// This effort is made to keep higher-priority entities moving
+// forward in their state machines at every possible step of
+// processing.  It's not entirely proven that this produces the
+// experiencial benefits promised.
 //
 
 //////////////////////////////////////////////////////////////////////////////
 
 // Tuning/Parameterization Constants
 
-static const S32 HTTP_REQUESTS_IN_QUEUE_HIGH_WATER = 40;
-static const S32 HTTP_REQUESTS_IN_QUEUE_LOW_WATER = 20;
+static const S32 HTTP_REQUESTS_IN_QUEUE_HIGH_WATER = 40;		// Maximum requests to have active in HTTP
+static const S32 HTTP_REQUESTS_IN_QUEUE_LOW_WATER = 20;			// Active level at which to refill
 
 
 //////////////////////////////////////////////////////////////////////////////
@@ -425,7 +503,6 @@ private:
 	BOOL mInLocalCache;
 	bool mCanUseHTTP ;
 	bool mCanUseNET ; //can get from asset server.
-	S32 mHTTPFailCount;
 	S32 mRetryAttempt;
 	S32 mActiveCount;
 	LLCore::HttpStatus mGetStatus;
@@ -745,7 +822,6 @@ LLTextureFetchWorker::LLTextureFetchWorker(LLTextureFetch* fetcher,
 	  mHaveAllData(FALSE),
 	  mInLocalCache(FALSE),
 	  mCanUseHTTP(true),
-	  mHTTPFailCount(0),
 	  mRetryAttempt(0),
 	  mActiveCount(0),
 	  mWorkMutex(NULL),
@@ -936,6 +1012,9 @@ void LLTextureFetchWorker::startWork(S32 param)
 // Threads:  Ttf
 bool LLTextureFetchWorker::doWork(S32 param)
 {
+	static const LLCore::HttpStatus http_not_found(HTTP_NOT_FOUND);
+	static const LLCore::HttpStatus http_service_unavail(HTTP_SERVICE_UNAVAILABLE);
+	
 	// Release waiters while we aren't holding the Mw lock.
 	mFetcher->releaseHttpWaiters();
 	
@@ -1286,7 +1365,6 @@ bool LLTextureFetchWorker::doWork(S32 param)
 		{
 			llwarns << "HTTP GET request failed for " << mID << llendl;
 			resetFormattedData();
-			++mHTTPFailCount;
 			return true; // failed
 		}
 
@@ -1313,10 +1391,8 @@ bool LLTextureFetchWorker::doWork(S32 param)
 			S32 cur_size = mFormattedImage.notNull() ? mFormattedImage->getDataSize() : 0;
 			if (mRequestedSize < 0)
 			{
-				S32 max_attempts;
-				if (mGetStatus == LLCore::HttpStatus(HTTP_NOT_FOUND, LLCore::HE_REPLY_ERROR))
+				if (http_not_found == mGetStatus)
 				{
-					mHTTPFailCount = max_attempts = 1; // Don't retry
 					llwarns << "Texture missing from server (404): " << mUrl << llendl;
 
 					// roll back to try UDP
@@ -1328,47 +1404,32 @@ bool LLTextureFetchWorker::doWork(S32 param)
 						return false;
 					}
 				}
-				else if (mGetStatus == LLCore::HttpStatus(HTTP_SERVICE_UNAVAILABLE, LLCore::HE_REPLY_ERROR))
+				else if (http_service_unavail == mGetStatus)
 				{
-					// *TODO: Should probably introduce a timer here to delay future HTTP requsts
-					// for a short time (~1s) to ease server load? Ideally the server would queue
-					// requests instead of returning 503... we already limit the number pending.
-					++mHTTPFailCount;
-					max_attempts = mHTTPFailCount+1; // Keep retrying
 					LL_INFOS_ONCE("Texture") << "Texture server busy (503): " << mUrl << LL_ENDL;
 				}
 				else
 				{
-					const S32 HTTP_MAX_RETRY_COUNT = 3;
-					max_attempts = HTTP_MAX_RETRY_COUNT + 1;
-					++mHTTPFailCount;
 					llinfos << "HTTP GET failed for: " << mUrl
 							<< " Status: " << mGetStatus.toHex()
 							<< " Reason: '" << mGetReason << "'"
-							<< " Attempt:" << mHTTPFailCount+1 << "/" << max_attempts << llendl;
+						// *FIXME:  Add retry info for reporting purposes...
+						// << " Attempt:" << mHTTPFailCount+1 << "/" << max_attempts
+							<< llendl;
 				}
 
-				if (mHTTPFailCount >= max_attempts)
-				{
-					if (cur_size > 0)
-					{
-						// Use available data
-						mLoadedDiscard = mFormattedImage->getDiscardLevel();
-						mState = DECODE_IMAGE;
-						return false; 
-					}
-					else
-					{
-						resetFormattedData();
-						mState = DONE;
-						return true; // failed
-					}
-				}
-				else
+				if (cur_size > 0)
 				{
-					mState = SEND_HTTP_REQ;
-					return false; // retry
+					// Use available data
+					mLoadedDiscard = mFormattedImage->getDiscardLevel();
+					mState = DECODE_IMAGE;
+					return false; 
 				}
+
+				// Fail harder
+				resetFormattedData();
+				mState = DONE;
+				return true; // failed
 			}
 			
 			if (! mHttpBufferArray || ! mHttpBufferArray->size())
@@ -1649,7 +1710,7 @@ void LLTextureFetchWorker::onCompleted(LLCore::HttpHandle handle, LLCore::HttpRe
 	}
 	else
 	{
-		static const LLCore::HttpStatus par_status(LLCore::HttpStatus(HTTP_PARTIAL_CONTENT, LLCore::HE_SUCCESS));
+		static const LLCore::HttpStatus par_status(HTTP_PARTIAL_CONTENT);
 
 		partial = (par_status == status);
 	}
diff --git a/indra/newview/lltexturefetch.h b/indra/newview/lltexturefetch.h
index 53b0f7885f..4ee13d171e 100644
--- a/indra/newview/lltexturefetch.h
+++ b/indra/newview/lltexturefetch.h
@@ -308,9 +308,9 @@ private:
 	// Interfaces and objects into the core http library used
 	// to make our HTTP requests.  These replace the various
 	// LLCurl interfaces used in the past.
-	LLCore::HttpRequest *		mHttpRequest;
-	LLCore::HttpOptions *		mHttpOptions;
-	LLCore::HttpHeaders *		mHttpHeaders;
+	LLCore::HttpRequest *		mHttpRequest;							// Ttf
+	LLCore::HttpOptions *		mHttpOptions;							// Ttf
+	LLCore::HttpHeaders *		mHttpHeaders;							// Ttf
 
 	typedef std::set<LLUUID> wait_http_res_queue_t;
 	wait_http_res_queue_t		mHttpWaitResource;						// Mfnq
-- 
cgit v1.3


From d2af82aafc9ef569c9552f269ccf4e4fd38a1f33 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Fri, 6 Jul 2012 19:14:42 -0400
Subject: Experiment with ignoring priority in the library.  Let upper layers
 sort things out or use policy classes (eventually) to arrange low and high
 priority traffic.  Subjectively, I think this works better in practice (as I
 haven't implemented a dynamic priority setter yet).

---
 indra/llcorehttp/_httpinternal.h   |  8 ++++++++
 indra/llcorehttp/_httpreadyqueue.h | 39 +++++++++++++++++++++++++++++++++++---
 2 files changed, 44 insertions(+), 3 deletions(-)

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httpinternal.h b/indra/llcorehttp/_httpinternal.h
index 4ccace2b30..5f966500c9 100644
--- a/indra/llcorehttp/_httpinternal.h
+++ b/indra/llcorehttp/_httpinternal.h
@@ -32,6 +32,14 @@
 // something wrong is probably happening.
 
 
+// If '1', internal ready queues will not order ready
+// requests by priority, instead it's first-come-first-served.
+// Reprioritization requests have the side-effect of then
+// putting the modified request at the back of the ready queue.
+
+#define	LLCORE_READY_QUEUE_IGNORES_PRIORITY		1
+
+
 namespace LLCore
 {
 
diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
index 87828834dc..968ca01258 100644
--- a/indra/llcorehttp/_httpreadyqueue.h
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -30,6 +30,7 @@
 
 #include <queue>
 
+#include "_httpinternal.h"
 #include "_httpoprequest.h"
 
 
@@ -47,10 +48,18 @@ namespace LLCore
 /// Threading:  not thread-safe.  Expected to be used entirely by
 /// a single thread, typically a worker thread of some sort.
 
+#if LLCORE_READY_QUEUE_IGNORES_PRIORITY
+
+typedef std::deque<HttpOpRequest *> HttpReadyQueueBase;
+
+#else
+
 typedef std::priority_queue<HttpOpRequest *,
 							std::deque<HttpOpRequest *>,
 							LLCore::HttpOpRequestCompare> HttpReadyQueueBase;
 
+#endif // LLCORE_READY_QUEUE_IGNORES_PRIORITY
+
 class HttpReadyQueue : public HttpReadyQueueBase
 {
 public:
@@ -66,16 +75,40 @@ protected:
 	void operator=(const HttpReadyQueue &);		// Not defined
 
 public:
+
+#if LLCORE_READY_QUEUE_IGNORES_PRIORITY
+	// Types and methods needed to make a std::deque look
+	// more like a std::priority_queue, at least for our
+	// purposes.
+	typedef HttpReadyQueueBase container_type;
+	
+	const_reference & top() const
+		{
+			return front();
+		}
+
+	void pop()
+		{
+			pop_front();
+		}
+
+	void push(const value_type & v)
+		{
+			push_back(v);
+		}
+	
+#endif // LLCORE_READY_QUEUE_IGNORES_PRIORITY
+	
 	const container_type & get_container() const
 		{
-			return c;
+			return *this;
 		}
 
 	container_type & get_container()
 		{
-			return c;
+			return *this;
 		}
-
+	
 }; // end class HttpReadyQueue
 
 
-- 
cgit v1.3


From 70e976d1a87f4225c7593129a9c1c4732e2d38e4 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Fri, 6 Jul 2012 19:17:23 -0400
Subject: Odd that this was accepted by VS2010.  It clearly wasn't right.

---
 indra/llcorehttp/_httpreadyqueue.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
index 968ca01258..8462b174b5 100644
--- a/indra/llcorehttp/_httpreadyqueue.h
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -82,7 +82,7 @@ public:
 	// purposes.
 	typedef HttpReadyQueueBase container_type;
 	
-	const_reference & top() const
+	const_reference top() const
 		{
 			return front();
 		}
-- 
cgit v1.3


From 334ce2556f0d51c38a76d655084ae1d4671f6aec Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Mon, 23 Jul 2012 17:00:11 -0400
Subject: Cleaning up comments, names, miscellany.

---
 indra/llcorehttp/_httpinternal.h   | 4 ++--
 indra/llcorehttp/_httpreadyqueue.h | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httpinternal.h b/indra/llcorehttp/_httpinternal.h
index 5f966500c9..97ec5ee1d6 100644
--- a/indra/llcorehttp/_httpinternal.h
+++ b/indra/llcorehttp/_httpinternal.h
@@ -1,5 +1,5 @@
 /**
- * @file httpinternal.h
+ * @file _httpinternal.h
  * @brief Implementation constants and magic numbers
  *
  * $LicenseInfo:firstyear=2012&license=viewerlgpl$
@@ -44,7 +44,7 @@ namespace LLCore
 {
 
 // Maxium number of policy classes that can be defined.
-// *FIXME:  Currently limited to the default class, extend.
+// *TODO:  Currently limited to the default class, extend.
 const int POLICY_CLASS_LIMIT = 1;
 
 // Debug/informational tracing.  Used both
diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
index 8462b174b5..9cf4b059a1 100644
--- a/indra/llcorehttp/_httpreadyqueue.h
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -45,6 +45,12 @@ namespace LLCore
 /// important of those rules is that any iterator becomes invalid
 /// on element erasure.  So pay attention.
 ///
+/// If LLCORE_READY_QUEUE_IGNORES_PRIORITY tests true, the class
+/// implements a std::priority_queue interface but on std::deque
+/// behavior to eliminate sensitivity to priority.  In the future,
+/// this will likely become the only behavior or it may become
+/// a run-time election.
+///
 /// Threading:  not thread-safe.  Expected to be used entirely by
 /// a single thread, typically a worker thread of some sort.
 
-- 
cgit v1.3


From 85e69b043b098dbe5a09f2eac6ff541123089f13 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Mon, 23 Jul 2012 23:40:07 +0000
Subject: Big comment and naming cleanup.  Ready for prime-time. Add to-do list
 to _httpinternal.h to guide anyone who wants to pitch in and help.

---
 indra/llcorehttp/_httpinternal.h       | 93 ++++++++++++++++++++++++++--------
 indra/llcorehttp/_httplibcurl.cpp      | 13 ++---
 indra/llcorehttp/_httplibcurl.h        | 27 ++++++++--
 indra/llcorehttp/_httpopcancel.cpp     |  7 ---
 indra/llcorehttp/_httpopcancel.h       |  5 +-
 indra/llcorehttp/_httpoperation.cpp    |  8 +--
 indra/llcorehttp/_httpoperation.h      | 77 +++++++++++++++++++++++++---
 indra/llcorehttp/_httpoprequest.cpp    | 47 +++++++----------
 indra/llcorehttp/_httpoprequest.h      | 26 ++++++++--
 indra/llcorehttp/_httpopsetget.cpp     |  8 ---
 indra/llcorehttp/_httpopsetget.h       |  3 ++
 indra/llcorehttp/_httppolicy.cpp       | 12 +++--
 indra/llcorehttp/_httppolicy.h         | 39 ++++++++++++--
 indra/llcorehttp/_httppolicyclass.cpp  |  8 +--
 indra/llcorehttp/_httppolicyglobal.cpp |  8 +--
 indra/llcorehttp/_httpreadyqueue.h     | 10 ++--
 indra/llcorehttp/_httpservice.cpp      | 24 ++++-----
 indra/llcorehttp/_httpservice.h        |  6 +--
 indra/llcorehttp/httpcommon.h          | 18 +++++--
 indra/llcorehttp/httpoptions.cpp       |  6 +--
 20 files changed, 313 insertions(+), 132 deletions(-)

(limited to 'indra/llcorehttp/_httpreadyqueue.h')

diff --git a/indra/llcorehttp/_httpinternal.h b/indra/llcorehttp/_httpinternal.h
index 97ec5ee1d6..465e2036b3 100644
--- a/indra/llcorehttp/_httpinternal.h
+++ b/indra/llcorehttp/_httpinternal.h
@@ -32,12 +32,65 @@
 // something wrong is probably happening.
 
 
+// --------------------------------------------------------------------
+// General library to-do list
+//
+// - Implement policy classes.  Structure is mostly there just didn't
+//   need it for the first consumer.
+// - Consider Removing 'priority' from the request interface.  Its use
+//   in an always active class can lead to starvation of low-priority
+//   requests.  Requires coodination of priority values across all
+//   components that share a class.  Changing priority across threads
+//   is slightly expensive (relative to gain) and hasn't been completely
+//   implemented.  And the major user of priority, texture fetches,
+//   may not really need it.
+// - Set/get for global policy and policy classes is clumsy.  Rework
+//   it heading in a direction that allows for more dynamic behavior.
+// - Move HttpOpRequest::prepareRequest() to HttpLibcurl for the
+//   pedantic.
+// - Update downloader and other long-duration services are going to
+//   need a progress notification.  Initial idea is to introduce a
+//   'repeating request' which can piggyback on another request and
+//   persist until canceled or carrier completes.  Current queue
+//   structures allow an HttpOperation object to be enqueued
+//   repeatedly, so...
+// - Investigate making c-ares' re-implementation of a resolver library
+//   more resilient or more intelligent on Mac.  Part of the DNS failure
+//   lies in here.  The mechanism also looks a little less dynamic
+//   than needed in an environments where networking is changing.
+// - Global optimizations:  'borrowing' connections from other classes,
+//   HTTP pipelining.
+// - Dynamic/control system stuff:  detect problems and self-adjust.
+//   This won't help in the face of the router problems we've looked
+//   at, however.  Detect starvation due to UDP activity and provide
+//   feedback to it.
+//
+// Integration to-do list
+// - LLTextureFetch still needs a major refactor.  The use of
+//   LLQueuedThread makes it hard to inspect workers and do the
+//   resource waiting we're now doing.  Rebuild along simpler lines
+//   some of which are suggested in new commentary at the top of
+//   the main source file.
+// - Expand areas of usage eventually leading to the removal of LLCurl.
+//   Rough order of expansion:
+//   .  Mesh fetch
+//   .  Avatar names
+//   .  Group membership lists
+//   .  Caps access in general
+//   .  'The rest'
+// - Adapt texture cache, image decode and other image consumers to
+//   the BufferArray model to reduce data copying.  Alternatively,
+//   adapt this library to something else.
+//
+// --------------------------------------------------------------------
+
+
 // If '1', internal ready queues will not order ready
 // requests by priority, instead it's first-come-first-served.
 // Reprioritization requests have the side-effect of then
 // putting the modified request at the back of the ready queue.
 
-#define	LLCORE_READY_QUEUE_IGNORES_PRIORITY		1
+#define	LLCORE_HTTP_READY_QUEUE_IGNORES_PRIORITY		1
 
 
 namespace LLCore
@@ -45,48 +98,48 @@ namespace LLCore
 
 // Maxium number of policy classes that can be defined.
 // *TODO:  Currently limited to the default class, extend.
-const int POLICY_CLASS_LIMIT = 1;
+const int HTTP_POLICY_CLASS_LIMIT = 1;
 
 // Debug/informational tracing.  Used both
 // as a global option and in per-request traces.
-const int TRACE_OFF = 0;
-const int TRACE_LOW = 1;
-const int TRACE_CURL_HEADERS = 2;
-const int TRACE_CURL_BODIES = 3;
+const int HTTP_TRACE_OFF = 0;
+const int HTTP_TRACE_LOW = 1;
+const int HTTP_TRACE_CURL_HEADERS = 2;
+const int HTTP_TRACE_CURL_BODIES = 3;
 
-const int TRACE_MIN = TRACE_OFF;
-const int TRACE_MAX = TRACE_CURL_BODIES;
+const int HTTP_TRACE_MIN = HTTP_TRACE_OFF;
+const int HTTP_TRACE_MAX = HTTP_TRACE_CURL_BODIES;
 
 // Request retry limits
-const int DEFAULT_RETRY_COUNT = 5;
-const int LIMIT_RETRY_MIN = 0;
-const int LIMIT_RETRY_MAX = 100;
+const int HTTP_RETRY_COUNT_DEFAULT = 5;
+const int HTTP_RETRY_COUNT_MIN = 0;
+const int HTTP_RETRY_COUNT_MAX = 100;
 
-const int DEFAULT_HTTP_REDIRECTS = 10;
+const int HTTP_REDIRECTS_DEFAULT = 10;
 
 // Timeout value used for both connect and protocol exchange.
 // Retries and time-on-queue are not included and aren't
 // accounted for.
-const long DEFAULT_TIMEOUT = 30L;
-const long LIMIT_TIMEOUT_MIN = 0L;
-const long LIMIT_TIMEOUT_MAX = 3600L;
+const long HTTP_REQUEST_TIMEOUT_DEFAULT = 30L;
+const long HTTP_REQUEST_TIMEOUT_MIN = 0L;
+const long HTTP_REQUEST_TIMEOUT_MAX = 3600L;
 
 // Limits on connection counts
-const int DEFAULT_CONNECTIONS = 8;
-const int LIMIT_CONNECTIONS_MIN = 1;
-const int LIMIT_CONNECTIONS_MAX = 256;
+const int HTTP_CONNECTION_LIMIT_DEFAULT = 8;
+const int HTTP_CONNECTION_LIMIT_MIN = 1;
+const int HTTP_CONNECTION_LIMIT_MAX = 256;
 
 // Tuning parameters
 
 // Time worker thread sleeps after a pass through the
 // request, ready and active queues.
-const int LOOP_SLEEP_NORMAL_MS = 2;
+const int HTTP_SERVICE_LOOP_SLEEP_NORMAL_MS = 2;
 
 // Block allocation size (a tuning parameter) is found
 // in bufferarray.h.
 
 // Compatibility controls
-const bool ENABLE_LINKSYS_WRT54G_V5_DNS_FIX = true;
+const bool HTTP_ENABLE_LINKSYS_WRT54G_V5_DNS_FIX = true;
 
 }  // end namespace LLCore
 
diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp
index e031efbc91..4e2e3f0e0e 100644
--- a/indra/llcorehttp/_httplibcurl.cpp
+++ b/indra/llcorehttp/_httplibcurl.cpp
@@ -85,7 +85,7 @@ void HttpLibcurl::shutdown()
 
 void HttpLibcurl::start(int policy_count)
 {
-	llassert_always(policy_count <= POLICY_CLASS_LIMIT);
+	llassert_always(policy_count <= HTTP_POLICY_CLASS_LIMIT);
 	llassert_always(! mMultiHandles);					// One-time call only
 	
 	mPolicyCount = policy_count;
@@ -156,6 +156,7 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport()
 }
 
 
+// Caller has provided us with a ref count on op.
 void HttpLibcurl::addOp(HttpOpRequest * op)
 {
 	llassert_always(op->mReqPolicy < mPolicyCount);
@@ -165,7 +166,7 @@ void HttpLibcurl::addOp(HttpOpRequest * op)
 	if (! op->prepareRequest(mService))
 	{
 		// Couldn't issue request, fail with notification
-		// *FIXME:  Need failure path
+		// *TODO:  Need failure path
 		return;
 	}
 
@@ -173,7 +174,7 @@ void HttpLibcurl::addOp(HttpOpRequest * op)
 	curl_multi_add_handle(mMultiHandles[op->mReqPolicy], op->mCurlHandle);
 	op->mCurlActive = true;
 	
-	if (op->mTracing > TRACE_OFF)
+	if (op->mTracing > HTTP_TRACE_OFF)
 	{
 		HttpPolicy & policy(mService->getPolicy());
 		
@@ -215,7 +216,7 @@ bool HttpLibcurl::cancel(HttpHandle handle)
 
 // *NOTE:  cancelRequest logic parallels completeRequest logic.
 // Keep them synchronized as necessary.  Caller is expected to
-// remove to op from the active list and release the op *after*
+// remove the op from the active list and release the op *after*
 // calling this method.  It must be called first to deliver the
 // op to the reply queue with refcount intact.
 void HttpLibcurl::cancelRequest(HttpOpRequest * op)
@@ -229,7 +230,7 @@ void HttpLibcurl::cancelRequest(HttpOpRequest * op)
 	op->mCurlHandle = NULL;
 
 	// Tracing
-	if (op->mTracing > TRACE_OFF)
+	if (op->mTracing > HTTP_TRACE_OFF)
 	{
 		LL_INFOS("CoreHttp") << "TRACE, RequestCanceled, Handle:  "
 							 << static_cast<HttpHandle>(op)
@@ -305,7 +306,7 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode
 	op->mCurlHandle = NULL;
 
 	// Tracing
-	if (op->mTracing > TRACE_OFF)
+	if (op->mTracing > HTTP_TRACE_OFF)
 	{
 		LL_INFOS("CoreHttp") << "TRACE, RequestComplete, Handle:  "
 							 << static_cast<HttpHandle>(op)
diff --git a/indra/llcorehttp/_httplibcurl.h b/indra/llcorehttp/_httplibcurl.h
index 53972b1ffa..611f029ef5 100644
--- a/indra/llcorehttp/_httplibcurl.h
+++ b/indra/llcorehttp/_httplibcurl.h
@@ -68,24 +68,41 @@ public:
 	/// Give cycles to libcurl to run active requests.  Completed
 	/// operations (successful or failed) will be retried or handed
 	/// over to the reply queue as final responses.
+	///
+	/// @return			Indication of how long this method is
+	///					willing to wait for next service call.
 	HttpService::ELoopSpeed processTransport();
 
 	/// Add request to the active list.  Caller is expected to have
-	/// provided us with a reference count to hold the request.  (No
-	/// additional references will be added.)
+	/// provided us with a reference count on the op to hold the
+	/// request.  (No additional references will be added.)
 	void addOp(HttpOpRequest * op);
 
 	/// One-time call to set the number of policy classes to be
 	/// serviced and to create the resources for each.  Value
 	/// must agree with HttpPolicy::setPolicies() call.
 	void start(int policy_count);
-	
+
+	/// Synchronously stop libcurl operations.  All active requests
+	/// are canceled and removed from libcurl's handling.  Easy
+	/// handles are detached from their multi handles and released.
+	/// Multi handles are also released.  Canceled requests are
+	/// completed with canceled status and made available on their
+	/// respective reply queues.
+	///
+	/// Can be restarted with a start() call.
 	void shutdown();
-	
+
+	/// Return global and per-class counts of active requests.
 	int getActiveCount() const;
 	int getActiveCountInClass(int policy_class) const;
 
-	// Shadows HttpService's method
+	/// Attempt to cancel a request identified by handle.
+	///
+	/// Interface shadows HttpService's method.
+	///
+	/// @return			True if handle was found and operation canceled.
+	///
 	bool cancel(HttpHandle handle);
 
 protected:
diff --git a/indra/llcorehttp/_httpopcancel.cpp b/indra/llcorehttp/_httpopcancel.cpp
index 8e1105dc81..c1912eb3db 100644
--- a/indra/llcorehttp/_httpopcancel.cpp
+++ b/indra/llcorehttp/_httpopcancel.cpp
@@ -26,18 +26,11 @@
 
 #include "_httpopcancel.h"
 
-#include <cstdio>
-#include <algorithm>
-
 #include "httpcommon.h"
 #include "httphandler.h"
 #include "httpresponse.h"
 
-#include "_httprequestqueue.h"
-#include "_httpreplyqueue.h"
 #include "_httpservice.h"
-#include "_httppolicy.h"
-#include "_httplibcurl.h"
 
 
 namespace LLCore
diff --git a/indra/llcorehttp/_httpopcancel.h b/indra/llcorehttp/_httpopcancel.h
index 659d28955f..336dfdc573 100644
--- a/indra/llcorehttp/_httpopcancel.h
+++ b/indra/llcorehttp/_httpopcancel.h
@@ -46,11 +46,14 @@ namespace LLCore
 /// be canceled, if possible.  This includes active requests
 /// that may be in the middle of an HTTP transaction.  Any
 /// completed request will not be canceled and will return
-/// its final status unchanged.
+/// its final status unchanged and *this* request will complete
+/// with an HE_HANDLE_NOT_FOUND error status.
 
 class HttpOpCancel : public HttpOperation
 {
 public:
+	/// @param	handle	Handle of previously-issued request to
+	///					be canceled.
 	HttpOpCancel(HttpHandle handle);
 
 protected:
diff --git a/indra/llcorehttp/_httpoperation.cpp b/indra/llcorehttp/_httpoperation.cpp
index 910dbf1f2f..5cf5bc5930 100644
--- a/indra/llcorehttp/_httpoperation.cpp
+++ b/indra/llcorehttp/_httpoperation.cpp
@@ -94,7 +94,7 @@ void HttpOperation::stageFromRequest(HttpService *)
 	// Default implementation should never be called.  This
 	// indicates an operation making a transition that isn't
 	// defined.
-	LL_ERRS("HttpCore") << "Default stateFromRequest method may not be called."
+	LL_ERRS("HttpCore") << "Default stageFromRequest method may not be called."
 						<< LL_ENDL;
 }
 
@@ -104,7 +104,7 @@ void HttpOperation::stageFromReady(HttpService *)
 	// Default implementation should never be called.  This
 	// indicates an operation making a transition that isn't
 	// defined.
-	LL_ERRS("HttpCore") << "Default stateFromReady method may not be called."
+	LL_ERRS("HttpCore") << "Default stageFromReady method may not be called."
 						<< LL_ENDL;
 }
 
@@ -114,7 +114,7 @@ void HttpOperation::stageFromActive(HttpService *)
 	// Default implementation should never be called.  This
 	// indicates an operation making a transition that isn't
 	// defined.
-	LL_ERRS("HttpCore") << "Default stateFromActive method may not be called."
+	LL_ERRS("HttpCore") << "Default stageFromActive method may not be called."
 						<< LL_ENDL;
 }
 
@@ -143,7 +143,7 @@ HttpStatus HttpOperation::cancel()
 
 void HttpOperation::addAsReply()
 {
-	if (mTracing > TRACE_OFF)
+	if (mTracing > HTTP_TRACE_OFF)
 	{
 		LL_INFOS("CoreHttp") << "TRACE, ToReplyQueue, Handle:  "
 							 << static_cast<HttpHandle>(this)
diff --git a/indra/llcorehttp/_httpoperation.h b/indra/llcorehttp/_httpoperation.h
index 717a9b0d72..914627fad0 100644
--- a/indra/llcorehttp/_httpoperation.h
+++ b/indra/llcorehttp/_httpoperation.h
@@ -72,9 +72,11 @@ class HttpService;
 class HttpOperation : public LLCoreInt::RefCounted
 {
 public:
+	/// Threading:  called by a consumer/application thread.
 	HttpOperation();
 
 protected:
+	/// Threading:  called by any thread.
 	virtual ~HttpOperation();							// Use release()
 
 private:
@@ -82,28 +84,87 @@ private:
 	void operator=(const HttpOperation &);				// Not defined
 
 public:
+	/// Register a reply queue and a handler for completion notifications.
+	///
+	/// Invokers of operations that want to receive notification that an
+	/// operation has been completed do so by binding a reply queue and
+	/// a handler object to the request.
+	///
+	/// @param	reply_queue		Pointer to the reply queue where completion
+	///							notifications are to be queued (typically
+	///							by addAsReply()).  This will typically be
+	///							the reply queue referenced by the request
+	///							object.  This method will increment the
+	///							refcount on the queue holding the queue
+	///							until delivery is complete.  Using a reply_queue
+	///							even if the handler is NULL has some benefits
+	///							for memory deallocation by keeping it in the
+	///							originating thread.
+	///
+	/// @param	handler			Possibly NULL pointer to a non-refcounted
+	////						handler object to be invoked (onCompleted)
+	///							when the operation is finished.  Note that
+	///							the handler object is never dereferenced
+	///							by the worker thread.  This is passible data
+	///							until notification is performed.
+	///
+	/// Threading:  called by application thread.
+	///
 	void setReplyPath(HttpReplyQueue * reply_queue,
 					  HttpHandler * handler);
 
-	HttpHandler * getUserHandler() const
-		{
-			return mUserHandler;
-		}
-	
+	/// The three possible staging steps in an operation's lifecycle.
+	/// Asynchronous requests like HTTP operations move from the
+	/// request queue to the ready queue via stageFromRequest.  Then
+	/// from the ready queue to the active queue by stageFromReady.  And
+	/// when complete, to the reply queue via stageFromActive and the
+	/// addAsReply utility.
+	///
+	/// Immediate mode operations (everything else) move from the
+	/// request queue to the reply queue directly via stageFromRequest
+	/// and addAsReply with no existence on the ready or active queues.
+	///
+	/// These methods will take out a reference count on the request,
+	/// caller only needs to dispose of its reference when done with
+	/// the request. 
+	///
+	/// Threading:  called by worker thread.
+	///
 	virtual void stageFromRequest(HttpService *);
 	virtual void stageFromReady(HttpService *);
 	virtual void stageFromActive(HttpService *);
 
+	/// Delivers a notification to a handler object on completion.
+	///
+	/// Once a request is complete and it has been removed from its
+	/// reply queue, a handler notification may be delivered by a
+	/// call to HttpRequest::update().  This method does the necessary
+	/// dispatching.
+	///
+	/// Threading:  called by application thread.
+	///
 	virtual void visitNotifier(HttpRequest *);
-	
+
+	/// Cancels the operation whether queued or active.
+	/// Final status of the request becomes canceled (an error) and
+	/// that will be delivered to caller via notification scheme.
+	///
+	/// Threading:  called by worker thread.
+	///
 	virtual HttpStatus cancel();
 	
 protected:
+	/// Delivers request to reply queue on completion.  After this
+	/// call, worker thread no longer accesses the object and it
+	/// is owned by the reply queue.
+	///
+	/// Threading:  called by worker thread.
+	///
 	void addAsReply();
 	
 protected:
 	HttpReplyQueue *			mReplyQueue;			// Have refcount
-	HttpHandler *				mUserHandler;
+	HttpHandler *				mUserHandler;			// Naked pointer
 
 public:
 	// Request Data
@@ -172,7 +233,7 @@ public:
 
 /// HttpOpSpin is a test-only request that puts the worker
 /// thread into a cpu spin.  Used for unit tests and cleanup
-/// evaluation.  You do not want to use this.
+/// evaluation.  You do not want to use this in production.
 class HttpOpSpin : public HttpOperation
 {
 public:
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index a18a164f0d..7db19b1841 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -111,10 +111,10 @@ HttpOpRequest::HttpOpRequest()
 	  mReplyHeaders(NULL),
 	  mPolicyRetries(0),
 	  mPolicyRetryAt(HttpTime(0)),
-	  mPolicyRetryLimit(DEFAULT_RETRY_COUNT)
+	  mPolicyRetryLimit(HTTP_RETRY_COUNT_DEFAULT)
 {
 	// *NOTE:  As members are added, retry initialization/cleanup
-	// may need to be extended in @prepareRequest().
+	// may need to be extended in @see prepareRequest().
 }
 
 
@@ -153,9 +153,6 @@ HttpOpRequest::~HttpOpRequest()
 		mCurlHeaders = NULL;
 	}
 
-	mReplyOffset = 0;
-	mReplyLength = 0;
-	mReplyFullLength = 0;
 	if (mReplyBody)
 	{
 		mReplyBody->release();
@@ -215,8 +212,6 @@ void HttpOpRequest::stageFromActive(HttpService * service)
 
 void HttpOpRequest::visitNotifier(HttpRequest * request)
 {
-	static const HttpStatus partial_content(HTTP_PARTIAL_CONTENT, HE_SUCCESS);
-	
 	if (mUserHandler)
 	{
 		HttpResponse * response = new HttpResponse();
@@ -339,8 +334,8 @@ void HttpOpRequest::setupCommon(HttpRequest::policy_t policy_id,
 			mProcFlags |= PF_SAVE_HEADERS;
 		}
 		mPolicyRetryLimit = options->getRetries();
-		mPolicyRetryLimit = llclamp(mPolicyRetryLimit, LIMIT_RETRY_MIN, LIMIT_RETRY_MAX);
-		mTracing = (std::max)(mTracing, llclamp(options->getTrace(), TRACE_MIN, TRACE_MAX));
+		mPolicyRetryLimit = llclamp(mPolicyRetryLimit, HTTP_RETRY_COUNT_MIN, HTTP_RETRY_COUNT_MAX);
+		mTracing = (std::max)(mTracing, llclamp(options->getTrace(), HTTP_TRACE_MIN, HTTP_TRACE_MAX));
 	}
 }
 
@@ -394,7 +389,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	curl_easy_setopt(mCurlHandle, CURLOPT_PRIVATE, this);
 	curl_easy_setopt(mCurlHandle, CURLOPT_ENCODING, "");
 
-	if (ENABLE_LINKSYS_WRT54G_V5_DNS_FIX)
+	if (HTTP_ENABLE_LINKSYS_WRT54G_V5_DNS_FIX)
 	{
 		// The Linksys WRT54G V5 router has an issue with frequent
 		// DNS lookups from LAN machines.  If they happen too often,
@@ -402,7 +397,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 		// about 700 or so requests and starts issuing TCP RSTs to
 		// new connections.  Reuse the DNS lookups for even a few
 		// seconds and no RSTs.
-		curl_easy_setopt(mCurlHandle, CURLOPT_DNS_CACHE_TIMEOUT, 10);
+		curl_easy_setopt(mCurlHandle, CURLOPT_DNS_CACHE_TIMEOUT, 15);
 	}
 	else
 	{
@@ -414,7 +409,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	}
 	curl_easy_setopt(mCurlHandle, CURLOPT_AUTOREFERER, 1);
 	curl_easy_setopt(mCurlHandle, CURLOPT_FOLLOWLOCATION, 1);
-	curl_easy_setopt(mCurlHandle, CURLOPT_MAXREDIRS, DEFAULT_HTTP_REDIRECTS);
+	curl_easy_setopt(mCurlHandle, CURLOPT_MAXREDIRS, HTTP_REDIRECTS_DEFAULT);
 	curl_easy_setopt(mCurlHandle, CURLOPT_WRITEFUNCTION, writeCallback);
 	curl_easy_setopt(mCurlHandle, CURLOPT_WRITEDATA, this);
 	curl_easy_setopt(mCurlHandle, CURLOPT_READFUNCTION, readCallback);
@@ -434,7 +429,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	}
 	else if (policy.get(HttpRequest::GP_HTTP_PROXY, &opt_value))
 	{
-		// *TODO:  This is fine for now but get fuller socks/
+		// *TODO:  This is fine for now but get fuller socks5/
 		// authentication thing going later....
 		curl_easy_setopt(mCurlHandle, CURLOPT_PROXY, opt_value->c_str());
 		curl_easy_setopt(mCurlHandle, CURLOPT_PROXYTYPE, CURLPROXY_HTTP);
@@ -497,7 +492,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	}
 
 	// Tracing
-	if (mTracing >= TRACE_CURL_HEADERS)
+	if (mTracing >= HTTP_TRACE_CURL_HEADERS)
 	{
 		curl_easy_setopt(mCurlHandle, CURLOPT_VERBOSE, 1);
 		curl_easy_setopt(mCurlHandle, CURLOPT_DEBUGDATA, this);
@@ -528,11 +523,11 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	mCurlHeaders = curl_slist_append(mCurlHeaders, "Pragma:");
 
 	// Request options
-	long timeout(DEFAULT_TIMEOUT);
+	long timeout(HTTP_REQUEST_TIMEOUT_DEFAULT);
 	if (mReqOptions)
 	{
 		timeout = mReqOptions->getTimeout();
-		timeout = llclamp(timeout, LIMIT_TIMEOUT_MIN, LIMIT_TIMEOUT_MAX);
+		timeout = llclamp(timeout, HTTP_REQUEST_TIMEOUT_MIN, HTTP_REQUEST_TIMEOUT_MAX);
 	}
 	curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, timeout);
 	curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, timeout);
@@ -605,12 +600,6 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi
 	static const char con_ran_line[] = "content-range:";
 	static const size_t con_ran_line_len = sizeof(con_ran_line) - 1;
 
-	static const char con_type_line[] = "content-type:";
-	static const size_t con_type_line_len = sizeof(con_type_line) - 1;
-	
-	static const char con_enc_line[] = "content-encoding:";
-	static const size_t con_enc_line_len = sizeof(con_enc_line) - 1;
-	
 	HttpOpRequest * op(static_cast<HttpOpRequest *>(userdata));
 
 	const size_t hdr_size(size * nmemb);
@@ -705,7 +694,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
 	switch (info)
 	{
 	case CURLINFO_TEXT:
-		if (op->mTracing >= TRACE_CURL_HEADERS)
+		if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
 		{
 			tag = "TEXT";
 			escape_libcurl_debug_data(buffer, len, true, safe_line);
@@ -714,7 +703,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
 		break;
 			
 	case CURLINFO_HEADER_IN:
-		if (op->mTracing >= TRACE_CURL_HEADERS)
+		if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
 		{
 			tag = "HEADERIN";
 			escape_libcurl_debug_data(buffer, len, true, safe_line);
@@ -723,7 +712,7 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
 		break;
 			
 	case CURLINFO_HEADER_OUT:
-		if (op->mTracing >= TRACE_CURL_HEADERS)
+		if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
 		{
 			tag = "HEADEROUT";
 			escape_libcurl_debug_data(buffer, 2 * len, true, safe_line);		// Goes out as one line
@@ -732,11 +721,11 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
 		break;
 			
 	case CURLINFO_DATA_IN:
-		if (op->mTracing >= TRACE_CURL_HEADERS)
+		if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
 		{
 			tag = "DATAIN";
 			logit = true;
-			if (op->mTracing >= TRACE_CURL_BODIES)
+			if (op->mTracing >= HTTP_TRACE_CURL_BODIES)
 			{
 				escape_libcurl_debug_data(buffer, len, false, safe_line);
 			}
@@ -750,11 +739,11 @@ int HttpOpRequest::debugCallback(CURL * handle, curl_infotype info, char * buffe
 		break;
 			
 	case CURLINFO_DATA_OUT:
-		if (op->mTracing >= TRACE_CURL_HEADERS)
+		if (op->mTracing >= HTTP_TRACE_CURL_HEADERS)
 		{
 			tag = "DATAOUT";
 			logit = true;
-			if (op->mTracing >= TRACE_CURL_BODIES)
+			if (op->mTracing >= HTTP_TRACE_CURL_BODIES)
 			{
 				escape_libcurl_debug_data(buffer, len, false, safe_line);
 			}
diff --git a/indra/llcorehttp/_httpoprequest.h b/indra/llcorehttp/_httpoprequest.h
index 36dc5dc876..7b65d17783 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -88,7 +88,14 @@ public:
 	virtual void visitNotifier(HttpRequest * request);
 			
 public:
-	// Setup Methods
+	/// Setup Methods
+	///
+	/// Basically an RPC setup for each type of HTTP method
+	/// invocation with one per method type.  These are
+	/// generally invoked right after construction.
+	///
+	/// Threading:  called by application thread
+	///
 	HttpStatus setupGet(HttpRequest::policy_t policy_id,
 						HttpRequest::priority_t priority,
 						const std::string & url,
@@ -116,19 +123,32 @@ public:
 						BufferArray * body,
 						HttpOptions * options,
 						HttpHeaders * headers);
-	
+
+	// Internal method used to setup the libcurl options for a request.
+	// Does all the libcurl handle setup in one place.
+	//
+	// Threading:  called by worker thread
+	//
 	HttpStatus prepareRequest(HttpService * service);
 	
 	virtual HttpStatus cancel();
 
 protected:
+	// Common setup for all the request methods.
+	//
+	// Threading:  called by application thread
+	//
 	void setupCommon(HttpRequest::policy_t policy_id,
 					 HttpRequest::priority_t priority,
 					 const std::string & url,
 					 BufferArray * body,
 					 HttpOptions * options,
 					 HttpHeaders * headers);
-	
+
+	// libcurl operational callbacks
+	//
+	// Threading:  called by worker thread
+	//
 	static size_t writeCallback(void * data, size_t size, size_t nmemb, void * userdata);
 	static size_t readCallback(void * data, size_t size, size_t nmemb, void * userdata);
 	static size_t headerCallback(void * data, size_t size, size_t nmemb, void * userdata);
diff --git a/indra/llcorehttp/_httpopsetget.cpp b/indra/llcorehttp/_httpopsetget.cpp
index c1357f9ae5..8198528a9b 100644
--- a/indra/llcorehttp/_httpopsetget.cpp
+++ b/indra/llcorehttp/_httpopsetget.cpp
@@ -26,18 +26,10 @@
 
 #include "_httpopsetget.h"
 
-#include <cstdio>
-#include <algorithm>
-
 #include "httpcommon.h"
-#include "httphandler.h"
-#include "httpresponse.h"
 
-#include "_httprequestqueue.h"
-#include "_httpreplyqueue.h"
 #include "_httpservice.h"
 #include "_httppolicy.h"
-#include "_httplibcurl.h"
 
 
 namespace LLCore
diff --git a/indra/llcorehttp/_httpopsetget.h b/indra/llcorehttp/_httpopsetget.h
index efb24855c5..6966b9d94e 100644
--- a/indra/llcorehttp/_httpopsetget.h
+++ b/indra/llcorehttp/_httpopsetget.h
@@ -44,6 +44,8 @@ namespace LLCore
 
 /// HttpOpSetGet requests dynamic changes to policy and
 /// configuration settings.
+///
+/// *NOTE:  Expect this to change.  Don't really like it yet.
 
 class HttpOpSetGet : public HttpOperation
 {
@@ -58,6 +60,7 @@ private:
 	void operator=(const HttpOpSetGet &);				// Not defined
 
 public:
+	/// Threading:  called by application thread
 	void setupGet(HttpRequest::EGlobalPolicy setting);
 	void setupSet(HttpRequest::EGlobalPolicy setting, const std::string & value);
 
diff --git a/indra/llcorehttp/_httppolicy.cpp b/indra/llcorehttp/_httppolicy.cpp
index 1e64924198..c7a69ad133 100644
--- a/indra/llcorehttp/_httppolicy.cpp
+++ b/indra/llcorehttp/_httppolicy.cpp
@@ -40,12 +40,17 @@ namespace LLCore
 {
 
 
+// Per-policy-class data for a running system.
+// Collection of queues, parameters, history, metrics, etc.
+// for a single policy class.
+//
+// Threading:  accessed only by worker thread
 struct HttpPolicy::State
 {
 public:
 	State()
-		: mConnMax(DEFAULT_CONNECTIONS),
-		  mConnAt(DEFAULT_CONNECTIONS),
+		: mConnMax(HTTP_CONNECTION_LIMIT_DEFAULT),
+		  mConnAt(HTTP_CONNECTION_LIMIT_DEFAULT),
 		  mConnMin(1),
 		  mNextSample(0),
 		  mErrorCount(0),
@@ -298,6 +303,7 @@ bool HttpPolicy::cancel(HttpHandle handle)
 	return false;
 }
 
+
 bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
 {
 	static const HttpStatus cant_connect(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
@@ -345,7 +351,7 @@ bool HttpPolicy::stageAfterCompletion(HttpOpRequest * op)
 }
 
 
-int HttpPolicy::getReadyCount(HttpRequest::policy_t policy_class)
+int HttpPolicy::getReadyCount(HttpRequest::policy_t policy_class) const
 {
 	if (policy_class < mActiveClasses)
 	{
diff --git a/indra/llcorehttp/_httppolicy.h b/indra/llcorehttp/_httppolicy.h
index a02bf084c1..03d92c0b8e 100644
--- a/indra/llcorehttp/_httppolicy.h
+++ b/indra/llcorehttp/_httppolicy.h
@@ -63,22 +63,37 @@ public:
 	/// Cancel all ready and retry requests sending them to
 	/// their notification queues.  Release state resources
 	/// making further request handling impossible.
+	///
+	/// Threading:  called by worker thread
 	void shutdown();
 
 	/// Deliver policy definitions and enable handling of
 	/// requests.  One-time call invoked before starting
 	/// the worker thread.
+	///
+	/// Threading:  called by application thread
 	void start(const HttpPolicyGlobal & global,
 			   const std::vector<HttpPolicyClass> & classes);
 
 	/// Give the policy layer some cycles to scan the ready
 	/// queue promoting higher-priority requests to active
 	/// as permited.
+	///
+	/// @return			Indication of how soon this method
+	///					should be called again.
+	///
+	/// Threading:  called by worker thread
 	HttpService::ELoopSpeed processReadyQueue();
 
 	/// Add request to a ready queue.  Caller is expected to have
 	/// provided us with a reference count to hold the request.  (No
 	/// additional references will be added.)
+	///
+	/// OpRequest is owned by the request queue after this call
+	/// and should not be modified by anyone until retrieved
+	/// from queue.
+	///
+	/// Threading:  called by any thread
 	void addOp(HttpOpRequest *);
 
 	/// Similar to addOp, used when a caller wants to retry a
@@ -87,12 +102,20 @@ public:
 	/// handling is the same and retried operations are considered
 	/// before new ones but that doesn't guarantee completion
 	/// order.
+	///
+	/// Threading:  called by worker thread
 	void retryOp(HttpOpRequest *);
 
-	// Shadows HttpService's method
+	/// Attempt to change the priority of an earlier request.
+	/// Request that Shadows HttpService's method
+	///
+	/// Threading:  called by worker thread
 	bool changePriority(HttpHandle handle, HttpRequest::priority_t priority);
 
-	// Shadows HttpService's method as well
+	/// Attempt to cancel a previous request.
+	/// Shadows HttpService's method as well
+	///
+	/// Threading:  called by worker thread
 	bool cancel(HttpHandle handle);
 
 	/// When transport is finished with an op and takes it off the
@@ -103,17 +126,25 @@ public:
 	/// @return			Returns true of the request is still active
 	///					or ready after staging, false if has been
 	///					sent on to the reply queue.
+	///
+	/// Threading:  called by worker thread
 	bool stageAfterCompletion(HttpOpRequest * op);
 	
 	// Get pointer to global policy options.  Caller is expected
 	// to do context checks like no setting once running.
+	///
+	/// Threading:  called by any thread *but* the object may
+	/// only be modified by the worker thread once running.
+	///
 	HttpPolicyGlobal & getGlobalOptions()
 		{
 			return mGlobalOptions;
 		}
 
-	// Get ready counts for a particular class
-	int getReadyCount(HttpRequest::policy_t policy_class);
+	/// Get ready counts for a particular policy class
+	///
+	/// Threading:  called by worker thread
+	int getReadyCount(HttpRequest::policy_t policy_class) const;
 	
 protected:
 	struct State;
diff --git a/indra/llcorehttp/_httppolicyclass.cpp b/indra/llcorehttp/_httppolicyclass.cpp
index 8007468d3c..a23b81322c 100644
--- a/indra/llcorehttp/_httppolicyclass.cpp
+++ b/indra/llcorehttp/_httppolicyclass.cpp
@@ -35,8 +35,8 @@ namespace LLCore
 
 HttpPolicyClass::HttpPolicyClass()
 	: mSetMask(0UL),
-	  mConnectionLimit(DEFAULT_CONNECTIONS),
-	  mPerHostConnectionLimit(DEFAULT_CONNECTIONS),
+	  mConnectionLimit(HTTP_CONNECTION_LIMIT_DEFAULT),
+	  mPerHostConnectionLimit(HTTP_CONNECTION_LIMIT_DEFAULT),
 	  mPipelining(0)
 {}
 
@@ -71,11 +71,11 @@ HttpStatus HttpPolicyClass::set(HttpRequest::EClassPolicy opt, long value)
 	switch (opt)
 	{
 	case HttpRequest::CP_CONNECTION_LIMIT:
-		mConnectionLimit = llclamp(value, long(LIMIT_CONNECTIONS_MIN), long(LIMIT_CONNECTIONS_MAX));
+		mConnectionLimit = llclamp(value, long(HTTP_CONNECTION_LIMIT_MIN), long(HTTP_CONNECTION_LIMIT_MAX));
 		break;
 
 	case HttpRequest::CP_PER_HOST_CONNECTION_LIMIT:
-		mPerHostConnectionLimit = llclamp(value, long(LIMIT_CONNECTIONS_MIN), mConnectionLimit);
+		mPerHostConnectionLimit = llclamp(value, long(HTTP_CONNECTION_LIMIT_MIN), mConnectionLimit);
 		break;
 
 	case HttpRequest::CP_ENABLE_PIPELINING:
diff --git a/indra/llcorehttp/_httppolicyglobal.cpp b/indra/llcorehttp/_httppolicyglobal.cpp
index ca04839eaf..72f409d3b1 100644
--- a/indra/llcorehttp/_httppolicyglobal.cpp
+++ b/indra/llcorehttp/_httppolicyglobal.cpp
@@ -35,8 +35,8 @@ namespace LLCore
 
 HttpPolicyGlobal::HttpPolicyGlobal()
 	: mSetMask(0UL),
-	  mConnectionLimit(DEFAULT_CONNECTIONS),
-	  mTrace(TRACE_OFF),
+	  mConnectionLimit(HTTP_CONNECTION_LIMIT_DEFAULT),
+	  mTrace(HTTP_TRACE_OFF),
 	  mUseLLProxy(0)
 {}
 
@@ -66,11 +66,11 @@ HttpStatus HttpPolicyGlobal::set(HttpRequest::EGlobalPolicy opt, long value)
 	switch (opt)
 	{
 	case HttpRequest::GP_CONNECTION_LIMIT:
-		mConnectionLimit = llclamp(value, long(LIMIT_CONNECTIONS_MIN), long(LIMIT_CONNECTIONS_MAX));
+		mConnectionLimit = llclamp(value, long(HTTP_CONNECTION_LIMIT_MIN), long(HTTP_CONNECTION_LIMIT_MAX));
 		break;
 
 	case HttpRequest::GP_TRACE:
-		mTrace = llclamp(value, long(TRACE_MIN), long(TRACE_MAX));
+		mTrace = llclamp(value, long(HTTP_TRACE_MIN), long(HTTP_TRACE_MAX));
 		break;
 
 	case HttpRequest::GP_LLPROXY:
diff --git a/indra/llcorehttp/_httpreadyqueue.h b/indra/llcorehttp/_httpreadyqueue.h
index 9cf4b059a1..5f19a9c5f9 100644
--- a/indra/llcorehttp/_httpreadyqueue.h
+++ b/indra/llcorehttp/_httpreadyqueue.h
@@ -45,7 +45,7 @@ namespace LLCore
 /// important of those rules is that any iterator becomes invalid
 /// on element erasure.  So pay attention.
 ///
-/// If LLCORE_READY_QUEUE_IGNORES_PRIORITY tests true, the class
+/// If LLCORE_HTTP_READY_QUEUE_IGNORES_PRIORITY tests true, the class
 /// implements a std::priority_queue interface but on std::deque
 /// behavior to eliminate sensitivity to priority.  In the future,
 /// this will likely become the only behavior or it may become
@@ -54,7 +54,7 @@ namespace LLCore
 /// Threading:  not thread-safe.  Expected to be used entirely by
 /// a single thread, typically a worker thread of some sort.
 
-#if LLCORE_READY_QUEUE_IGNORES_PRIORITY
+#if LLCORE_HTTP_READY_QUEUE_IGNORES_PRIORITY
 
 typedef std::deque<HttpOpRequest *> HttpReadyQueueBase;
 
@@ -64,7 +64,7 @@ typedef std::priority_queue<HttpOpRequest *,
 							std::deque<HttpOpRequest *>,
 							LLCore::HttpOpRequestCompare> HttpReadyQueueBase;
 
-#endif // LLCORE_READY_QUEUE_IGNORES_PRIORITY
+#endif // LLCORE_HTTP_READY_QUEUE_IGNORES_PRIORITY
 
 class HttpReadyQueue : public HttpReadyQueueBase
 {
@@ -82,7 +82,7 @@ protected:
 
 public:
 
-#if LLCORE_READY_QUEUE_IGNORES_PRIORITY
+#if LLCORE_HTTP_READY_QUEUE_IGNORES_PRIORITY
 	// Types and methods needed to make a std::deque look
 	// more like a std::priority_queue, at least for our
 	// purposes.
@@ -103,7 +103,7 @@ public:
 			push_back(v);
 		}
 	
-#endif // LLCORE_READY_QUEUE_IGNORES_PRIORITY
+#endif // LLCORE_HTTP_READY_QUEUE_IGNORES_PRIORITY
 	
 	const container_type & get_container() const
 		{
diff --git a/indra/llcorehttp/_httpservice.cpp b/indra/llcorehttp/_httpservice.cpp
index f7d9813db0..0825888d0f 100644
--- a/indra/llcorehttp/_httpservice.cpp
+++ b/indra/llcorehttp/_httpservice.cpp
@@ -55,8 +55,8 @@ HttpService::HttpService()
 {
 	// Create the default policy class
 	HttpPolicyClass pol_class;
-	pol_class.set(HttpRequest::CP_CONNECTION_LIMIT, DEFAULT_CONNECTIONS);
-	pol_class.set(HttpRequest::CP_PER_HOST_CONNECTION_LIMIT, DEFAULT_CONNECTIONS);
+	pol_class.set(HttpRequest::CP_CONNECTION_LIMIT, HTTP_CONNECTION_LIMIT_DEFAULT);
+	pol_class.set(HttpRequest::CP_PER_HOST_CONNECTION_LIMIT, HTTP_CONNECTION_LIMIT_DEFAULT);
 	pol_class.set(HttpRequest::CP_ENABLE_PIPELINING, 0L);
 	mPolicyClasses.push_back(pol_class);
 }
@@ -150,7 +150,7 @@ void HttpService::term()
 HttpRequest::policy_t HttpService::createPolicyClass()
 {
 	const HttpRequest::policy_t policy_class(mPolicyClasses.size());
-	if (policy_class >= POLICY_CLASS_LIMIT)
+	if (policy_class >= HTTP_POLICY_CLASS_LIMIT)
 	{
 		return 0;
 	}
@@ -219,12 +219,12 @@ bool HttpService::changePriority(HttpHandle handle, HttpRequest::priority_t prio
 }
 
 
-	/// Try to find the given request handle on any of the request
-	/// queues and cancel the operation.
-	///
-	/// @return			True if the request was canceled.
-	///
-	/// Threading:  callable by worker thread.
+/// Try to find the given request handle on any of the request
+/// queues and cancel the operation.
+///
+/// @return			True if the request was canceled.
+///
+/// Threading:  callable by worker thread.
 bool HttpService::cancel(HttpHandle handle)
 {
 	bool canceled(false);
@@ -297,7 +297,7 @@ void HttpService::threadRun(LLCoreInt::HttpThread * thread)
 		// Determine whether to spin, sleep briefly or sleep for next request
 		if (REQUEST_SLEEP != loop)
 		{
-			ms_sleep(LOOP_SLEEP_NORMAL_MS);
+			ms_sleep(HTTP_SERVICE_LOOP_SLEEP_NORMAL_MS);
 		}
 	}
 
@@ -321,11 +321,11 @@ HttpService::ELoopSpeed HttpService::processRequestQueue(ELoopSpeed loop)
 		if (! mExitRequested)
 		{
 			// Setup for subsequent tracing
-			long tracing(TRACE_OFF);
+			long tracing(HTTP_TRACE_OFF);
 			mPolicy->getGlobalOptions().get(HttpRequest::GP_TRACE, &tracing);
 			op->mTracing = (std::max)(op->mTracing, int(tracing));
 
-			if (op->mTracing > TRACE_OFF)
+			if (op->mTracing > HTTP_TRACE_OFF)
 			{
 				LL_INFOS("CoreHttp") << "TRACE, FromRequestQueue, Handle:  "
 									 << static_cast<HttpHandle>(op)
diff --git a/indra/llcorehttp/_httpservice.h b/indra/llcorehttp/_httpservice.h
index d24c497ca9..ffe0349d4d 100644
--- a/indra/llcorehttp/_httpservice.h
+++ b/indra/llcorehttp/_httpservice.h
@@ -55,7 +55,7 @@ class HttpPolicy;
 class HttpLibcurl;
 
 
-/// The HttpService class does the work behind the request  queue.  It
+/// The HttpService class does the work behind the request queue.  It
 /// oversees the HTTP workflow carrying out a number of tasks:
 /// - Pulling requests from the global request queue
 /// - Executing 'immediate' requests directly
@@ -76,7 +76,7 @@ class HttpLibcurl;
 /// HttpPolicy and HttpLibcurl (transport).  These always exist in a
 /// 1:1:1 relationship with HttpService managing instances of the other
 /// two.  So, these classes do not use reference counting to refer
-/// to one-another, their lifecycles are always managed together.
+/// to one another, their lifecycles are always managed together.
 
 class HttpService
 {
@@ -206,7 +206,7 @@ protected:
 	
 	// === shared data ===
 	static volatile EState				sState;
-	HttpRequestQueue *					mRequestQueue;
+	HttpRequestQueue *					mRequestQueue;	// Refcounted
 	LLAtomicU32							mExitRequested;
 	LLCoreInt::HttpThread *				mThread;
 	
diff --git a/indra/llcorehttp/httpcommon.h b/indra/llcorehttp/httpcommon.h
index dd5798edf9..c0d4ec5aad 100644
--- a/indra/llcorehttp/httpcommon.h
+++ b/indra/llcorehttp/httpcommon.h
@@ -60,8 +60,10 @@
 /// Using the library is fairly easy.  Global setup needs a few
 /// steps:
 ///
-/// - libcurl initialization with thread-safely callbacks for c-ares
-///   DNS lookups.
+/// - libcurl initialization including thread-safely callbacks for SSL:
+///   .  curl_global_init(...)
+///   .  CRYPTO_set_locking_callback(...)
+///   .  CRYPTO_set_id_callback(...)
 /// - HttpRequest::createService() called to instantiate singletons
 ///   and support objects.
 ///
@@ -90,8 +92,18 @@
 /// - Do completion processing in your onCompletion() method.
 ///
 /// Code fragments:
-/// <TBD>
+/// Rather than a poorly-maintained example in comments, look in the
+/// example subdirectory which is a minimal yet functional tool to do
+/// GET request performance testing.  With four calls:
 ///
+///   	init_curl();
+///     LLCore::HttpRequest::createService();
+///     LLCore::HttpRequest::startThread();
+///     LLCore::HttpRequest * hr = new LLCore::HttpRequest();
+///
+/// the program is basically ready to issue requests.
+///
+
 
 #include "linden_common.h"		// Modifies curl/curl.h interfaces
 
diff --git a/indra/llcorehttp/httpoptions.cpp b/indra/llcorehttp/httpoptions.cpp
index 68f7277ed3..1699d19f8d 100644
--- a/indra/llcorehttp/httpoptions.cpp
+++ b/indra/llcorehttp/httpoptions.cpp
@@ -36,9 +36,9 @@ namespace LLCore
 HttpOptions::HttpOptions()
 	: RefCounted(true),
 	  mWantHeaders(false),
-	  mTracing(TRACE_OFF),
-	  mTimeout(DEFAULT_TIMEOUT),
-	  mRetries(DEFAULT_RETRY_COUNT)
+	  mTracing(HTTP_TRACE_OFF),
+	  mTimeout(HTTP_REQUEST_TIMEOUT_DEFAULT),
+	  mRetries(HTTP_RETRY_COUNT_DEFAULT)
 {}
 
 
-- 
cgit v1.3