From ed5db306545e414a1c975c1fff5908b6c2fe1389 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Thu, 21 Jun 2012 21:32:33 -0400
Subject: [PATCH] Preparing for better shutdown/cleanup logic.

---
 indra/llcorehttp/_httprequestqueue.cpp      | 29 ++++++++-
 indra/llcorehttp/_httprequestqueue.h        | 11 +++-
 indra/llcorehttp/_httpservice.cpp           |  2 +
 indra/llcorehttp/httprequest.cpp            | 67 +++++++++++++++++----
 indra/llcorehttp/tests/test_httprequest.hpp | 14 ++---
 5 files changed, 100 insertions(+), 23 deletions(-)

diff --git a/indra/llcorehttp/_httprequestqueue.cpp b/indra/llcorehttp/_httprequestqueue.cpp
index 92bb5ec5c1e..6487ef6fa5e 100644
--- a/indra/llcorehttp/_httprequestqueue.cpp
+++ b/indra/llcorehttp/_httprequestqueue.cpp
@@ -39,7 +39,8 @@ HttpRequestQueue * HttpRequestQueue::sInstance(NULL);
 
 
 HttpRequestQueue::HttpRequestQueue()
-	: RefCounted(true)
+	: RefCounted(true),
+	  mQueueStopped(false)
 {
 }
 
@@ -72,14 +73,25 @@ void HttpRequestQueue::term()
 }
 
 
-void HttpRequestQueue::addOp(HttpOperation * op)
+HttpStatus HttpRequestQueue::addOp(HttpOperation * op)
 {
+	bool wake(false);
 	{
 		HttpScopedLock lock(mQueueMutex);
 
+		if (mQueueStopped)
+		{
+			// Return op and error to caller
+			return HttpStatus(HttpStatus::LLCORE, HE_SHUTTING_DOWN);
+		}
+		wake = mQueue.empty();
 		mQueue.push_back(op);
 	}
-	mQueueCV.notify_all();
+	if (wake)
+	{
+		mQueueCV.notify_all();
+	}
+	return HttpStatus();
 }
 
 
@@ -129,4 +141,15 @@ void HttpRequestQueue::fetchAll(bool wait, OpContainer & ops)
 	return;
 }
 
+
+void HttpRequestQueue::stopQueue()
+{
+	{
+		HttpScopedLock lock(mQueueMutex);
+
+		mQueueStopped = true;
+	}
+}
+
+
 } // end namespace LLCore
diff --git a/indra/llcorehttp/_httprequestqueue.h b/indra/llcorehttp/_httprequestqueue.h
index 26d7d9dca67..6e8f00c4dae 100644
--- a/indra/llcorehttp/_httprequestqueue.h
+++ b/indra/llcorehttp/_httprequestqueue.h
@@ -30,6 +30,7 @@
 
 #include <vector>
 
+#include "httpcommon.h"
 #include "_refcounted.h"
 #include "_mutex.h"
 
@@ -74,11 +75,11 @@ class HttpRequestQueue : public LLCoreInt::RefCounted
 
 	/// Insert an object at the back of the reply queue.
 	///
-	/// Caller my provide one refcount to the Library which takes
+	/// Caller must provide one refcount to the queue which takes
 	/// possession of the count.
 	///
 	/// Threading:  callable by any thread.
-	void addOp(HttpOperation * op);
+	HttpStatus addOp(HttpOperation * op);
 
 	/// Caller acquires reference count on returned operation
 	///
@@ -89,6 +90,11 @@ class HttpRequestQueue : public LLCoreInt::RefCounted
 	///
 	/// Threading:  callable by any thread.
 	void fetchAll(bool wait, OpContainer & ops);
+
+	/// Disallow further request queuing
+	///
+	/// Threading:  callable by any thread.
+	void stopQueue();
 	
 protected:
 	static HttpRequestQueue *			sInstance;
@@ -97,6 +103,7 @@ class HttpRequestQueue : public LLCoreInt::RefCounted
 	OpContainer							mQueue;
 	LLCoreInt::HttpMutex				mQueueMutex;
 	LLCoreInt::HttpConditionVariable	mQueueCV;
+	bool								mQueueStopped;
 	
 }; // end class HttpRequestQueue
 
