Implement timeout and retry count options for requests.

Pretty straightforward.  Still don't like how I'm managing
the options block.  Struct?  Accessors?  Can't decide.  But
the options now speed up the unit test runs even as I add
tests.

6 files changed, 204 insertions, 30 deletions

diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index f78971d8f2..ce41ebcce0 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -108,7 +108,7 @@ HttpOpRequest::HttpOpRequest()
 	  mReplyHeaders(NULL),
 	  mPolicyRetries(0),
 	  mPolicyRetryAt(HttpTime(0)),
-	  mPolicyRetryLimit(5)				// *FIXME:  Get from policy definitions
+	  mPolicyRetryLimit(5)
 {
 	// *NOTE:  As members are added, retry initialization/cleanup
 	// may need to be extended in @prepareRequest().
@@ -333,6 +333,8 @@ void HttpOpRequest::setupCommon(HttpRequest::policy_t policy_id,
 		{
 			mProcFlags |= PF_SAVE_HEADERS;
 		}
+		mPolicyRetryLimit = options->getRetries();
+		mPolicyRetryLimit = llclamp(mPolicyRetryLimit, 0, 100);
 		mTracing = (std::max)(mTracing, llclamp(options->getTrace(), 0, 3));
 	}
 }
@@ -371,10 +373,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	HttpPolicyGlobal & policy(service->getPolicy().getGlobalOptions());
 	
 	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);
 	curl_easy_setopt(mCurlHandle, CURLOPT_NOPROGRESS, 1);
 	curl_easy_setopt(mCurlHandle, CURLOPT_URL, mReqURL.c_str());
@@ -493,13 +492,25 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
 	}
 
 	mCurlHeaders = curl_slist_append(mCurlHeaders, "Pragma:");
+
+	// Request options
+	long timeout(30);
+	if (mReqOptions)
+	{
+		timeout = mReqOptions->getTimeout();
+		timeout = llclamp(timeout, 0L, 3600L);
+	}
+	curl_easy_setopt(mCurlHandle, CURLOPT_TIMEOUT, timeout);
+	curl_easy_setopt(mCurlHandle, CURLOPT_CONNECTTIMEOUT, timeout);
+
+	// Request headers
 	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))
 	{
 		curl_easy_setopt(mCurlHandle, CURLOPT_HEADERFUNCTION, headerCallback);
diff --git a/indra/llcorehttp/_httpoprequest.h b/indra/llcorehttp/_httpoprequest.h
index 4643cc3b75..9278445763 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -155,7 +155,7 @@ public:
 	// Policy data
 	int					mPolicyRetries;
 	HttpTime			mPolicyRetryAt;
-	const int			mPolicyRetryLimit;
+	int					mPolicyRetryLimit;
 };  // end class HttpOpRequest
 
 
diff --git a/indra/llcorehttp/httpoptions.cpp b/indra/llcorehttp/httpoptions.cpp
index 155fbda7f1..c11d89e619 100644
--- a/indra/llcorehttp/httpoptions.cpp
+++ b/indra/llcorehttp/httpoptions.cpp
@@ -34,14 +34,9 @@ namespace LLCore
 HttpOptions::HttpOptions()
 	: RefCounted(true),
 	  mWantHeaders(false),
-	  mTracing(0)
-{}
-
-
-HttpOptions::HttpOptions(const HttpOptions & rhs)
-	: RefCounted(true),
-	  mWantHeaders(rhs.mWantHeaders),
-	  mTracing(rhs.mTracing)
+	  mTracing(0),
+	  mTimeout(30),
+	  mRetries(5)
 {}
 
 
@@ -61,4 +56,16 @@ void HttpOptions::setTrace(long level)
 }
 
 
+void HttpOptions::setTimeout(unsigned int timeout)
+{
+	mTimeout = timeout;
+}
+
+
+void HttpOptions::setRetries(unsigned int retries)
+{
+	mRetries = retries;
+}
+
+
 }   // end namespace LLCore
diff --git a/indra/llcorehttp/httpoptions.h b/indra/llcorehttp/httpoptions.h
index 78d0aadb2e..a0b2253c11 100644
--- a/indra/llcorehttp/httpoptions.h
+++ b/indra/llcorehttp/httpoptions.h
@@ -60,29 +60,44 @@ class HttpOptions : public LLCoreInt::RefCounted
 {
 public:
 	HttpOptions();
-	HttpOptions(const HttpOptions &);
 
 protected:
 	virtual ~HttpOptions();						// Use release()
 	
+	HttpOptions(const HttpOptions &);			// Not defined
 	void operator=(const HttpOptions &);		// Not defined
 
 public:
-	void		setWantHeaders();
-	bool		getWantHeaders() const
+	void				setWantHeaders();
+	bool				getWantHeaders() const
 		{
 			return mWantHeaders;
 		}
 	
-	void		setTrace(long level);
-	int			getTrace() const
+	void				setTrace(int long);
+	int					getTrace() const
 		{
 			return mTracing;
 		}
+
+	void				setTimeout(unsigned int timeout);
+	unsigned int		getTimeout() const
+		{
+			return mTimeout;
+		}
+
+	void				setRetries(unsigned int retries);
+	unsigned int		getRetries() const
+		{
+			return mRetries;
+		}
 	
 protected:
-	bool		mWantHeaders;
-	long		int mTracing;
+	bool				mWantHeaders;
+	int					mTracing;
+	unsigned int		mTimeout;
+	unsigned int		mRetries;
+	
 }; // end class HttpOptions
 
 
