diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index b6d35f1db..b0cc64e59 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -99,11 +99,21 @@ void *APR_THREAD_FUNC LLThread::staticRun(apr_thread_t *apr_threadp, void *datap // Run the user supplied function threadp->run(); - llinfos << "LLThread::staticRun() Exiting: " << threadp->mName << llendl; + // Setting mStatus to STOPPED is done non-thread-safe, so it's + // possible that the thread is deleted by another thread at + // the moment it happens... therefore make a copy here. + char const* name = threadp->mName; // We're done with the run function, this thread is done executing now. threadp->mStatus = STOPPED; + // Only now print this info [doing that before setting mStatus + // to STOPPED makes it much more likely that another thread runs + // after the LLCurl::Multi::run() function exits and we actually + // change this variable (which really SHOULD have been inside + // the critical area of the mSignal lock)]. + llinfos << "LLThread::staticRun() Exiting: " << name << llendl; + return NULL; } @@ -432,18 +442,8 @@ LLCondition::~LLCondition() mAPRCondp = NULL; } - void LLCondition::wait() { - if (!isLocked()) - { //mAPRMutexp MUST be locked before calling apr_thread_cond_wait - apr_thread_mutex_lock(mAPRMutexp); -#if MUTEX_DEBUG - // avoid asserts on destruction in non-release builds - U32 id = LLThread::currentID(); - mIsLocked[id] = TRUE; -#endif - } apr_thread_cond_wait(mAPRCondp, mAPRMutexp); } diff --git a/indra/llmessage/llcurl.cpp b/indra/llmessage/llcurl.cpp index 6fe9dea2c..524c0ef17 100644 --- a/indra/llmessage/llcurl.cpp +++ b/indra/llmessage/llcurl.cpp @@ -722,14 +722,17 @@ CURLMsg* LLCurl::Multi::info_read(S32* msgs_in_queue) void LLCurl::Multi::perform() { + mSignal->lock(); if (mPerformState == PERFORM_STATE_READY) { mSignal->signal(); } + mSignal->unlock(); } void LLCurl::Multi::run() { + mSignal->lock(); while (!mQuitting) { mSignal->wait(); @@ -753,6 +756,7 @@ void LLCurl::Multi::run() mPerformState = PERFORM_STATE_COMPLETED; } } + mSignal->unlock(); } S32 LLCurl::Multi::process() @@ -879,12 +883,16 @@ LLCurlRequest::~LLCurlRequest() for (curlmulti_set_t::iterator iter = mMultiSet.begin(); iter != mMultiSet.end(); ++iter) { LLCurl::Multi* multi = *iter; + multi->mSignal->lock(); multi->mQuitting = true; while (!multi->isStopped()) { multi->mSignal->signal(); + multi->mSignal->unlock(); apr_sleep(1000); + multi->mSignal->lock(); } + multi->mSignal->unlock(); } for_each(mMultiSet.begin(), mMultiSet.end(), DeletePointer()); } @@ -1023,12 +1031,16 @@ S32 LLCurlRequest::process() if (multi != mActiveMulti && tres == 0 && multi->mQueued == 0) { mMultiSet.erase(curiter); + multi->mSignal->lock(); multi->mQuitting = true; while (!multi->isStopped()) { multi->mSignal->signal(); + multi->mSignal->unlock(); apr_sleep(1000); + multi->mSignal->unlock(); } + multi->mSignal->unlock(); delete multi; } @@ -1071,12 +1083,16 @@ LLCurlEasyRequest::LLCurlEasyRequest() LLCurlEasyRequest::~LLCurlEasyRequest() { + mMulti->mSignal->lock(); mMulti->mQuitting = true; while (!mMulti->isStopped()) { mMulti->mSignal->signal(); + mMulti->mSignal->unlock(); apr_sleep(1000); + mMulti->mSignal->lock(); } + mMulti->mSignal->unlock(); delete mMulti; }