From c35bc713346ea40ab93d1a44879c438e30bc4ace Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Mon, 22 Oct 2012 00:44:19 +0200 Subject: [PATCH] Fix crash when libcurl calls read callback before calling socket callback. In days of usage this has never happened before, but apparently it's possible. The solution chosen is to create the AIHTTPTimeout object on the fly when it doesn't exist and let it be picked up later when the CurlSocketInfo for the transfer is created. --- indra/aistatemachine/aicurl.cpp | 19 +++++++++++++++++++ indra/aistatemachine/aicurlprivate.h | 15 ++++++++++----- indra/aistatemachine/aicurlthread.cpp | 11 +++++++---- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/indra/aistatemachine/aicurl.cpp b/indra/aistatemachine/aicurl.cpp index 58e2eaf37..c9f393ff5 100644 --- a/indra/aistatemachine/aicurl.cpp +++ b/indra/aistatemachine/aicurl.cpp @@ -1202,6 +1202,25 @@ void CurlEasyRequest::set_timeout_opts(void) setopt(CURLOPT_TIMEOUT, mTimeoutPolicy->getCurlTransaction()); } +void CurlEasyRequest::create_timeout_object(ThreadSafeCurlEasyRequest* lockobj) +{ + mTimeout = new curlthread::HTTPTimeout(mTimeoutPolicy, lockobj); +} + +LLPointer& CurlEasyRequest::get_timeout_object(ThreadSafeCurlEasyRequest* lockobj) +{ + if (mTimeoutIsOrphan) + { + mTimeoutIsOrphan = false; + llassert_always(mTimeout); + } + else + { + create_timeout_object(lockobj); + } + return mTimeout; +} + void CurlEasyRequest::print_curl_timings(void) const { double t; diff --git a/indra/aistatemachine/aicurlprivate.h b/indra/aistatemachine/aicurlprivate.h index d6f941a5d..fbf70fcce 100644 --- a/indra/aistatemachine/aicurlprivate.h +++ b/indra/aistatemachine/aicurlprivate.h @@ -325,6 +325,9 @@ class CurlEasyRequest : public CurlEasyHandle { // Used in applyDefaultOptions. static CURLcode curlCtxCallback(CURL* curl, void* sslctx, void* parm); + // Called from get_timeout_object and httptimeout. + void create_timeout_object(ThreadSafeCurlEasyRequest* lockobj); + public: // Set default options that we want applied to all curl easy handles. void applyDefaultOptions(void); @@ -370,7 +373,8 @@ class CurlEasyRequest : public CurlEasyHandle { AIHTTPTimeoutPolicy const* mTimeoutPolicy; std::string mLowercaseHostname; // Lowercase hostname (canonicalized) extracted from the url. - LLPointer mTimeout; + LLPointer mTimeout;// Timeout administration object associated with last created CurlSocketInfo. + bool mTimeoutIsOrphan; // Set to true when mTimeout is not (yet) associated with a CurlSocketInfo. #if defined(CWDEBUG) || defined(DEBUG_CURLIO) public: bool mDebugIsGetMethod; @@ -381,9 +385,10 @@ class CurlEasyRequest : public CurlEasyHandle { AIHTTPTimeoutPolicy const* getTimeoutPolicy(void) const { return mTimeoutPolicy; } std::string const& getLowercaseHostname(void) const { return mLowercaseHostname; } // Called by CurlSocketInfo to allow access to the last (after a redirect) HTTPTimeout object related to this request. - void set_timeout_object(LLPointer& timeout) { mTimeout = timeout; } - // And the accessor for it. - LLPointer& httptimeout(void) { return mTimeout; } + // This creates mTimeout (unless mTimeoutIsOrphan is set in which case it adopts the orphan). + LLPointer& get_timeout_object(ThreadSafeCurlEasyRequest* lockobj); + // Accessor for mTimeout with optional creation of orphaned object (if lockobj != NULL). + LLPointer& httptimeout(ThreadSafeCurlEasyRequest* lockobj = NULL) { if (lockobj && !mTimeout) create_timeout_object(lockobj); return mTimeout; } // Return true if no data has been received on the latest socket (if any) for too long. bool has_stalled(void) const { return mTimeout && mTimeout->has_stalled(); } @@ -391,7 +396,7 @@ class CurlEasyRequest : public CurlEasyHandle { // This class may only be created by constructing a ThreadSafeCurlEasyRequest. friend class ThreadSafeCurlEasyRequest; // Throws AICurlNoEasyHandle. - CurlEasyRequest(void) : mHeaders(NULL), mEventsTarget(NULL), mResult(CURLE_FAILED_INIT), mTimeoutPolicy(NULL) + CurlEasyRequest(void) : mHeaders(NULL), mEventsTarget(NULL), mResult(CURLE_FAILED_INIT), mTimeoutPolicy(NULL), mTimeoutIsOrphan(false) #if defined(CWDEBUG) || defined(DEBUG_CURLIO) , mDebugIsGetMethod(false) #endif diff --git a/indra/aistatemachine/aicurlthread.cpp b/indra/aistatemachine/aicurlthread.cpp index 080908538..e03d92e7f 100644 --- a/indra/aistatemachine/aicurlthread.cpp +++ b/indra/aistatemachine/aicurlthread.cpp @@ -773,8 +773,7 @@ CurlSocketInfo::CurlSocketInfo(MultiHandle& multi_handle, CURL* easy, curl_socke // CurlSocketInfo objects for a request and we need upload_finished() to be called on the HTTPTimeout // object related to THIS CurlSocketInfo. AICurlEasyRequest_wat easy_request_w(*lockobj); - mTimeout = new HTTPTimeout(easy_request_w->getTimeoutPolicy(), lockobj); - easy_request_w->set_timeout_object(mTimeout); + mTimeout = easy_request_w->get_timeout_object(lockobj); } CurlSocketInfo::~CurlSocketInfo() @@ -2170,8 +2169,12 @@ size_t CurlResponderBuffer::curlReadCallback(char* data, size_t size, size_t nme S32 bytes = size * nmemb; // The maximum amount to read. AICurlResponderBuffer_wat buffer_w(*lockobj); buffer_w->mLastRead = buffer_w->getInput()->readAfter(sChannels.out(), buffer_w->mLastRead, (U8*)data, bytes); - buffer_w->mRequestTransferedBytes += bytes; // Accumulate data sent to the server. - if (buffered_easy_request_w->httptimeout()->data_sent(bytes)) // Timeout administration. + buffer_w->mRequestTransferedBytes += bytes; // Accumulate data sent to the server. + // Timeout administration. Note that it can happen that we get here + // before the socket callback has been called, because the silly libcurl + // writes headers without informing us. In that case it's OK to create + // the Timeout object on the fly, so pass lockobj. + if (buffered_easy_request_w->httptimeout(lockobj)->data_sent(bytes)) { // Transfer timed out. Return CURL_READFUNC_ABORT which will abort with error CURLE_ABORTED_BY_CALLBACK. return CURL_READFUNC_ABORT;