From 14e5b4668734e15ff15daeaf1cd70c73d63aa5fc Mon Sep 17 00:00:00 2001 From: Aleric Inglewood Date: Sun, 15 Jul 2012 22:46:38 +0200 Subject: [PATCH] Fixed and adjusted remainders of isValid() code. Note that in the code, and still, has_curl_request was always false. However, instead of deleting all code paths that are only executed when has_curl_request would be true, I fixed the code to work as intended with my current implementation; which also results in LLCurlRequests to never expire. This way things won't break unexpectedly when this ever changes. Since on this branch isValid was only called still (the rest was removed already) to check if the curl download expired, I took the liberty to rename isValid to hasNotExpired. --- indra/llmessage/aicurl.h | 3 +++ indra/llmessage/aicurlprivate.h | 8 ++++++++ indra/llmessage/lliopipe.cpp | 9 ++++++++- indra/llmessage/lliopipe.h | 11 ++++++++++- indra/llmessage/llpumpio.cpp | 32 ++++++++++++++++++++------------ indra/llmessage/llpumpio.h | 5 ++--- indra/llmessage/llurlrequest.cpp | 21 ++++++++++++++++----- indra/llmessage/llurlrequest.h | 3 ++- 8 files changed, 69 insertions(+), 23 deletions(-) diff --git a/indra/llmessage/aicurl.h b/indra/llmessage/aicurl.h index a9a2a9d64..ac89fdac3 100644 --- a/indra/llmessage/aicurl.h +++ b/indra/llmessage/aicurl.h @@ -274,6 +274,9 @@ class AICurlEasyRequest { // Queue a command to remove this request from the multi session (or cancel a queued command to add it). void removeRequest(void); + // Returns true when this AICurlEasyRequest wraps a AICurlPrivate::ThreadSafeBufferedCurlEasyRequest. + bool isBuffered(void) const { return mCurlEasyRequest->isBuffered(); } + private: // The actual pointer to the ThreadSafeCurlEasyRequest instance. AICurlPrivate::CurlEasyRequestPtr mCurlEasyRequest; diff --git a/indra/llmessage/aicurlprivate.h b/indra/llmessage/aicurlprivate.h index 892228bae..0bed1de7f 100644 --- a/indra/llmessage/aicurlprivate.h +++ b/indra/llmessage/aicurlprivate.h @@ -318,6 +318,10 @@ class CurlResponderBuffer : protected AICurlEasyHandleEvents { public: // Return pointer to the ThreadSafe (wrapped) version of this object. ThreadSafeBufferedCurlEasyRequest* get_lockobj(void); + + // Return true when prepRequest was already called and the object has not been + // invalidated as a result of calling timed_out(). + bool isValid(void) const { return mResponder; } }; // This class wraps CurlEasyRequest for thread-safety and adds a reference counter so we can @@ -333,6 +337,8 @@ class ThreadSafeCurlEasyRequest : public AIThreadSafeSimple { virtual ~ThreadSafeCurlEasyRequest() { Dout(dc::curl, "Destructing ThreadSafeCurlEasyRequest with this = " << (void*)this); } + /*virtual*/ bool isBuffered(void) const { return false; } + private: LLAtomicU32 mReferenceCount; @@ -351,6 +357,8 @@ class ThreadSafeBufferedCurlEasyRequest : public ThreadSafeCurlEasyRequest, publ public: // Throws AICurlNoEasyHandle. ThreadSafeBufferedCurlEasyRequest(void) { new (AIThreadSafeSimple::ptr()) CurlResponderBuffer; } + + /*virtual*/ bool isBuffered(void) const { return true; } }; // The curl easy request type wrapped in a reference counting pointer. diff --git a/indra/llmessage/lliopipe.cpp b/indra/llmessage/lliopipe.cpp index 8f827f7a3..c8bef05c6 100644 --- a/indra/llmessage/lliopipe.cpp +++ b/indra/llmessage/lliopipe.cpp @@ -76,7 +76,14 @@ LLIOPipe::~LLIOPipe() } //virtual -bool LLIOPipe::isValid() +bool LLIOPipe::hasExpiration(void) const +{ + // LLIOPipe::hasNotExpired always returns true. + return false; +} + +//virtual +bool LLIOPipe::hasNotExpired(void) const { return true ; } diff --git a/indra/llmessage/lliopipe.h b/indra/llmessage/lliopipe.h index 2def3229f..86ae7e425 100644 --- a/indra/llmessage/lliopipe.h +++ b/indra/llmessage/lliopipe.h @@ -231,7 +231,16 @@ public: */ virtual ~LLIOPipe(); - virtual bool isValid() ; + /** + * @brief External expiration facility. + * + * If hasExpiration() returns true, then we need to check hasNotExpired() + * to see if the LLIOPipe is still valid. In the legacy LL code the + * latter was called isValid() and was overloaded for two purposes: + * either expiration or failure to initialize. + */ + virtual bool hasExpiration(void) const; + virtual bool hasNotExpired(void) const; protected: /** diff --git a/indra/llmessage/llpumpio.cpp b/indra/llmessage/llpumpio.cpp index b498e5e2b..c249d7132 100644 --- a/indra/llmessage/llpumpio.cpp +++ b/indra/llmessage/llpumpio.cpp @@ -198,19 +198,25 @@ LLPumpIO::~LLPumpIO() } } -bool LLPumpIO::addChain(const chain_t& chain, F32 timeout, bool has_curl_request) +bool LLPumpIO::addChain(chain_t const& chain, F32 timeout) { LLMemType m1(LLMemType::MTYPE_IO_PUMP); - if(chain.empty()) return false; -#if LL_THREADS_APR - LLScopedLock lock(mChainsMutex); -#endif + chain_t::const_iterator it = chain.begin(); + chain_t::const_iterator const end = chain.end(); + if (it == end) return false; + LLChainInfo info; - info.mHasCurlRequest = has_curl_request; + for(; it != end; ++it) + { + if ((*it)->hasExpiration()) + { + info.mHasExpiration = true; + break; + } + } info.setTimeoutSeconds(timeout); info.mData = LLIOPipe::buffer_ptr_t(new LLBufferArray); - info.mData->setThreaded(has_curl_request); LLLinkInfo link; #if LL_DEBUG_PIPE_TYPE_IN_PUMP lldebugs << "LLPumpIO::addChain() " << chain[0] << " '" @@ -218,14 +224,16 @@ bool LLPumpIO::addChain(const chain_t& chain, F32 timeout, bool has_curl_request #else lldebugs << "LLPumpIO::addChain() " << chain[0] <nextChannel(); info.mChainLinks.push_back(link); } +#if LL_THREADS_APR + LLScopedLock lock(mChainsMutex); +#endif mPendingChains.push_back(info); return true; } @@ -1086,14 +1094,14 @@ void LLPumpIO::processChain(LLChainInfo& chain) bool LLPumpIO::isChainExpired(LLChainInfo& chain) { - if(!chain.mHasCurlRequest) + if(!chain.mHasExpiration) { return false ; } for(links_t::iterator iter = chain.mChainLinks.begin(); iter != chain.mChainLinks.end(); ++iter) { - if(!(*iter).mPipe->isValid()) + if(!(*iter).mPipe->hasNotExpired()) { return true ; } @@ -1168,7 +1176,7 @@ LLPumpIO::LLChainInfo::LLChainInfo() : mInit(false), mLock(0), mEOS(false), - mHasCurlRequest(false), + mHasExpiration(false), mDescriptorsPool(new LLAPRPool(LLThread::tldata().mRootPool)) { LLMemType m1(LLMemType::MTYPE_IO_PUMP); diff --git a/indra/llmessage/llpumpio.h b/indra/llmessage/llpumpio.h index 3771be9cf..0d1387257 100644 --- a/indra/llmessage/llpumpio.h +++ b/indra/llmessage/llpumpio.h @@ -100,10 +100,9 @@ public: * @param chain The pipes for the chain * @param timeout The number of seconds in the future to * expire. Pass in 0.0f to never expire. - * @param has_curl_request The chain contains LLURLRequest if true. * @return Returns true if anything was added to the pump. */ - bool addChain(const chain_t& chain, F32 timeout, bool has_curl_request = false); + bool addChain(const chain_t& chain, F32 timeout); /** * @brief Struct to associate a pipe with it's buffer io indexes. @@ -347,7 +346,7 @@ protected: // basic member data bool mInit; bool mEOS; - bool mHasCurlRequest; + bool mHasExpiration; S32 mLock; LLFrameTimer mTimer; links_t::iterator mHead; diff --git a/indra/llmessage/llurlrequest.cpp b/indra/llmessage/llurlrequest.cpp index 97a8e48d9..9d5b18074 100644 --- a/indra/llmessage/llurlrequest.cpp +++ b/indra/llmessage/llurlrequest.cpp @@ -260,11 +260,22 @@ void LLURLRequest::allowCookies() } //virtual -bool LLURLRequest::isValid() +bool LLURLRequest::hasExpiration(void) const { - //FIXME - wtf is with this isValid? - //return mDetail->mCurlRequest->isValid(); - return true; + // Currently, this ALWAYS returns false -- because only AICurlEasyRequestStateMachine uses buffered + // AICurlEasyRequest objects, and LLURLRequest uses (unbuffered) AICurlEasyRequest directly, which + // have no expiration facility. + return mDetail->mCurlEasyRequest.isBuffered(); +} + +//virtual +bool LLURLRequest::hasNotExpired(void) const +{ + if (!mDetail->mCurlEasyRequest.isBuffered()) + return true; + AICurlEasyRequest_wat buffered_easy_request_w(*mDetail->mCurlEasyRequest); + AICurlResponderBuffer_wat buffer_w(*mDetail->mCurlEasyRequest); + return buffer_w->isValid(); } // virtual @@ -274,7 +285,7 @@ LLIOPipe::EStatus LLURLRequest::handleError( { LLMemType m1(LLMemType::MTYPE_IO_URL_REQUEST); - if(!isValid()) + if(!hasNotExpired()) { return STATUS_EXPIRED ; } diff --git a/indra/llmessage/llurlrequest.h b/indra/llmessage/llurlrequest.h index 6cfe83a5e..d265a0dae 100644 --- a/indra/llmessage/llurlrequest.h +++ b/indra/llmessage/llurlrequest.h @@ -190,7 +190,8 @@ public: */ void allowCookies(); - /*virtual*/ bool isValid() ; + /*virtual*/ bool hasExpiration(void) const; + /*virtual*/ bool hasNotExpired(void) const; public: /**