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.
This commit is contained in:
Aleric Inglewood
2012-07-15 22:46:38 +02:00
parent a6bb2604f6
commit 14e5b46687
8 changed files with 69 additions and 23 deletions

View File

@@ -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;

View File

@@ -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<CurlEasyRequest> {
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<CurlResponderBuffer>::ptr()) CurlResponderBuffer; }
/*virtual*/ bool isBuffered(void) const { return true; }
};
// The curl easy request type wrapped in a reference counting pointer.

View File

@@ -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 ;
}

View File

@@ -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:
/**

View File

@@ -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] <<llendl;
#endif
chain_t::const_iterator it = chain.begin();
chain_t::const_iterator end = chain.end();
it = chain.begin();
for(; it != end; ++it)
{
link.mPipe = (*it);
link.mChannels = info.mData->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);

View File

@@ -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;

View File

@@ -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 ;
}

View File

@@ -190,7 +190,8 @@ public:
*/
void allowCookies();
/*virtual*/ bool isValid() ;
/*virtual*/ bool hasExpiration(void) const;
/*virtual*/ bool hasNotExpired(void) const;
public:
/**