diff --git a/indra/llcommon/aiframetimer.cpp b/indra/llcommon/aiframetimer.cpp index 66da2f02c..a92343d5b 100644 --- a/indra/llcommon/aiframetimer.cpp +++ b/indra/llcommon/aiframetimer.cpp @@ -31,9 +31,114 @@ #include "aiframetimer.h" +static F64 const NEVER = 1e16; // 317 million years. + F64 AIFrameTimer::sNextExpiration; +AIFrameTimer::timer_list_type AIFrameTimer::sTimerList; +LLMutex AIFrameTimer::sMutex; + +// Notes on thread-safety of AIRunningFrameTimer (continued from aiframetimer.h) +// +// Most notably, the constructor and init() should be called as follows: +// 1) The object is constructed (AIRunningFrameTimer::AIRunningFrameTimer). +// 2) The lock is obtained. +// 3) The object is inserted in the list (operator<(AIRunningFrameTimer const&, AIRunningFrameTimer const&)). +// 4) The object is initialized (AIRunningFrameTimer::init). +// 5) The lock is released. +// This assures that the object is not yet shared at the moment +// that it is initialized (assignment to mConnection is not thread-safe). + +void AIFrameTimer::create(F64 expiration, signal_type::slot_type const& slot) +{ + AIRunningFrameTimer new_timer(expiration, this); + sMutex.lock(); + llassert(mHandle.mRunningTimer == sTimerList.end()); // Create may only be called when the timer isn't already running. + mHandle.init(sTimerList.insert(new_timer), slot); + sNextExpiration = sTimerList.begin()->expiration(); + sMutex.unlock(); +} + +void AIFrameTimer::cancel(void) +{ + mHandle.mMutex.lock(); + sMutex.lock(); + mHandle.mMutex.unlock(); + if (mHandle.mRunningTimer != sTimerList.end()) + { + timer_list_type::iterator running_timer = mHandle.mRunningTimer; + mHandle.mRunningTimer = sTimerList.end(); + sTimerList.erase(running_timer); + sNextExpiration = sTimerList.empty() ? NEVER : sTimerList.begin()->expiration(); + } + sMutex.unlock(); +} void AIFrameTimer::handleExpiration(F64 current_frame_time) { + sMutex.lock(); + for(;;) + { + if (sTimerList.empty()) + { + // No running timers left. + sNextExpiration = NEVER; + break; + } + timer_list_type::iterator running_timer = sTimerList.begin(); + sNextExpiration = running_timer->expiration(); + if (sNextExpiration > current_frame_time) + { + // Didn't expire yet. + break; + } + + // Obtain handle of running timer through the associated AIFrameTimer object. + // Note that if the AIFrameTimer object was destructed (when running_timer->getTimer() + // would return an invalid pointer) then it called cancel(), so we can't be here. + Handle& handle(running_timer->getTimer()->mHandle); + llassert_always(running_timer == handle.mRunningTimer); + + // We're going to erase this timer, so stop cancel() from doing the same. + handle.mRunningTimer = sTimerList.end(); + + // We keep handle.mMutex during the callback to prevent the thread that + // owns the AIFrameTimer from deleting the callback function while we + // call it: in order to do so it first has to call cancel(), which will + // block until we release this mutex again, or we won't call the callback + // function here because the trylock fails. + // + // Assuming the main thread arrived here, there are two possible states + // for the other thread that tries to delete the call back function, + // right after calling the cancel() function too: + // + // 1. It hasn't obtained the first lock yet, we obtain the handle.mMutex + // lock and the other thread will stall on the first line of cancel(). + // After do_callback returns, the other thread will do nothing because + // handle.mRunningTimer equals sTimerList.end(), exit the function and + // (possibly) delete the callback object, but that is ok as we already + // returned from the callback function. + // + // 2. It already called cancel() and hangs on the second line trying to + // obtain sMutex.lock(). The trylock below fails and we never call the + // callback function. We erase the running timer here and release sMutex + // at the end, after which the other thread does nothing because + // handle.mRunningTimer equals sTimerList.end(), exits the function and + // (possibly) deletes the callback object. + // + // Note that if the other thread actually obtained the sMutex and + // released handle.mMutex again, then we can't be here: this is still + // inside the critical area of sMutex. + if (handle.mMutex.tryLock()) // If this fails then another thread is in the process of cancelling this timer, so do nothing. + { + sMutex.unlock(); + running_timer->do_callback(); // May not throw exceptions. + sMutex.lock(); + handle.mMutex.unlock(); // Allow other thread to return from cancel() and possibly delete the callback object. + } + + // Erase the timer from the running list. + sTimerList.erase(running_timer); + } + sMutex.unlock(); } diff --git a/indra/llcommon/aiframetimer.h b/indra/llcommon/aiframetimer.h index c3e380d69..b8d287f6e 100644 --- a/indra/llcommon/aiframetimer.h +++ b/indra/llcommon/aiframetimer.h @@ -32,23 +32,108 @@ #define AIFRAMETIMER_H #include "llframetimer.h" +#include "llthread.h" +#include +#include class LL_COMMON_API AIFrameTimer { - private: - F64 mExpire; // Time at which the timer expires, in seconds since application start (compared to LLFrameTimer::sFrameTime). - static std::set sTimerList; // List with all running timers. + protected: + typedef boost::signals2::signal signal_type; - friend class LLFrameTimer; + private: + // Use separate struct for this object because it is non-copyable. + struct Signal { + signal_type mSignal; + }; + + // Notes on Thread-Safety + // + // This is the type of the objects stored in AIFrameTimer::sTimerList, and as such leans + // for it's thread-safety on the same lock as is used for that std::multiset as follows. + // An arbitrary thread can create, insert and initialize this object. Other threads can + // not access it until that has completed. + // + // After creation two threads can access it: the thread that created it (owns the + // AIFrameTimer object, which has an mHandle that points to this object), or the main + // thread by finding it in sTimerList. + // + // See aiframetimer.cpp for more notes. + class AIRunningFrameTimer { + private: + F64 mExpire; // Time at which the timer expires, in seconds since application start (compared to LLFrameTimer::sFrameTime). + Signal* mCallback; + AIFrameTimer* mTimer; + + public: + AIRunningFrameTimer(F64 expiration, AIFrameTimer* timer) : mExpire(LLFrameTimer::getElapsedSeconds() + expiration), mCallback(new Signal), mTimer(timer) { } + ~AIRunningFrameTimer() { delete mCallback; } + void init(signal_type::slot_type const& slot) const { mCallback->mSignal.connect(slot); } + + friend bool operator<(AIRunningFrameTimer const& ft1, AIRunningFrameTimer const& ft2) { return ft1.mExpire < ft2.mExpire; } + + void do_callback(void) const { mCallback->mSignal(); } + F64 expiration(void) const { return mExpire; } + AIFrameTimer* getTimer(void) const { return mTimer; } + }; + + typedef std::multiset timer_list_type; + + static LLMutex sMutex; // Mutex for the two global variables below. + static timer_list_type sTimerList; // List with all running timers. static F64 sNextExpiration; // Cache of smallest value in sTimerList. + friend class LLFrameTimer; // Access to sNextExpiration. + + class Handle { + public: + timer_list_type::iterator mRunningTimer; // Points to the running timer, or to sTimerList.end() when not running. + // Access to this iterator is protected by the AIFrameTimer::sMutex! + LLMutex mMutex; // A mutex used to protect us from deletion of the callback object while + // calling the callback function in the case of simultaneous expiration + // and cancellation by the thread owning the AIFrameTimer (by calling + // AIFrameTimer::cancel). + + // Construction for a not-running timer. + Handle(void) : mRunningTimer(sTimerList.end()) { } + + // Actual initialization used by AIFrameTimer::create. + void init(timer_list_type::iterator const& running_timer, signal_type::slot_type const& slot) + { + // Locking AIFrameTimer::sMutex is not neccessary here, because we're creating + // the object and no other thread knows of mRunningTimer at this point. + mRunningTimer = running_timer; + mRunningTimer->init(slot); + } + + private: + // LLMutex has no assignment operator. + Handle& operator=(Handle const&) { return *this; } + }; + + Handle mHandle; public: - AIFrameTimer(F64 expiration) : mExpire(LLFrameTimer::getElapsedSeconds() + expiration) { } + // Construct an AIFrameTimer that is not initialized. + // + // The call to end() is deliberately not put inside a critical area because it's a read-only + // operation on something that is never written to while there are threads. + AIFrameTimer(void) { } + // Construction of a running AIFrameTimer with expiration time expiration in seconds, and callback slot. + AIFrameTimer(F64 expiration, signal_type::slot_type const& slot) { create(expiration, slot); } + + // Destructing the AIFrameTimer object terminates the running timer (if still running). + // Note that cancel() must have returned BEFORE anything is destructed that would disallow the callback function to be called. + // So, if the AIFrameTimer is a member of an object whose callback function is called then cancel() has + // be called manually (or from the destructor of THAT object), before that object is destructed. + // Cancel may be called multiple times. + ~AIFrameTimer() { cancel(); } + + void create(F64 expiration, signal_type::slot_type const& slot); + void cancel(void); + + protected: static void handleExpiration(F64 current_frame_time); - - friend bool operator<(AIFrameTimer const& ft1, AIFrameTimer const& ft2) { return ft1.mExpire < ft2.mExpire; } }; - #endif diff --git a/indra/llcommon/llframetimer.cpp b/indra/llcommon/llframetimer.cpp index 73059eca3..b46cca053 100644 --- a/indra/llcommon/llframetimer.cpp +++ b/indra/llcommon/llframetimer.cpp @@ -41,6 +41,7 @@ // Local constants. static F64 const USEC_PER_SECOND = 1000000.0; static F64 const USEC_TO_SEC_F64 = 0.000001; +static F64 const NEVER = 1e16; // Static members U64 const LLFrameTimer::sStartTotalTime = totalTime(); // Application start in microseconds since epoch. @@ -59,6 +60,7 @@ apr_thread_mutex_t* LLFrameTimer::sGlobalMutex; void LLFrameTimer::global_initialization(void) { apr_thread_mutex_create(&sGlobalMutex, APR_THREAD_MUTEX_UNNESTED, AIAPRRootPool::get()()); + AIFrameTimer::sNextExpiration = NEVER; } // static