diff --git a/indra/llcorehttp/tests/test_httprequest.hpp b/indra/llcorehttp/tests/test_httprequest.hpp
index 5b04796c8a..42e4857037 100644
--- a/indra/llcorehttp/tests/test_httprequest.hpp
+++ b/indra/llcorehttp/tests/test_httprequest.hpp
@@ -30,6 +30,7 @@
 #include "bufferarray.h"
 #include "httphandler.h"
 #include "httpresponse.h"
+#include "httpoptions.h"
 #include "_httpservice.h"
 #include "_httprequestqueue.h"
 
@@ -407,7 +408,8 @@ void HttpRequestTestObjectType::test<5>()
 	mHandlerCalls = 0;
 
 	HttpRequest * req = NULL;
-
+	HttpOptions * opts = NULL;
+	
 	try
 	{
 		// Get singletons created
@@ -421,6 +423,9 @@ void HttpRequestTestObjectType::test<5>()
 		req = new HttpRequest();
 		ensure("Memory allocated on construction", mMemTotal < GetMemTotal());
 
+		opts = new HttpOptions();
+		opts->setRetries(1);			// Don't try for too long - default retries take about 18S
+		
 		// Issue a GET that can't connect
 		mStatus = HttpStatus(HttpStatus::EXT_CURL_EASY, CURLE_COULDNT_CONNECT);
 		HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
@@ -428,14 +433,14 @@ void HttpRequestTestObjectType::test<5>()
 													 "http://127.0.0.1:2/nothing/here",
 													 0,
 													 0,
-													 NULL,
+													 opts,
 													 NULL,
 													 &handler);
 		ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
 
 		// Run the notification pump.
 		int count(0);
-		int limit(180);				// With retries, can take more than 10 seconds to give up
+		int limit(50);				// With one retry, should fail quickish
 		while (count++ < limit && mHandlerCalls < 1)
 		{
 			req->update(1000);
@@ -468,7 +473,11 @@ void HttpRequestTestObjectType::test<5>()
 			usleep(100000);
 		}
 		ensure("Thread actually stopped running", HttpService::isStopped());
-	
+
+		// release options
+		opts->release();
+		opts = NULL;
+		
 		// release the request object
 		delete req;
 		req = NULL;
@@ -490,6 +499,11 @@ void HttpRequestTestObjectType::test<5>()
 	catch (...)
 	{
 		stop_thread(req);
+		if (opts)
+		{
+			opts->release();
+			opts = NULL;
+		}
 		delete req;
 		HttpRequest::destroyService();
 		throw;
@@ -503,7 +517,7 @@ void HttpRequestTestObjectType::test<6>()
 	ScopedCurlInit ready;
 
 	std::string url_base(get_base_url());
-	std::cerr << "Base:  "  << url_base << std::endl;
+	// std::cerr << "Base:  "  << url_base << std::endl;
 	
 	set_test_name("HttpRequest GET to real service");
 
@@ -611,7 +625,7 @@ void HttpRequestTestObjectType::test<7>()
 	ScopedCurlInit ready;
 
 	std::string url_base(get_base_url());
-	std::cerr << "Base:  "  << url_base << std::endl;
+	// std::cerr << "Base:  "  << url_base << std::endl;
 	
 	set_test_name("HttpRequest GET with Range: header to real service");
 
@@ -721,7 +735,7 @@ void HttpRequestTestObjectType::test<8>()
 	ScopedCurlInit ready;
 
 	std::string url_base(get_base_url());
-	std::cerr << "Base:  "  << url_base << std::endl;
+	// std::cerr << "Base:  "  << url_base << std::endl;
 	
 	set_test_name("HttpRequest PUT to real service");
 
@@ -840,7 +854,7 @@ void HttpRequestTestObjectType::test<9>()
 	ScopedCurlInit ready;
 
 	std::string url_base(get_base_url());
-	std::cerr << "Base:  "  << url_base << std::endl;
+	// std::cerr << "Base:  "  << url_base << std::endl;
 	
 	set_test_name("HttpRequest POST to real service");
 
@@ -959,7 +973,7 @@ void HttpRequestTestObjectType::test<10>()
 	ScopedCurlInit ready;
 
 	std::string url_base(get_base_url());
-	std::cerr << "Base:  "  << url_base << std::endl;
+	// std::cerr << "Base:  "  << url_base << std::endl;
 	
 	set_test_name("HttpRequest GET with some tracing");
 
@@ -1065,6 +1079,129 @@ void HttpRequestTestObjectType::test<10>()
 	}
 }
 