diff --git a/indra/llcorehttp/_httpservice.cpp b/indra/llcorehttp/_httpservice.cpp
index 25f64acc424..faafd9a6c70 100644
--- a/indra/llcorehttp/_httpservice.cpp
+++ b/indra/llcorehttp/_httpservice.cpp
@@ -172,6 +172,8 @@ bool HttpService::changePriority(HttpHandle handle, HttpRequest::priority_t prio
 
 void HttpService::shutdown()
 {
+	mRequestQueue->stopQueue();
+
 	// *FIXME:  Run down everything....
 }
 
diff --git a/indra/llcorehttp/httprequest.cpp b/indra/llcorehttp/httprequest.cpp
index e906ff8a1e6..6d13a213f55 100644
--- a/indra/llcorehttp/httprequest.cpp
+++ b/indra/llcorehttp/httprequest.cpp
@@ -154,7 +154,12 @@ HttpHandle HttpRequest::requestGet(policy_t policy_id,
 		return handle;
 	}
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfers refcount
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 	
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
@@ -183,7 +188,12 @@ HttpHandle HttpRequest::requestGetByteRange(policy_t policy_id,
 		return handle;
 	}
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfers refcount
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 	
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
@@ -211,7 +221,12 @@ HttpHandle HttpRequest::requestPost(policy_t policy_id,
 		return handle;
 	}
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfers refcount
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 	
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
@@ -239,7 +254,12 @@ HttpHandle HttpRequest::requestPut(policy_t policy_id,
 		return handle;
 	}
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfers refcount
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 	
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
@@ -255,7 +275,12 @@ HttpHandle HttpRequest::requestNoOp(HttpHandler * user_handler)
 
 	HttpOpNull * op = new HttpOpNull();
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfer refcount as well
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
@@ -287,14 +312,19 @@ HttpStatus HttpRequest::update(long millis)
 // Request Management Methods
 // ====================================
 
-HttpHandle HttpRequest::requestCancel(HttpHandle handle, HttpHandler * user_handler)
+HttpHandle HttpRequest::requestCancel(HttpHandle request, HttpHandler * user_handler)
 {
 	HttpStatus status;
 	HttpHandle ret_handle(LLCORE_HTTP_HANDLE_INVALID);
 
-	HttpOpCancel * op = new HttpOpCancel(handle);
+	HttpOpCancel * op = new HttpOpCancel(request);
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfer refcount as well
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return ret_handle;
+	}
 
 	mLastReqStatus = status;
 	ret_handle = static_cast<HttpHandle>(op);
@@ -311,7 +341,12 @@ HttpHandle HttpRequest::requestSetPriority(HttpHandle request, priority_t priori
 
 	HttpOpSetPriority * op = new HttpOpSetPriority(request, priority);
 	op->setReplyPath(mReplyQueue, handler);
-	mRequestQueue->addOp(op);			// transfer refcount as well
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return ret_handle;
+	}
 
 	mLastReqStatus = status;
 	ret_handle = static_cast<HttpHandle>(op);
@@ -368,7 +403,12 @@ HttpHandle HttpRequest::requestStopThread(HttpHandler * user_handler)
 
 	HttpOpStop * op = new HttpOpStop();
 	op->setReplyPath(mReplyQueue, user_handler);
-	mRequestQueue->addOp(op);			// transfer refcount as well
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
@@ -388,7 +428,12 @@ HttpHandle HttpRequest::requestSetHttpProxy(const std::string & proxy, HttpHandl
 	HttpOpSetGet * op = new HttpOpSetGet();
 	op->setupSet(GP_HTTP_PROXY, proxy);
 	op->setReplyPath(mReplyQueue, handler);
-	mRequestQueue->addOp(op);			// transfer refcount as well
+	if (! (status = mRequestQueue->addOp(op)))			// transfers refcount
+	{
+		op->release();
+		mLastReqStatus = status;
+		return handle;
+	}
 
 	mLastReqStatus = status;
 	handle = static_cast<HttpHandle>(op);
diff --git a/indra/llcorehttp/tests/test_httprequest.hpp b/indra/llcorehttp/tests/test_httprequest.hpp
index cac927cfcae..8e62cd0f92a 100644
--- a/indra/llcorehttp/tests/test_httprequest.hpp
+++ b/indra/llcorehttp/tests/test_httprequest.hpp
@@ -149,7 +149,7 @@ void HttpRequestTestObjectType::test<1>()
 		
 		// create a new ref counted object with an implicit reference
 		req = new HttpRequest();
-		ensure(mMemTotal < GetMemTotal());
+		ensure("Memory being used", mMemTotal < GetMemTotal());
 		
 		// release the request object
 		delete req;
@@ -158,7 +158,7 @@ void HttpRequestTestObjectType::test<1>()
 		HttpRequest::destroyService();
 
 		// make sure we didn't leak any memory
-		ensure(mMemTotal == GetMemTotal());
+		ensure("Memory returned", mMemTotal == GetMemTotal());
 	}
 	catch (...)
 	{
@@ -187,11 +187,11 @@ void HttpRequestTestObjectType::test<2>()
 		
 		// create a new ref counted object with an implicit reference
 		req = new HttpRequest();
-		ensure(mMemTotal < GetMemTotal());
+		ensure("Memory being used", mMemTotal < GetMemTotal());
 
 		// Issue a NoOp
 		HttpHandle handle = req->requestNoOp(NULL);
-		ensure(handle != LLCORE_HTTP_HANDLE_INVALID);
+		ensure("Request issued", handle != LLCORE_HTTP_HANDLE_INVALID);
 		
 		// release the request object
 		delete req;
@@ -199,15 +199,15 @@ void HttpRequestTestObjectType::test<2>()
 
 		// We're still holding onto the operation which is
 		// sitting, unserviced, on the request queue so...
-		ensure(mMemTotal < GetMemTotal());
+		ensure("Memory being used 2", mMemTotal < GetMemTotal());
 
 		// Request queue should have two references:  global singleton & service object
 		ensure("Two references to request queue", 2 == HttpRequestQueue::instanceOf()->getRefCount());
 
 		// Okay, tear it down
 		HttpRequest::destroyService();
-		// printf("Old mem:  %d, New mem:  %d\n", mMemTotal, GetMemTotal());
-		ensure(mMemTotal == GetMemTotal());
+		printf("Old mem:  %d, New mem:  %d\n", mMemTotal, GetMemTotal());
+		ensure("Memory returned", mMemTotal == GetMemTotal());
 	}
 	catch (...)
 	{
-- 
GitLab