From f0353abe7605778048d69ce3acb8f5ddd5693083 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Tue, 19 Jun 2012 15:43:29 -0400
Subject: [PATCH] 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.
---
indra/llcorehttp/_httpoprequest.cpp | 21 ++-
indra/llcorehttp/_httpoprequest.h | 2 +-
indra/llcorehttp/httpoptions.cpp | 23 ++-
indra/llcorehttp/httpoptions.h | 29 +++-
indra/llcorehttp/tests/test_httprequest.hpp | 155 +++++++++++++++++-
.../llcorehttp/tests/test_llcorehttp_peer.py | 4 +
6 files changed, 204 insertions(+), 30 deletions(-)
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index f78971d8f27..ce41ebcce0b 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 4643cc3b755..92784457631 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -155,7 +155,7 @@ class HttpOpRequest : public HttpOperation
// 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 155fbda7f1d..c11d89e6196 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 78d0aadb2e1..a0b2253c114 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 5b04796c8ad..42e48570377 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 8c3ad805b3e..5f0116d3841 100644
--- a/indra/llcorehttp/tests/test_llcorehttp_peer.py
+++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py
@@ -31,6 +31,7 @@
import os
import sys
+import time
from threading import Thread
from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
@@ -97,6 +98,9 @@ def do_PUT(self):
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)
--
GitLab