+
+template <> template <>
+void HttpRequestTestObjectType::test<11>()
+{
+	ScopedCurlInit ready;
+
+	set_test_name("HttpRequest GET timeout");
+
+	// Handler can be stack-allocated *if* there are no dangling
+	// references to it after completion of this method.
+	// Create before memory record as the string copy will bump numbers.
+	TestHandler2 handler(this, "handler");
+	std::string url_base(get_base_url() + "/sleep/");	// path to a 30-second sleep
+		
+	// record the total amount of dynamically allocated memory
+	mMemTotal = GetMemTotal();
+	mHandlerCalls = 0;
+
+	HttpRequest * req = NULL;
+	HttpOptions * opts = NULL;
+	
+	try
+	{
+		// Get singletons created
+		HttpRequest::createService();
+		
+		// Start threading early so that thread memory is invariant
+		// over the test.
+		HttpRequest::startThread();
+
+		// create a new ref counted object with an implicit reference
+		req = new HttpRequest();
+		ensure("Memory allocated on construction", mMemTotal < GetMemTotal());
+
+		opts = new HttpOptions();
+		opts->setRetries(0);			// Don't retry
+		opts->setTimeout(2);
+		
+		// Issue a GET that can't connect
+		mStatus = HttpStatus(HttpStatus::EXT_CURL_EASY, CURLE_OPERATION_TIMEDOUT);
+		HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
+													 0U,
+													 url_base,
+													 0,
+													 0,
+													 opts,
+													 NULL,
+													 &handler);
+		ensure("Valid handle returned for ranged request", handle != LLCORE_HTTP_HANDLE_INVALID);
+
+		// Run the notification pump.
+		int count(0);
+		int limit(50);				// With one retry, should fail quickish
+		while (count++ < limit && mHandlerCalls < 1)
+		{
+			req->update(1000);
+			usleep(100000);
+		}
+		ensure("Request executed in reasonable time", count < limit);
+		ensure("One handler invocation for request", mHandlerCalls == 1);
+
+		// Okay, request a shutdown of the servicing thread
+		mStatus = HttpStatus();
+		handle = req->requestStopThread(&handler);
+		ensure("Valid handle returned for second request", handle != LLCORE_HTTP_HANDLE_INVALID);
+	
+		// Run the notification pump again
+		count = 0;
+		limit = 100;
+		while (count++ < limit && mHandlerCalls < 2)
+		{
+			req->update(1000);
+			usleep(100000);
+		}
+		ensure("Second request executed in reasonable time", count < limit);
+		ensure("Second handler invocation", mHandlerCalls == 2);
+
+		// See that we actually shutdown the thread
+		count = 0;
+		limit = 10;
+		while (count++ < limit && ! HttpService::isStopped())
+		{
+			usleep(100000);
+		}
+		ensure("Thread actually stopped running", HttpService::isStopped());
+
+		// release options
+		opts->release();
+		opts = NULL;
+		
+		// release the request object
+		delete req;
+		req = NULL;
+
+		// Shut down service
+		HttpRequest::destroyService();
+	
+		ensure("Two handler calls on the way out", 2 == mHandlerCalls);
+
+#if defined(WIN32)
+		// Can only do this memory test on Windows.  On other platforms,
+		// the LL logging system holds on to memory and produces what looks
+		// like memory leaks...
+	
+		// printf("Old mem:  %d, New mem:  %d\n", mMemTotal, GetMemTotal());
+		ensure("Memory usage back to that at entry", mMemTotal == GetMemTotal());
+#endif
+	}
+	catch (...)
+	{
+		stop_thread(req);
+		if (opts)
+		{
+			opts->release();
+			opts = NULL;
+		}
+		delete req;
+		HttpRequest::destroyService();
+		throw;
+	}
+}
+
+
 }  // end namespace tut
 
 namespace
diff --git a/indra/llcorehttp/tests/test_llcorehttp_peer.py b/indra/llcorehttp/tests/test_llcorehttp_peer.py
index 8c3ad805b3..5f0116d384 100644
--- a/indra/llcorehttp/tests/test_llcorehttp_peer.py
+++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py
@@ -31,6 +31,7 @@ $/LicenseInfo$
 
 import os
 import sys
+import time
 from threading import Thread
 from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
 
@@ -97,6 +98,9 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler):
 
     def answer(self, data):
         debug("%s.answer(%s): self.path = %r", self.__class__.__name__, data, self.path)
+        if self.path.find("/sleep/") != -1:
+            time.sleep(30)
+
         if "fail" not in self.path:
             response = llsd.format_xml(data.get("reply", llsd.LLSD("success")))
             debug("success: %s", response)