From b08125a5874a89ce5210f8fb2c961ae17fb80fde Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Thu, 14 Jun 2012 16:31:48 -0400 Subject: LLMutex recursive lock, global & per-request tracing, simple GET request, LLProxy support, HttpOptions starting to work, HTTP resource waiting fixed. Non-LLThread-based threads need to do some registration or LLMutex locks taken out in these threads will not work as expected (SH-3154). We'll get a better solution later, this fixes some things for now. Tracing of operations now supported. Global and per-request (via HttpOptions) tracing levels of [0..3]. The 2 and 3 levels use libcurl's VERBOSE mode combined with CURLOPT_DEBUGFUNCTION to stream high levels of detail into the log. *Very* laggy but useful. Simple GET request supported (no Range: header). Really just a degenrate case of a ranged get but supplied an API anyway. Global option to use the LLProxy interface to setup CURL handles for either socks5 or http proxy usage. This isn't really the most encapsulated way to do this but a better solution will have to come later. The wantHeaders and tracing options are now supported in HttpOptions giving per-request controls. Big refactoring of the HTTP resource waiter in lltexturefetch. What I was doing before wasn't correct. Instead, I'm implementing the resource wait after the Semaphore model (though not using system semaphores). So instead of having a sequence like: SEND_HTTP_REQ -> WAIT_HTTP_RESOURCE -> SEND_HTTP_REQ, we now do WAIT_HTTP_RESOURCE -> WAIT_HTTP_RESOURCE2 (actual wait) -> SEND_HTTP_REQ. Works well but the prioritized filling of the corehttp library needs some performance work later. --- indra/llcommon/llthread.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'indra/llcommon/llthread.cpp') diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index a6ad6b125c..b27b64b26f 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -71,6 +71,13 @@ LL_COMMON_API void assert_main_thread() } } +void LLThread::registerThreadID() +{ +#if !LL_DARWIN + sThreadID = ++sIDIter; +#endif +} + // // Handed to the APR thread creation function // -- cgit v1.3 From 374f20edf09ff8194c715d190c114eaacac00bfe Mon Sep 17 00:00:00 2001 From: Dave Parks Date: Wed, 3 Oct 2012 14:30:21 -0500 Subject: Fix non-thread-safe refcounting of LLHTTPClient::Responder and fix out-of-order deletion of LLTextureFetch on shutdown --- indra/llcommon/llthread.cpp | 35 ++++++++++++++----- indra/llcommon/llthread.h | 30 ++++++++++++----- indra/llmessage/llcurl.cpp | 38 +++++++-------------- indra/llmessage/llcurl.h | 15 +++------ indra/newview/llappviewer.cpp | 6 ++-- indra/newview/llassetuploadresponders.cpp | 8 ++--- indra/newview/lleventpoll.cpp | 2 +- indra/newview/llfloatertos.cpp | 6 ++-- indra/newview/lltexturefetch.cpp | 2 ++ indra/newview/lltranslate.h | 4 +-- indra/newview/llviewermessage.cpp | 4 +-- indra/newview/llviewerregion.cpp | 5 ++- .../viewer_components/updater/llupdatechecker.cpp | 39 +--------------------- indra/viewer_components/updater/llupdatechecker.h | 34 +++++++++++++++++-- 14 files changed, 117 insertions(+), 111 deletions(-) (limited to 'indra/llcommon/llthread.cpp') diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index a6ad6b125c..c2fbb544a8 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -114,7 +114,7 @@ LLThread::LLThread(const std::string& name, apr_pool_t *poolp) : apr_pool_create(&mAPRPoolp, NULL); // Create a subpool for this thread } mRunCondition = new LLCondition(mAPRPoolp); - + mDataLock = new LLMutex(mAPRPoolp); mLocalAPRFilePoolp = NULL ; } @@ -173,7 +173,10 @@ void LLThread::shutdown() } delete mRunCondition; - mRunCondition = 0; + mRunCondition = NULL; + + delete mDataLock; + mDataLock = NULL; if (mIsLocalPool && mAPRPoolp) { @@ -242,28 +245,30 @@ bool LLThread::runCondition(void) // Stop thread execution if requested until unpaused. void LLThread::checkPause() { - mRunCondition->lock(); + mDataLock->lock(); // This is in a while loop because the pthread API allows for spurious wakeups. while(shouldSleep()) { + mDataLock->unlock(); mRunCondition->wait(); // unlocks mRunCondition + mDataLock->lock(); // mRunCondition is locked when the thread wakes up } - mRunCondition->unlock(); + mDataLock->unlock(); } //============================================================================ void LLThread::setQuitting() { - mRunCondition->lock(); + mDataLock->lock(); if (mStatus == RUNNING) { mStatus = QUITTING; } - mRunCondition->unlock(); + mDataLock->unlock(); wake(); } @@ -285,12 +290,12 @@ void LLThread::yield() void LLThread::wake() { - mRunCondition->lock(); + mDataLock->lock(); if(!shouldSleep()) { mRunCondition->signal(); } - mRunCondition->unlock(); + mDataLock->unlock(); } void LLThread::wakeLocked() @@ -481,6 +486,19 @@ LLThreadSafeRefCount::LLThreadSafeRefCount() : { } +LLThreadSafeRefCount::LLThreadSafeRefCount(const LLThreadSafeRefCount& src) +{ + if (sMutex) + { + sMutex->lock(); + } + mRef = 0; + if (sMutex) + { + sMutex->unlock(); + } +} + LLThreadSafeRefCount::~LLThreadSafeRefCount() { if (mRef != 0) @@ -489,6 +507,7 @@ LLThreadSafeRefCount::~LLThreadSafeRefCount() } } + //============================================================================ LLResponder::~LLResponder() diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index b52e70ab2e..892e144911 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -97,6 +97,7 @@ private: protected: std::string mName; LLCondition* mRunCondition; + LLMutex* mDataLock; apr_thread_t *mAPRThreadp; apr_pool_t *mAPRPoolp; @@ -122,15 +123,15 @@ protected: inline void unlockData(); // This is the predicate that decides whether the thread should sleep. - // It should only be called with mRunCondition locked, since the virtual runCondition() function may need to access + // It should only be called with mDataLock locked, since the virtual runCondition() function may need to access // data structures that are thread-unsafe. bool shouldSleep(void) { return (mStatus == RUNNING) && (isPaused() || (!runCondition())); } // To avoid spurious signals (and the associated context switches) when the condition may or may not have changed, you can do the following: - // mRunCondition->lock(); + // mDataLock->lock(); // if(!shouldSleep()) // mRunCondition->signal(); - // mRunCondition->unlock(); + // mDataLock->unlock(); }; //============================================================================ @@ -205,12 +206,12 @@ private: void LLThread::lockData() { - mRunCondition->lock(); + mDataLock->lock(); } void LLThread::unlockData() { - mRunCondition->unlock(); + mDataLock->unlock(); } @@ -227,15 +228,26 @@ public: private: static LLMutex* sMutex; -private: - LLThreadSafeRefCount(const LLThreadSafeRefCount&); // not implemented - LLThreadSafeRefCount&operator=(const LLThreadSafeRefCount&); // not implemented - protected: virtual ~LLThreadSafeRefCount(); // use unref() public: LLThreadSafeRefCount(); + LLThreadSafeRefCount(const LLThreadSafeRefCount&); + LLThreadSafeRefCount&operator=(const LLThreadSafeRefCount& ref) + { + if (sMutex) + { + sMutex->lock(); + } + mRef = 0; + if (sMutex) + { + sMutex->unlock(); + } + } + + void ref() { diff --git a/indra/llmessage/llcurl.cpp b/indra/llmessage/llcurl.cpp index 0de47e0181..158e4ce091 100644 --- a/indra/llmessage/llcurl.cpp +++ b/indra/llmessage/llcurl.cpp @@ -133,12 +133,12 @@ std::string LLCurl::getVersionString() ////////////////////////////////////////////////////////////////////////////// LLCurl::Responder::Responder() - : mReferenceCount(0) { } LLCurl::Responder::~Responder() { + LL_CHECK_MEMORY } // virtual @@ -202,23 +202,6 @@ void LLCurl::Responder::completedHeader(U32 status, const std::string& reason, c } -namespace boost -{ - void intrusive_ptr_add_ref(LLCurl::Responder* p) - { - ++p->mReferenceCount; - } - - void intrusive_ptr_release(LLCurl::Responder* p) - { - if (p && 0 == --p->mReferenceCount) - { - delete p; - } - } -}; - - ////////////////////////////////////////////////////////////////////////////// std::set LLCurl::Easy::sFreeHandles; @@ -267,15 +250,18 @@ void LLCurl::Easy::releaseEasyHandle(CURL* handle) LLMutexLock lock(sHandleMutexp) ; if (sActiveHandles.find(handle) != sActiveHandles.end()) { + LL_CHECK_MEMORY sActiveHandles.erase(handle); - + LL_CHECK_MEMORY if(sFreeHandles.size() < MAX_NUM_FREE_HANDLES) { - sFreeHandles.insert(handle); - } - else - { + sFreeHandles.insert(handle); + LL_CHECK_MEMORY + } + else + { LLCurl::deleteEasyHandle(handle) ; + LL_CHECK_MEMORY } } else @@ -315,9 +301,7 @@ LLCurl::Easy* LLCurl::Easy::getEasy() LLCurl::Easy::~Easy() { - LL_CHECK_MEMORY releaseEasyHandle(mCurlEasyHandle); - LL_CHECK_MEMORY --gCurlEasyCount; curl_slist_free_all(mHeaders); LL_CHECK_MEMORY @@ -331,7 +315,6 @@ LLCurl::Easy::~Easy() LL_CHECK_MEMORY } mResponder = NULL; - LL_CHECK_MEMORY } void LLCurl::Easy::resetState() @@ -617,6 +600,7 @@ void LLCurl::Multi::cleanup(bool deleted) if(deleted) { easy->mResponder = NULL ; //avoid triggering mResponder. + LL_CHECK_MEMORY } delete easy; LL_CHECK_MEMORY @@ -1623,7 +1607,9 @@ void LLCurl::deleteEasyHandle(CURL* handle) if(handle) { LLMutexLock lock(sHandleMutexp) ; + LL_CHECK_MEMORY curl_easy_cleanup(handle) ; + LL_CHECK_MEMORY sTotalHandles-- ; } } diff --git a/indra/llmessage/llcurl.h b/indra/llmessage/llcurl.h index d6a7714d4c..62947bdb9d 100644 --- a/indra/llmessage/llcurl.h +++ b/indra/llmessage/llcurl.h @@ -44,6 +44,8 @@ #include "llthread.h" #include "llqueuedthread.h" #include "llframetimer.h" +#include "llpointer.h" + class LLMutex; class LLCurlThread; @@ -67,7 +69,7 @@ public: F64 mSpeedDownload; }; - class Responder + class Responder : public LLThreadSafeRefCount { //LOG_CLASS(Responder); public: @@ -126,13 +128,10 @@ public: return false; } - public: /* but not really -- don't touch this */ - U32 mReferenceCount; - private: std::string mURL; }; - typedef boost::intrusive_ptr ResponderPtr; + typedef LLPointer ResponderPtr; /** @@ -378,12 +377,6 @@ private: void cleanupMulti(LLCurl::Multi* multi) ; } ; -namespace boost -{ - void intrusive_ptr_add_ref(LLCurl::Responder* p); - void intrusive_ptr_release(LLCurl::Responder* p); -}; - class LLCurlRequest { diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index fa68b9322e..2b6d6d15fa 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1888,8 +1888,6 @@ bool LLAppViewer::cleanup() delete sTextureCache; sTextureCache = NULL; - delete sTextureFetch; - sTextureFetch = NULL; delete sImageDecodeThread; sImageDecodeThread = NULL; delete mFastTimerLogThread; @@ -1962,6 +1960,10 @@ bool LLAppViewer::cleanup() LLCurl::cleanupClass(); LL_CHECK_MEMORY + //MUST happen AFTER LLCurl::cleanupClass + delete sTextureFetch; + sTextureFetch = NULL; + // If we're exiting to launch an URL, do that here so the screen // is at the right resolution before we launch IE. if (!gLaunchFileOnQuit.empty()) diff --git a/indra/newview/llassetuploadresponders.cpp b/indra/newview/llassetuploadresponders.cpp index 65bfc990d1..7b2c536f5a 100644 --- a/indra/newview/llassetuploadresponders.cpp +++ b/indra/newview/llassetuploadresponders.cpp @@ -919,7 +919,7 @@ public: bool uploadConfirmationCallback( const LLSD& notification, const LLSD& response, - boost::intrusive_ptr responder) + LLPointer responder) { S32 option; std::string confirmation_url; @@ -949,7 +949,7 @@ public: void confirmUpload( const std::string& confirmation_url, - boost::intrusive_ptr responder) + LLPointer responder) { if ( getFilename().empty() ) { @@ -1124,7 +1124,7 @@ void LLNewAgentInventoryVariablePriceResponder::showConfirmationDialog( // and cause sadness. mImpl->confirmUpload( confirmation_url, - boost::intrusive_ptr(this)); + LLPointer(this)); } else { @@ -1157,7 +1157,7 @@ void LLNewAgentInventoryVariablePriceResponder::showConfirmationDialog( mImpl, _1, _2, - boost::intrusive_ptr(this))); + LLPointer(this))); } } diff --git a/indra/newview/lleventpoll.cpp b/indra/newview/lleventpoll.cpp index 4f4d9a40b4..2c786b7f8b 100644 --- a/indra/newview/lleventpoll.cpp +++ b/indra/newview/lleventpoll.cpp @@ -86,7 +86,7 @@ namespace class LLEventPollEventTimer : public LLEventTimer { - typedef boost::intrusive_ptr EventPollResponderPtr; + typedef LLPointer EventPollResponderPtr; public: LLEventPollEventTimer(F32 period, EventPollResponderPtr responder) diff --git a/indra/newview/llfloatertos.cpp b/indra/newview/llfloatertos.cpp index c5df7e16e9..a242b224cd 100644 --- a/indra/newview/llfloatertos.cpp +++ b/indra/newview/llfloatertos.cpp @@ -71,9 +71,9 @@ class LLIamHere : public LLHTTPClient::Responder public: - static boost::intrusive_ptr< LLIamHere > build( LLFloaterTOS* parent ) + static LLIamHere* build( LLFloaterTOS* parent ) { - return boost::intrusive_ptr< LLIamHere >( new LLIamHere( parent ) ); + return new LLIamHere( parent ); }; virtual void setParent( LLFloaterTOS* parentIn ) @@ -102,7 +102,7 @@ class LLIamHere : public LLHTTPClient::Responder // this is global and not a class member to keep crud out of the header file namespace { - boost::intrusive_ptr< LLIamHere > gResponsePtr = 0; + LLPointer< LLIamHere > gResponsePtr = 0; }; BOOL LLFloaterTOS::postBuild() diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp index 7e6dfbc9d9..a5eebf6c77 100644 --- a/indra/newview/lltexturefetch.cpp +++ b/indra/newview/lltexturefetch.cpp @@ -2944,7 +2944,9 @@ TFReqSendMetrics::doWork(LLTextureFetch * fetcher) ~lcl_responder() { + LL_CHECK_MEMORY mFetcher->decrCurlPOSTCount(); + LL_CHECK_MEMORY } // virtual diff --git a/indra/newview/lltranslate.h b/indra/newview/lltranslate.h index c58e1adb8c..db5ad9479c 100755 --- a/indra/newview/lltranslate.h +++ b/indra/newview/lltranslate.h @@ -263,8 +263,8 @@ public : EService mService; }; - typedef boost::intrusive_ptr TranslationReceiverPtr; - typedef boost::intrusive_ptr KeyVerificationReceiverPtr; + typedef LLPointer TranslationReceiverPtr; + typedef LLPointer KeyVerificationReceiverPtr; /** * Translate given text. diff --git a/indra/newview/llviewermessage.cpp b/indra/newview/llviewermessage.cpp index 7d7d1f3047..04c7a87374 100755 --- a/indra/newview/llviewermessage.cpp +++ b/indra/newview/llviewermessage.cpp @@ -3339,9 +3339,9 @@ public : { } - static boost::intrusive_ptr build(const std::string &from_lang, const std::string &to_lang, const std::string &mesg, const LLChat &chat, const LLSD &toast_args) + static ChatTranslationReceiver* build(const std::string &from_lang, const std::string &to_lang, const std::string &mesg, const LLChat &chat, const LLSD &toast_args) { - return boost::intrusive_ptr(new ChatTranslationReceiver(from_lang, to_lang, mesg, chat, toast_args)); + return new ChatTranslationReceiver(from_lang, to_lang, mesg, chat, toast_args); } protected: diff --git a/indra/newview/llviewerregion.cpp b/indra/newview/llviewerregion.cpp index 493195aaa3..1ef729e2ff 100644 --- a/indra/newview/llviewerregion.cpp +++ b/indra/newview/llviewerregion.cpp @@ -255,10 +255,9 @@ public: } } - static boost::intrusive_ptr build( U64 region_handle, S32 id ) + static BaseCapabilitiesComplete* build( U64 region_handle, S32 id ) { - return boost::intrusive_ptr( - new BaseCapabilitiesComplete(region_handle, id) ); + return new BaseCapabilitiesComplete(region_handle, id); } private: diff --git a/indra/viewer_components/updater/llupdatechecker.cpp b/indra/viewer_components/updater/llupdatechecker.cpp index 4da774a5f6..5edbbf9914 100644 --- a/indra/viewer_components/updater/llupdatechecker.cpp +++ b/indra/viewer_components/updater/llupdatechecker.cpp @@ -51,37 +51,6 @@ public: }; -class LLUpdateChecker::Implementation: - public LLHTTPClient::Responder -{ -public: - Implementation(Client & client); - ~Implementation(); - void checkVersion(std::string const & protocolVersion, std::string const & hostUrl, - std::string const & servicePath, std::string channel, std::string version); - - // Responder: - virtual void completed(U32 status, - const std::string & reason, - const LLSD& content); - virtual void error(U32 status, const std::string & reason); - -private: - static const char * sProtocolVersion; - - Client & mClient; - LLHTTPClient mHttpClient; - bool mInProgress; - std::string mVersion; - - std::string buildUrl(std::string const & protocolVersion, std::string const & hostUrl, - std::string const & servicePath, std::string channel, std::string version); - - LOG_CLASS(LLUpdateChecker::Implementation); -}; - - - // LLUpdateChecker //----------------------------------------------------------------------------- @@ -134,13 +103,7 @@ void LLUpdateChecker::Implementation::checkVersion(std::string const & protocolV std::string checkUrl = buildUrl(protocolVersion, hostUrl, servicePath, channel, version); LL_INFOS("UpdateCheck") << "checking for updates at " << checkUrl << llendl; - // The HTTP client will wrap a raw pointer in a boost::intrusive_ptr causing the - // passed object to be silently and automatically deleted. We pass a self- - // referential intrusive pointer to which we add a reference to keep the - // client from deleting the update checker implementation instance. - LLHTTPClient::ResponderPtr temporaryPtr(this); - boost::intrusive_ptr_add_ref(temporaryPtr.get()); - mHttpClient.get(checkUrl, temporaryPtr); + mHttpClient.get(checkUrl, this); } void LLUpdateChecker::Implementation::completed(U32 status, diff --git a/indra/viewer_components/updater/llupdatechecker.h b/indra/viewer_components/updater/llupdatechecker.h index d882169068..23f62a7c5e 100644 --- a/indra/viewer_components/updater/llupdatechecker.h +++ b/indra/viewer_components/updater/llupdatechecker.h @@ -29,6 +29,7 @@ #include +#include "llhttpclient.h" // // Implements asynchronous checking for updates. @@ -36,7 +37,36 @@ class LLUpdateChecker { public: class Client; - class Implementation; + class Implementation: + + public LLHTTPClient::Responder + { + public: + Implementation(Client & client); + ~Implementation(); + void checkVersion(std::string const & protocolVersion, std::string const & hostUrl, + std::string const & servicePath, std::string channel, std::string version); + + // Responder: + virtual void completed(U32 status, + const std::string & reason, + const LLSD& content); + virtual void error(U32 status, const std::string & reason); + + private: + static const char * sProtocolVersion; + + Client & mClient; + LLHTTPClient mHttpClient; + bool mInProgress; + std::string mVersion; + + std::string buildUrl(std::string const & protocolVersion, std::string const & hostUrl, + std::string const & servicePath, std::string channel, std::string version); + + LOG_CLASS(LLUpdateChecker::Implementation); + }; + // An exception that may be raised on check errors. class CheckError; @@ -48,7 +78,7 @@ public: std::string const & servicePath, std::string channel, std::string version); private: - boost::shared_ptr mImplementation; + LLPointer mImplementation; }; -- cgit v1.3