From b22f89c9fa9e6ee95b552b27808df77f710caad6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 23 Nov 2019 22:18:45 -0500 Subject: DRTVWR-494: Improve thread safety of LLSingleton machinery. Remove warnings about LLSingleton not being thread-safe because, at this point, we have devoted considerable effort to trying to make it thread-safe. Add LLSingleton::Locker, a nested class which both provides a function- static mutex and a scoped lock that uses it. Instantiating Locker, which has a nullary constructor, replaces the somewhat cumbersome idiom of declaring a std::unique_lock lk(getMutex); This eliminates (or rather, absorbs) the typedefs and getMutex() method from LLParamSingleton. Replace explicit std::unique_lock declarations in LLParamSingleton methods with Locker declarations. Remove LLSingleton::SingletonInitializer nested struct. Instead of getInstance() relying on function-static initialization to protect (only) constructSingleton() calls, explicitly use a Locker instance to cover its whole scope, and make the UNINITIALIZED case call constructSingleton(). Rearrange cases so that after constructSingleton(), control falls through to the CONSTRUCTED case and the finishInitializing() call. Use a Locker instance in other public-facing methods too: instanceExists(), wasDeleted(), ~LLSingleton(). Make destructor protected so it can only be called via deleteSingleton() (but must be accessible to subclasses for overrides). Remove LLSingletonBase::get_master() and get_initializing(), which permitted directly manipulating the master list and the initializing stack without any locking mechanism. Replace with get_initializing_size(). Similarly, replace LLSingleton_manage_master::get_initializing() with get_initializing_size(). Use in constructSingleton() in place of get_initializing().size(). Remove LLSingletonBase::capture_dependency()'s list_t parameter, which accepted the list returned by get_initializing(). Encapsulate that retrieval within the scope of the new lock in capture_dependency(). Add LLSingleton_manage_master::capture_dependency(LLSingletonBase*, EInitState) to forward (or not) a call to LLSingletonBase::capture_dependency(). Nullary LLSingleton::capture_dependency() calls new LLSingleton_manage_master method. Equip LLSingletonBase::MasterList with a mutex of its own, separate from the one donated by the LLSingleton machinery, to serialize use of MasterList data members. Introduce MasterList::Lock nested class to lock the MasterList mutex while providing a reference to the MasterList instance. Introduce subclasses LockedMaster, which provides a reference to the actual mMaster master list while holding the MasterList lock; and LockedInitializing, which does the same for the initializing list. Make mMaster and get_initializing_() private so that consuming code can *only* access those lists via LockedInitializing and LockedMaster. Make MasterList::cleanup_initializing_() private, with a LockedInitializing public forwarding method. This avoids another call to MasterList::instance(), and also mandates that the lock is currently held during every call. Similarly, move LLSingletonBase::log_initializing() to a LockedInitializing log() method. (transplanted from dca0f16266c7bddedb51ae7d7dca468ba87060d5) --- indra/llcommon/llsingleton.cpp | 151 +++++++++++++++++++++++++++++++---------- 1 file changed, 117 insertions(+), 34 deletions(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index c45c144570..812fd31719 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -31,6 +31,7 @@ #include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" #include "llcoro_get_id.h" +#include "llexception.h" #include #include #include @@ -57,17 +58,59 @@ bool oktolog(); class LLSingletonBase::MasterList: public LLSingleton { +private: LLSINGLETON_EMPTY_CTOR(MasterList); -public: - // No need to make this private with accessors; nobody outside this source - // file can see it. + // Independently of the LLSingleton locks governing construction, + // destruction and other state changes of the MasterList instance itself, + // we must also defend each of the data structures owned by the + // MasterList. + // This must be a recursive_mutex because, while the lock is held for + // manipulating some data in the master list, we must also check whether + // it's safe to log -- which involves querying a different LLSingleton -- + // which requires accessing the master list. + typedef std::recursive_mutex mutex_t; + typedef std::unique_lock lock_t; + + mutex_t mMutex; +public: + // Instantiate this to both obtain a reference to MasterList::instance() + // and lock its mutex for the lifespan of this Lock instance. + class Lock + { + public: + Lock(): + mMasterList(MasterList::instance()), + mLock(mMasterList.mMutex) + {} + Lock(const Lock&) = delete; + Lock& operator=(const Lock&) = delete; + MasterList& get() const { return mMasterList; } + operator MasterList&() const { return get(); } + + protected: + MasterList& mMasterList; + MasterList::lock_t mLock; + }; + +private: // This is the master list of all instantiated LLSingletons (save the // MasterList itself) in arbitrary order. You MUST call dep_sort() before // traversing this list. - LLSingletonBase::list_t mMaster; + list_t mMaster; + +public: + // Instantiate this to obtain a reference to MasterList::mMaster and to + // hold the MasterList lock for the lifespan of this LockedMaster + // instance. + struct LockedMaster: public Lock + { + list_t& get() const { return mMasterList.mMaster; } + operator list_t&() const { return get(); } + }; +private: // We need to maintain a stack of LLSingletons currently being // initialized, either in the constructor or in initSingleton(). However, // managing that as a stack depends on having a DISTINCT 'initializing' @@ -83,10 +126,44 @@ public: // Instead, use a map of llcoro::id to select the appropriate // coro-specific 'initializing' stack. llcoro::get_id() is carefully // implemented to avoid requiring LLCoros. - typedef boost::unordered_map InitializingMap; + typedef boost::unordered_map InitializingMap; InitializingMap mInitializing; - // non-static method, cf. LLSingletonBase::get_initializing() +public: + // Instantiate this to obtain a reference to the coroutine-specific + // initializing list and to hold the MasterList lock for the lifespan of + // this LockedInitializing instance. + struct LockedInitializing: public Lock + { + public: + LockedInitializing(): + // only do the lookup once, cache the result + // note that the lock is already locked during this lookup + mList(&mMasterList.get_initializing_()) + {} + list_t& get() const + { + if (! mList) + { + LLTHROW(std::runtime_error("Trying to use LockedInitializing " + "after cleanup_initializing()")); + } + return *mList; + } + operator list_t&() const { return get(); } + void log(const char* verb, const char* name); + void cleanup_initializing() + { + mMasterList.cleanup_initializing_(); + mList = nullptr; + } + + private: + // Store pointer since cleanup_initializing() must clear it. + list_t* mList; + }; + +private: list_t& get_initializing_() { // map::operator[] has find-or-create semantics, exactly what we need @@ -104,16 +181,12 @@ public: } }; -//static -LLSingletonBase::list_t& LLSingletonBase::get_master() -{ - return LLSingletonBase::MasterList::instance().mMaster; -} - void LLSingletonBase::add_master() { // As each new LLSingleton is constructed, add to the master list. - get_master().push_back(this); + // This temporary LockedMaster should suffice to hold the MasterList lock + // during the push_back() call. + MasterList::LockedMaster().get().push_back(this); } void LLSingletonBase::remove_master() @@ -125,27 +198,32 @@ void LLSingletonBase::remove_master() // master list, and remove this item IF FOUND. We have few enough // LLSingletons, and they are so rarely destroyed (once per run), that the // cost of a linear search should not be an issue. - get_master().remove(this); + // This temporary LockedMaster should suffice to hold the MasterList lock + // during the remove() call. + MasterList::LockedMaster().get().remove(this); } //static -LLSingletonBase::list_t& LLSingletonBase::get_initializing() +LLSingletonBase::list_t::size_type LLSingletonBase::get_initializing_size() { - return LLSingletonBase::MasterList::instance().get_initializing_(); + return MasterList::LockedInitializing().get().size(); } LLSingletonBase::~LLSingletonBase() {} void LLSingletonBase::push_initializing(const char* name) { + MasterList::LockedInitializing locked_list; // log BEFORE pushing so logging singletons don't cry circularity - log_initializing("Pushing", name); - get_initializing().push_back(this); + locked_list.log("Pushing", name); + locked_list.get().push_back(this); } void LLSingletonBase::pop_initializing() { - list_t& list(get_initializing()); + // Lock the MasterList for the duration of this call + MasterList::LockedInitializing locked_list; + list_t& list(locked_list.get()); if (list.empty()) { @@ -165,7 +243,7 @@ void LLSingletonBase::pop_initializing() // entirely. if (list.empty()) { - MasterList::instance().cleanup_initializing_(); + locked_list.cleanup_initializing(); } // Now validate the newly-popped LLSingleton. @@ -177,7 +255,7 @@ void LLSingletonBase::pop_initializing() } // log AFTER popping so logging singletons don't cry circularity - log_initializing("Popping", typeid(*back).name()); + locked_list.log("Popping", typeid(*back).name()); } void LLSingletonBase::reset_initializing(list_t::size_type size) @@ -191,7 +269,8 @@ void LLSingletonBase::reset_initializing(list_t::size_type size) // push_initializing() call in LLSingletonBase's constructor. So only // remove the stack top if in fact we've pushed something more than the // previous size. - list_t& list(get_initializing()); + MasterList::LockedInitializing locked_list; + list_t& list(locked_list.get()); while (list.size() > size) { @@ -201,29 +280,32 @@ void LLSingletonBase::reset_initializing(list_t::size_type size) // as in pop_initializing() if (list.empty()) { - MasterList::instance().cleanup_initializing_(); + locked_list.cleanup_initializing(); } } -//static -void LLSingletonBase::log_initializing(const char* verb, const char* name) +void LLSingletonBase::MasterList::LockedInitializing::log(const char* verb, const char* name) { if (oktolog()) { LL_DEBUGS("LLSingleton") << verb << ' ' << demangle(name) << ';'; - list_t& list(get_initializing()); - for (list_t::const_reverse_iterator ri(list.rbegin()), rend(list.rend()); - ri != rend; ++ri) + if (mList) { - LLSingletonBase* sb(*ri); - LL_CONT << ' ' << classname(sb); + for (list_t::const_reverse_iterator ri(mList->rbegin()), rend(mList->rend()); + ri != rend; ++ri) + { + LLSingletonBase* sb(*ri); + LL_CONT << ' ' << classname(sb); + } } LL_ENDL; } } -void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initState) +void LLSingletonBase::capture_dependency(EInitState initState) { + MasterList::LockedInitializing locked_list; + list_t& initializing(locked_list.get()); // Did this getInstance() call come from another LLSingleton, or from // vanilla application code? Note that although this is a nontrivial // method, the vast majority of its calls arrive here with initializing @@ -313,8 +395,9 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() // deleteAll(). typedef LLDependencies SingletonDeps; SingletonDeps sdeps; - list_t& master(get_master()); - BOOST_FOREACH(LLSingletonBase* sp, master) + // Lock while traversing the master list + MasterList::LockedMaster master; + BOOST_FOREACH(LLSingletonBase* sp, master.get()) { // Build the SingletonDeps structure by adding, for each // LLSingletonBase* sp in the master list, sp itself. It has no @@ -326,7 +409,7 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() SingletonDeps::KeyList(sp->mDepends.begin(), sp->mDepends.end())); } vec_t ret; - ret.reserve(master.size()); + ret.reserve(master.get().size()); // We should be able to effect this with a transform_iterator that // extracts just the first (key) element from each sorted_iterator, then // uses vec_t's range constructor... but frankly this is more -- cgit v1.3 From a6f5e55d42f417ea8bc565e473354b64c192802d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 12 Dec 2019 16:14:38 -0500 Subject: DRTVWR-494: Dispatch all LLSingleton construction to the main thread. Given the viewer's mutually-dependent LLSingletons, given that different threads might simultaneously request different LLSingletons from such a chain of circular dependencies, the key to avoiding deadlock is to serialize all LLSingleton construction on one thread: the main thread. Add comments to LLSingleton::getInstance() explaining the problem and the solution. Recast LLSingleton's static SingletonData to use LockStatic. Instead of using Locker, and simply trusting that every reference to sData is within the dynamic scope of a Locker instance, LockStatic enforces that: you can only access SingletonData members via LockStatic. Reorganize the switch in getInstance() to group the CONSTRUCTING error, the INITIALIZING/INITIALIZED success case, and the DELETED/UNINITIALIZED construction case. When [re]constructing an instance, on the main thread, retain the lock and call constructSingleton() (and capture_dependency()) directly. On a secondary thread, unlock LockStatic and use LLMainThreadTask::dispatch() to call getInstance() on the main thread. Since we might end up enqueuing multiple such tasks, it's important to let getInstance() notice when the instance has already been constructed and simply return the existing pointer. Add loginfos() method, sibling to logerrs(), logwarns() and logdebugs(). Produce loginfos() messages when dispatching to the main thread, when actually running on the main thread and when resuming the suspended requesting thread. Make deleteSingleton() manage all associated state, instead of delegating some of that work to ~LLSingleton(). Now, within LockStatic, extract the instance pointer and set state to DELETED; that lets subsequent code, which retains the only remaining pointer to the instance, remove the master-list entry, call the subclass cleanupSingleton() and destructor without needing to hold the lock. In fact, entirely remove ~LLSingleton(). Import LLSingletonBase::cleanup_() method to wrap the call to subclass cleanupSingleton() in try/catch. Remove cleanupAll() calls from llsingleton_test.cpp, and reorder the success cases to reflect the fact that T::cleanupSingleton() is called immediately before ~T() for each distinct LLSingleton subclass T. When getInstance() on a secondary thread dispatches to the main thread, it necessarily unlocks its LockStatic lock. But an LLSingleton dependency chain strongly depends on the function stack on which getInstance() is invoked -- the task dispatched to the main thread doesn't know the dependencies tracked on the requesting thread stack. So, once the main thread delivers the instance pointer, the requesting thread captures its own dependencies for that instance. Back in the requesting thread, obtaining the current EInitState to pass to capture_dependencies() would have required relocking LockStatic. Instead, I've convinced myself that (a) capture_dependencies() only wanted to know EInitState to produce an error for CONSTRUCTING, and (b) in CONSTRUCTING state, we never get as far as capture_dependencies() because getInstance() produces an error first. Eliminate the EInitState parameter from all capture_dependencies() methods. Remove the LLSingletonBase::capture_dependency() stanza that tested EInitState. Make the capture_dependencies() variants that accepted LockStatic instead accept LLSingletonBase*. That lets getInstance(), in the LLMainThreadTask case, pass the newly-returned instance pointer. For symmetry, make pop_initializing() accept LLSingletonBase* as well, instead of accepting LockStatic and extracting mInstance. --- indra/llcommon/llsingleton.cpp | 36 ++-- indra/llcommon/llsingleton.h | 334 +++++++++++++++++------------- indra/llcommon/tests/llsingleton_test.cpp | 14 +- 3 files changed, 218 insertions(+), 166 deletions(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 812fd31719..bf594f122c 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -302,7 +302,7 @@ void LLSingletonBase::MasterList::LockedInitializing::log(const char* verb, cons } } -void LLSingletonBase::capture_dependency(EInitState initState) +void LLSingletonBase::capture_dependency() { MasterList::LockedInitializing locked_list; list_t& initializing(locked_list.get()); @@ -334,21 +334,8 @@ void LLSingletonBase::capture_dependency(EInitState initState) LLSingletonBase* foundp(*found); out << classname(foundp) << " -> "; } - // We promise to capture dependencies from both the constructor - // and the initSingleton() method, so an LLSingleton's instance - // pointer is on the initializing list during both. Now that we've - // detected circularity, though, we must distinguish the two. If - // the recursive call is from the constructor, we CAN'T honor it: - // otherwise we'd be returning a pointer to a partially- - // constructed object! But from initSingleton() is okay: that - // method exists specifically to support circularity. // Decide which log helper to call. - if (initState == CONSTRUCTING) - { - logerrs("LLSingleton circularity in Constructor: ", out.str().c_str(), - classname(this).c_str(), ""); - } - else if (it_next == initializing.end()) + if (it_next == initializing.end()) { // Points to self after construction, but during initialization. // Singletons can initialize other classes that depend onto them, @@ -457,6 +444,19 @@ void LLSingletonBase::cleanupAll() } } +void LLSingletonBase::cleanup_() +{ + logdebugs("calling ", classname(this).c_str(), "::cleanupSingleton()"); + try + { + cleanupSingleton(); + } + catch (...) + { + LOG_UNHANDLED_EXCEPTION(classname(this) + "::cleanupSingleton()"); + } +} + //static void LLSingletonBase::deleteAll() { @@ -536,6 +536,12 @@ void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, c log(LLError::LEVEL_WARN, p1, p2, p3, p4); } +//static +void LLSingletonBase::loginfos(const char* p1, const char* p2, const char* p3, const char* p4) +{ + log(LLError::LEVEL_INFO, p1, p2, p3, p4); +} + //static void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) { diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index ebae601029..4f3b8ceb38 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -31,6 +31,9 @@ #include #include #include "mutex.h" +#include "lockstatic.h" +#include "llthread.h" // on_main_thread() +#include "llmainthreadtask.h" class LLSingletonBase: private boost::noncopyable { @@ -105,7 +108,7 @@ protected: protected: // If a given call to B::getInstance() happens during either A::A() or // A::initSingleton(), record that A directly depends on B. - void capture_dependency(EInitState); + void capture_dependency(); // delegate LL_ERRS() logging to llsingleton.cpp static void logerrs(const char* p1, const char* p2="", @@ -113,6 +116,9 @@ protected: // delegate LL_WARNS() logging to llsingleton.cpp static void logwarns(const char* p1, const char* p2="", const char* p3="", const char* p4=""); + // delegate LL_INFOS() logging to llsingleton.cpp + static void loginfos(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); static std::string demangle(const char* mangled); template static std::string classname() { return demangle(typeid(T).name()); } @@ -123,6 +129,9 @@ protected: virtual void initSingleton() {} virtual void cleanupSingleton() {} + // internal wrapper around calls to cleanupSingleton() + void cleanup_(); + // deleteSingleton() isn't -- and shouldn't be -- a virtual method. It's a // class static. However, given only Foo*, deleteAll() does need to be // able to reach Foo::deleteSingleton(). Make LLSingleton (which declares @@ -193,9 +202,9 @@ struct LLSingleton_manage_master { return LLSingletonBase::get_initializing_size(); } - void capture_dependency(LLSingletonBase* sb, LLSingletonBase::EInitState state) + void capture_dependency(LLSingletonBase* sb) { - sb->capture_dependency(state); + sb->capture_dependency(); } }; @@ -210,14 +219,14 @@ struct LLSingleton_manage_master // since we never pushed, no need to clean up void reset_initializing(LLSingletonBase::list_t::size_type size) {} LLSingletonBase::list_t::size_type get_initializing_size() { return 0; } - void capture_dependency(LLSingletonBase*, LLSingletonBase::EInitState) {} + void capture_dependency(LLSingletonBase*) {} }; // Now we can implement LLSingletonBase's template constructor. template LLSingletonBase::LLSingletonBase(tag): mCleaned(false), - mDeleteSingleton(NULL) + mDeleteSingleton(nullptr) { // This is the earliest possible point at which we can push this new // instance onto the init stack. LLSingleton::constructSingleton() can't @@ -295,65 +304,40 @@ template class LLSingleton : public LLSingletonBase { private: - // Allow LLParamSingleton subclass -- but NOT DERIVED_TYPE itself -- to - // access our private members. - friend class LLParamSingleton; - - // Scoped lock on the mutex associated with this LLSingleton - class Locker + // LLSingleton must have a distinct instance of + // SingletonData for every distinct DERIVED_TYPE. It's tempting to + // consider hoisting SingletonData up into LLSingletonBase. Don't do it. + struct SingletonData { - public: - Locker(): mLock(getMutex()) {} - - private: // Use a recursive_mutex in case of constructor circularity. With a // non-recursive mutex, that would result in deadlock. typedef std::recursive_mutex mutex_t; + mutex_t mMutex; // LockStatic looks for mMutex - // LLSingleton must have a distinct instance of sMutex for every - // distinct T. It's tempting to consider hoisting Locker up into - // LLSingletonBase. Don't do it. - // - // sMutex must be a function-local static rather than a static member. One - // of the essential features of LLSingleton and friends is that they must - // support getInstance() even when the containing module's static - // variables have not yet been runtime-initialized. A mutex requires - // construction. A static class member might not yet have been - // constructed. - // - // We could store a dumb mutex_t*, notice when it's NULL and allocate a - // heap mutex -- but that's vulnerable to race conditions. And we can't - // defend the dumb pointer with another mutex. - // - // We could store a std::atomic -- but a default-constructed - // std::atomic does not contain a valid T, even a default-constructed - // T! Which means std::atomic, too, requires runtime initialization. - // - // But a function-local static is guaranteed to be initialized exactly - // once, the first time control reaches that declaration. - static mutex_t& getMutex() - { - static mutex_t sMutex; - return sMutex; - } - - std::unique_lock mLock; + EInitState mInitState{UNINITIALIZED}; + DERIVED_TYPE* mInstance{nullptr}; }; + typedef llthread::LockStatic LockStatic; + + // Allow LLParamSingleton subclass -- but NOT DERIVED_TYPE itself -- to + // access our private members. + friend class LLParamSingleton; // LLSingleton only supports a nullary constructor. However, the specific // purpose for its subclass LLParamSingleton is to support Singletons // requiring constructor arguments. constructSingleton() supports both use // cases. + // Accepting LockStatic& requires that the caller has already locked our + // static data before calling. template - static void constructSingleton(Args&&... args) + static void constructSingleton(LockStatic& lk, Args&&... args) { auto prev_size = LLSingleton_manage_master().get_initializing_size(); - // getInstance() calls are from within constructor - sData.mInitState = CONSTRUCTING; + // Any getInstance() calls after this point are from within constructor + lk->mInitState = CONSTRUCTING; try { - sData.mInstance = new DERIVED_TYPE(std::forward(args)...); - // we have called constructor, have not yet called initSingleton() + lk->mInstance = new DERIVED_TYPE(std::forward(args)...); } catch (const std::exception& err) { @@ -367,55 +351,56 @@ private: // There isn't a separate EInitState value meaning "we attempted // to construct this LLSingleton subclass but could not," so use // DELETED. That seems slightly more appropriate than UNINITIALIZED. - sData.mInitState = DELETED; + lk->mInitState = DELETED; // propagate the exception throw; } - // getInstance() calls are from within initSingleton() - sData.mInitState = INITIALIZING; + // Any getInstance() calls after this point are from within initSingleton() + lk->mInitState = INITIALIZING; try { // initialize singleton after constructing it so that it can // reference other singletons which in turn depend on it, thus // breaking cyclic dependencies - sData.mInstance->initSingleton(); - sData.mInitState = INITIALIZED; + lk->mInstance->initSingleton(); + lk->mInitState = INITIALIZED; // pop this off stack of initializing singletons - pop_initializing(); + pop_initializing(lk->mInstance); } catch (const std::exception& err) { // pop this off stack of initializing singletons here, too -- // BEFORE logging, so log-machinery LLSingletons don't record a // dependency on DERIVED_TYPE! - pop_initializing(); + pop_initializing(lk->mInstance); logwarns("Error in ", classname().c_str(), "::initSingleton(): ", err.what()); - // and get rid of the instance entirely + // Get rid of the instance entirely. This call depends on our + // recursive_mutex. We could have a deleteSingleton(LockStatic&) + // overload and pass lk, but we don't strictly need it. deleteSingleton(); // propagate the exception throw; } } - static void pop_initializing() + static void pop_initializing(LLSingletonBase* sb) { // route through LLSingleton_manage_master so we Do The Right Thing // (namely, nothing) for MasterList - LLSingleton_manage_master().pop_initializing(sData.mInstance); + LLSingleton_manage_master().pop_initializing(sb); } - static void capture_dependency() + static void capture_dependency(LLSingletonBase* sb) { // By this point, if DERIVED_TYPE was pushed onto the initializing // stack, it has been popped off. So the top of that stack, if any, is // an LLSingleton that directly depends on DERIVED_TYPE. If // getInstance() was called by another LLSingleton, rather than from // vanilla application code, record the dependency. - LLSingleton_manage_master().capture_dependency( - sData.mInstance, sData.mInitState); + LLSingleton_manage_master().capture_dependency(sb); } // We know of no way to instruct the compiler that every subclass @@ -442,19 +427,6 @@ protected: LLSingleton_manage_master().add(this); } -protected: - virtual ~LLSingleton() - { - // In case racing threads call getInstance() at the same moment as - // this destructor, serialize the calls. - Locker lk; - - // remove this instance from the master list - LLSingleton_manage_master().remove(this); - sData.mInstance = NULL; - sData.mInitState = DELETED; - } - public: /** * @brief Immediately delete the singleton. @@ -479,54 +451,149 @@ public: */ static void deleteSingleton() { - delete sData.mInstance; - // SingletonData state handled by destructor, above + DERIVED_TYPE* lameduck; + { + LockStatic lk; + // Capture the instance and clear SingletonData. This sequence + // guards against the chance that the destructor throws, somebody + // catches it and there's a subsequent call to getInstance(). + lameduck = lk->mInstance; + lk->mInstance = nullptr; + lk->mInitState = DELETED; + // At this point we can safely unlock SingletonData during the + // remaining cleanup. If another thread calls deleteSingleton() (or + // getInstance(), or whatever) it won't find our instance, now + // referenced only as 'lameduck'. + } + // of course, only cleanup and delete if there's something there + if (lameduck) + { + // remove this instance from the master list BEFORE attempting + // cleanup so possible destructor exception won't leave the master + // list confused + LLSingleton_manage_master().remove(lameduck); + lameduck->cleanup_(); + delete lameduck; + } } static DERIVED_TYPE* getInstance() { - // In case racing threads call getInstance() at the same moment, - // serialize the calls. - Locker lk; - - switch (sData.mInitState) - { - case CONSTRUCTING: - // here if DERIVED_TYPE's constructor (directly or indirectly) - // calls DERIVED_TYPE::getInstance() - logerrs("Tried to access singleton ", - classname().c_str(), - " from singleton constructor!"); - return NULL; - - case UNINITIALIZED: - constructSingleton(); - break; - - case INITIALIZING: - // here if DERIVED_TYPE::initSingleton() (directly or indirectly) - // calls DERIVED_TYPE::getInstance(): go ahead and allow it - case INITIALIZED: - // normal subsequent calls - break; - - case DELETED: - // called after deleteSingleton() - logwarns("Trying to access deleted singleton ", - classname().c_str(), - " -- creating new instance"); - constructSingleton(); - break; - } - - // record the dependency, if any: check if we got here from another - // LLSingleton's constructor or initSingleton() method - capture_dependency(); - return sData.mInstance; + // We know the viewer has LLSingleton dependency circularities. If you + // feel strongly motivated to eliminate them, cheers and good luck. + // (At that point we could consider a much simpler locking mechanism.) + + // If A and B depend on each other, and thread T1 requests A at the + // same moment thread T2 requests B, you could get a sequence like this: + // - T1 locks A + // - T2 locks B + // - T1, having constructed A, calls A::initSingleton(), which calls + // B::getInstance() and blocks on B's lock + // - T2, having constructed B, calls B::initSingleton(), which calls + // A::getInstance() and blocks on A's lock + // In other words, classic deadlock. + + // Avoid that by constructing and initializing every LLSingleton on + // the main thread. In that scenario: + // - T1 locks A + // - T2 locks B + // - T1 discovers A is UNINITIALIZED, so it queues a task for the main + // thread, unlocks A and blocks on the std::future. + // - T2 discovers B is UNINITIALIZED, so it queues a task for the main + // thread, unlocks B and blocks on the std::future. + // - The main thread executes T1's request for A. It locks A and + // starts to construct it. + // - A::initSingleton() calls B::getInstance(). Fine: nobody's holding + // B's lock. + // - The main thread locks B, constructs B, calls B::initSingleton(), + // which calls A::getInstance(), which returns A. + // - B::getInstance() returns B to A::initSingleton(), unlocking B. + // - A::getInstance() returns A to the task wrapper, unlocking A. + // - The task wrapper passes A to T1 via the future. T1 resumes. + // - The main thread executes T2's request for B. Oh look, B already + // exists. The task wrapper passes B to T2 via the future. T2 + // resumes. + // This still works even if one of T1 or T2 *is* the main thread. + // This still works even if thread T3 requests B at the same moment as + // T2. Finding B still UNINITIALIZED, T3 also queues a task for the + // main thread, unlocks B and blocks on a (distinct) std::future. By + // the time the main thread executes T3's request for B, B already + // exists, and is simply delivered via the future. + + { // nested scope for 'lk' + // In case racing threads call getInstance() at the same moment, + // serialize the calls. + LockStatic lk; + + switch (lk->mInitState) + { + case CONSTRUCTING: + // here if DERIVED_TYPE's constructor (directly or indirectly) + // calls DERIVED_TYPE::getInstance() + logerrs("Tried to access singleton ", + classname().c_str(), + " from singleton constructor!"); + return nullptr; + + case INITIALIZING: + // here if DERIVED_TYPE::initSingleton() (directly or indirectly) + // calls DERIVED_TYPE::getInstance(): go ahead and allow it + case INITIALIZED: + // normal subsequent calls + // record the dependency, if any: check if we got here from another + // LLSingleton's constructor or initSingleton() method + capture_dependency(lk->mInstance); + return lk->mInstance; + + case DELETED: + // called after deleteSingleton() + logwarns("Trying to access deleted singleton ", + classname().c_str(), + " -- creating new instance"); + // fall through + case UNINITIALIZED: + break; + } + + // Here we need to construct a new instance. + if (on_main_thread()) + { + // On the main thread, directly construct the instance while + // holding the lock. + constructSingleton(lk); + capture_dependency(lk->mInstance); + return lk->mInstance; + } + } // unlock 'lk' + + // Here we need to construct a new instance, but we're on a secondary + // thread. Per the comment block above, dispatch to the main thread. + loginfos(classname().c_str(), + "::getInstance() dispatching to main thread"); + auto instance = LLMainThreadTask::dispatch( + [](){ + // VERY IMPORTANT to call getInstance() on the main thread, + // rather than going straight to constructSingleton()! + // During the time window before mInitState is INITIALIZED, + // multiple requests might be queued. It's essential that, as + // the main thread processes them, only the FIRST such request + // actually constructs the instance -- every subsequent one + // simply returns the existing instance. + loginfos(classname().c_str(), + "::getInstance() on main thread"); + return getInstance(); + }); + // record the dependency chain tracked on THIS thread, not the main + // thread (consider a getInstance() overload with a tag param that + // suppresses dep tracking when dispatched to the main thread) + capture_dependency(instance); + loginfos(classname().c_str(), + "::getInstance() returning on invoking thread"); + return instance; } // Reference version of getInstance() - // Preferred over getInstance() as it disallows checking for NULL + // Preferred over getInstance() as it disallows checking for nullptr static DERIVED_TYPE& instance() { return *getInstance(); @@ -537,8 +604,8 @@ public: static bool instanceExists() { // defend any access to sData from racing threads - Locker lk; - return sData.mInitState == INITIALIZED; + LockStatic lk; + return lk->mInitState == INITIALIZED; } // Has this singleton been deleted? This can be useful during shutdown @@ -547,24 +614,11 @@ public: static bool wasDeleted() { // defend any access to sData from racing threads - Locker lk; - return sData.mInitState == DELETED; + LockStatic lk; + return lk->mInitState == DELETED; } - -private: - struct SingletonData - { - // explicitly has a default constructor so that member variables are zero initialized in BSS - // and only changed by singleton logic, not constructor running during startup - EInitState mInitState; - DERIVED_TYPE* mInstance; - }; - static SingletonData sData; }; -template -typename LLSingleton::SingletonData LLSingleton::sData; - /** * LLParamSingleton is like LLSingleton, except in the following ways: @@ -589,7 +643,7 @@ class LLParamSingleton : public LLSingleton { private: typedef LLSingleton super; - using typename super::Locker; + using typename super::LockStatic; public: using super::deleteSingleton; @@ -603,9 +657,9 @@ public: // In case racing threads both call initParamSingleton() at the same // time, serialize them. One should initialize; the other should see // mInitState already set. - Locker lk; + LockStatic lk; // For organizational purposes this function shouldn't be called twice - if (super::sData.mInitState != super::UNINITIALIZED) + if (lk->mInitState != super::UNINITIALIZED) { super::logerrs("Tried to initialize singleton ", super::template classname().c_str(), @@ -613,7 +667,7 @@ public: } else { - super::constructSingleton(std::forward(args)...); + super::constructSingleton(lk, std::forward(args)...); } } @@ -621,9 +675,9 @@ public: { // In case racing threads call getInstance() at the same moment as // initParamSingleton(), serialize the calls. - Locker lk; + LockStatic lk; - switch (super::sData.mInitState) + switch (lk->mInitState) { case super::UNINITIALIZED: super::logerrs("Uninitialized param singleton ", @@ -641,8 +695,8 @@ public: // within initSingleton() case super::INITIALIZED: // for any valid call, capture dependencies - super::capture_dependency(); - return super::sData.mInstance; + super::capture_dependency(lk->mInstance); + return lk->mInstance; case super::DELETED: super::logerrs("Trying to access deleted param singleton ", diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp index 75ddff9d7d..15ffe68e67 100644 --- a/indra/llcommon/tests/llsingleton_test.cpp +++ b/indra/llcommon/tests/llsingleton_test.cpp @@ -143,8 +143,6 @@ namespace tut \ (void)CLS::instance(); \ ensure_equals(sLog, #CLS "i" #CLS); \ - LLSingletonBase::cleanupAll(); \ - ensure_equals(sLog, #CLS "i" #CLS "x" #CLS); \ LLSingletonBase::deleteAll(); \ ensure_equals(sLog, #CLS "i" #CLS "x" #CLS "~" #CLS); \ } \ @@ -159,10 +157,8 @@ namespace tut \ (void)CLS::instance(); \ ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS); \ - LLSingletonBase::cleanupAll(); \ - ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS "x" #CLS "x" #OTHER); \ LLSingletonBase::deleteAll(); \ - ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS "x" #CLS "x" #OTHER "~" #CLS "~" #OTHER); \ + ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS "x" #CLS "~" #CLS "x" #OTHER "~" #OTHER); \ } \ \ template<> template<> \ @@ -175,10 +171,8 @@ namespace tut \ (void)CLS::instance(); \ ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER); \ - LLSingletonBase::cleanupAll(); \ - ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER); \ LLSingletonBase::deleteAll(); \ - ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER "~" #CLS "~" #OTHER); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "~" #CLS "x" #OTHER "~" #OTHER); \ } \ \ template<> template<> \ @@ -191,10 +185,8 @@ namespace tut \ (void)CLS::instance(); \ ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER); \ - LLSingletonBase::cleanupAll(); \ - ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER); \ LLSingletonBase::deleteAll(); \ - ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER "~" #CLS "~" #OTHER); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "~" #CLS "x" #OTHER "~" #OTHER); \ } TESTS(A, B, 4, 5, 6, 7) -- cgit v1.3 From 31863d833c7b573f3608e3353b9e5f694b611627 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 17 Dec 2019 15:42:34 -0500 Subject: DRTVWR-494: Move most LLSingleton cleanup back to destructor instead of deleteSingleton(). Specifically, clear static SingletonData and remove the instance from the MasterList in the destructor. Empirically, some consumers are manually deleting LLSingleton instances, instead of calling deleteSingleton(). If deleteSingleton() handles cleanup rather than the destructor, we're left with dangling pointers in the Master List. We don't also call cleanupSingleton() from the destructor because only deleteSingleton() promises to call cleanupSingleton(). Hopefully whoever is directly deleting an LLSingleton subclass instance isn't relying on cleanupSingleton(). --- indra/llcommon/llsingleton.cpp | 6 +++--- indra/llcommon/llsingleton.h | 44 ++++++++++++++++++++++-------------------- 2 files changed, 26 insertions(+), 24 deletions(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index bf594f122c..4c76206d8d 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -384,7 +384,7 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() SingletonDeps sdeps; // Lock while traversing the master list MasterList::LockedMaster master; - BOOST_FOREACH(LLSingletonBase* sp, master.get()) + for (LLSingletonBase* sp : master.get()) { // Build the SingletonDeps structure by adding, for each // LLSingletonBase* sp in the master list, sp itself. It has no @@ -401,14 +401,14 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() // extracts just the first (key) element from each sorted_iterator, then // uses vec_t's range constructor... but frankly this is more // straightforward, as long as we remember the above reserve() call! - BOOST_FOREACH(SingletonDeps::sorted_iterator::value_type pair, sdeps.sort()) + for (const SingletonDeps::sorted_iterator::value_type& pair : sdeps.sort()) { ret.push_back(pair.first); } // The master list is not itself pushed onto the master list. Add it as // the very last entry -- it is the LLSingleton on which ALL others // depend! -- so our caller will process it. - ret.push_back(MasterList::getInstance()); + ret.push_back(&master.Lock::get()); return ret; } diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 0c11e54910..5ee40a658a 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -428,6 +428,21 @@ protected: LLSingleton_manage_master().add(this); } +protected: + virtual ~LLSingleton() + { + // This phase of cleanup is performed in the destructor rather than in + // deleteSingleton() to defend against manual deletion. When we moved + // cleanup to deleteSingleton(), we hit crashes due to dangling + // pointers in the MasterList. + LockStatic lk; + lk->mInstance = nullptr; + lk->mInitState = DELETED; + + // Remove this instance from the master list. + LLSingleton_manage_master().remove(this); + } + public: /** * @brief Immediately delete the singleton. @@ -452,29 +467,16 @@ public: */ static void deleteSingleton() { - DERIVED_TYPE* lameduck; - { - LockStatic lk; - // Capture the instance and clear SingletonData. This sequence - // guards against the chance that the destructor throws, somebody - // catches it and there's a subsequent call to getInstance(). - lameduck = lk->mInstance; - lk->mInstance = nullptr; - lk->mInitState = DELETED; - // At this point we can safely unlock SingletonData during the - // remaining cleanup. If another thread calls deleteSingleton() (or - // getInstance(), or whatever) it won't find our instance, now - // referenced only as 'lameduck'. - } + // Hold the lock while we call cleanupSingleton() and the destructor. + // Our destructor also instantiates LockStatic, requiring a recursive + // mutex. + LockStatic lk; // of course, only cleanup and delete if there's something there - if (lameduck) + if (lk->mInstance) { - // remove this instance from the master list BEFORE attempting - // cleanup so possible destructor exception won't leave the master - // list confused - LLSingleton_manage_master().remove(lameduck); - lameduck->cleanup_(); - delete lameduck; + lk->mInstance->cleanup_(); + delete lk->mInstance; + // destructor clears mInstance (and mInitState) } } -- cgit v1.3 From 47ec6ab3be5df5ee3f80a642d9c2ef7f4dac0d8a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 30 May 2019 08:23:32 -0400 Subject: SL-11216: Remove LLSingletonBase::cleanupAll(). Remove call from LLAppViewer::cleanup(). Instead, make each LLSingleton::deleteSingleton() call cleanupSingleton() just before destroying the instance. Since deleteSingleton() is not a destructor, it's fine to call cleanupSingleton() from there; and since deleteAll() calls deleteSingleton() on every remaining instance, the former cleanupAll() functionality has been subsumed into deleteAll(). Since cleanupSingleton() is now called at exactly one point in the instance's lifetime, we no longer need a bool indicating whether it has been called. The previous protocol of calling cleanupAll() before deleteAll() implemented a two-phase cleanup strategy for the application. That is no longer needed. Moreover, the cleanupAll() / deleteAll() sequence created a time window during which individual LLSingleton instances weren't usable (to the extent that their cleanupSingleton() methods released essential resources) but still existed -- so a getInstance() call would return the crippled instance rather than recreating it. Remove cleanupAll() calls from tests; adjust to new order of expected side effects: instead of A::cleanupSingleton(), B::cleanupSingleton(), ~A(), ~B(), now we get A::cleanupSingleton(), ~A(), B::cleanupSingleton(), ~B(). --- indra/llcommon/llsingleton.cpp | 35 +------------- indra/llcommon/llsingleton.h | 105 +++++++++++++++-------------------------- indra/newview/llappviewer.cpp | 16 ++----- 3 files changed, 44 insertions(+), 112 deletions(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 4c76206d8d..f5f3aec270 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -378,8 +378,7 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() // SingletonDeps through the life of the program, dynamically adding and // removing LLSingletons as they are created and destroyed, in practice // it's less messy to construct it on demand. The overhead of doing so - // should happen basically twice: once for cleanupAll(), once for - // deleteAll(). + // should happen basically once: for deleteAll(). typedef LLDependencies SingletonDeps; SingletonDeps sdeps; // Lock while traversing the master list @@ -412,38 +411,6 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() return ret; } -//static -void LLSingletonBase::cleanupAll() -{ - // It's essential to traverse these in dependency order. - BOOST_FOREACH(LLSingletonBase* sp, dep_sort()) - { - // Call cleanupSingleton() only if we haven't already done so for this - // instance. - if (! sp->mCleaned) - { - sp->mCleaned = true; - - logdebugs("calling ", - classname(sp).c_str(), "::cleanupSingleton()"); - try - { - sp->cleanupSingleton(); - } - catch (const std::exception& e) - { - logwarns("Exception in ", classname(sp).c_str(), - "::cleanupSingleton(): ", e.what()); - } - catch (...) - { - logwarns("Unknown exception in ", classname(sp).c_str(), - "::cleanupSingleton()"); - } - } - } -} - void LLSingletonBase::cleanup_() { logdebugs("calling ", classname(this).c_str(), "::cleanupSingleton()"); diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 5ee40a658a..65dd332afb 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -50,7 +50,6 @@ private: typedef std::vector vec_t; static vec_t dep_sort(); - bool mCleaned; // cleanupSingleton() has been called // we directly depend on these other LLSingletons typedef boost::unordered_set set_t; set_t mDepends; @@ -142,32 +141,15 @@ protected: public: /** - * Call this to call the cleanupSingleton() method for every LLSingleton - * constructed since the start of the last cleanupAll() call. (Any - * LLSingleton constructed DURING a cleanupAll() call won't be cleaned up - * until the next cleanupAll() call.) cleanupSingleton() neither deletes - * nor destroys its LLSingleton; therefore it's safe to include logic that - * might take significant realtime or even throw an exception. - * - * The most important property of cleanupAll() is that cleanupSingleton() - * methods are called in dependency order, leaf classes last. Thus, given - * two LLSingleton subclasses A and B, if A's dependency on B is properly - * expressed as a B::getInstance() or B::instance() call during either - * A::A() or A::initSingleton(), B will be cleaned up after A. - * - * If a cleanupSingleton() method throws an exception, the exception is - * logged, but cleanupAll() attempts to continue calling the rest of the - * cleanupSingleton() methods. - */ - static void cleanupAll(); - /** - * Call this to call the deleteSingleton() method for every LLSingleton - * constructed since the start of the last deleteAll() call. (Any - * LLSingleton constructed DURING a deleteAll() call won't be cleaned up - * until the next deleteAll() call.) deleteSingleton() deletes and - * destroys its LLSingleton. Any cleanup logic that might take significant - * realtime -- or throw an exception -- must not be placed in your - * LLSingleton's destructor, but rather in its cleanupSingleton() method. + * deleteAll() calls the cleanupSingleton() and deleteSingleton() methods + * for every LLSingleton constructed since the start of the last + * deleteAll() call. (Any LLSingleton constructed DURING a deleteAll() + * call won't be cleaned up until the next deleteAll() call.) + * deleteSingleton() deletes and destroys its LLSingleton. Any cleanup + * logic that might take significant realtime -- or throw an exception -- + * must not be placed in your LLSingleton's destructor, but rather in its + * cleanupSingleton() method, which is called implicitly by + * deleteSingleton(). * * The most important property of deleteAll() is that deleteSingleton() * methods are called in dependency order, leaf classes last. Thus, given @@ -175,9 +157,9 @@ public: * expressed as a B::getInstance() or B::instance() call during either * A::A() or A::initSingleton(), B will be cleaned up after A. * - * If a deleteSingleton() method throws an exception, the exception is - * logged, but deleteAll() attempts to continue calling the rest of the - * deleteSingleton() methods. + * If a cleanupSingleton() or deleteSingleton() method throws an + * exception, the exception is logged, but deleteAll() attempts to + * continue calling the rest of the deleteSingleton() methods. */ static void deleteAll(); }; @@ -226,7 +208,6 @@ struct LLSingleton_manage_master // Now we can implement LLSingletonBase's template constructor. template LLSingletonBase::LLSingletonBase(tag): - mCleaned(false), mDeleteSingleton(nullptr) { // This is the earliest possible point at which we can push this new @@ -269,10 +250,19 @@ class LLParamSingleton; * leading back to yours, move the instance reference from your constructor to * your initSingleton() method. * - * If you override LLSingleton::cleanupSingleton(), your method will be - * called if someone calls LLSingletonBase::cleanupAll(). The significant part - * of this promise is that cleanupAll() will call individual - * cleanupSingleton() methods in reverse dependency order. + * If you override LLSingleton::cleanupSingleton(), your method will + * implicitly be called by LLSingleton::deleteSingleton() just before the + * instance is destroyed. We introduce a special cleanupSingleton() method + * because cleanupSingleton() operations can involve nontrivial realtime, or + * throw an exception. A destructor should do neither! + * + * If your cleanupSingleton() method throws an exception, we log that + * exception but carry on. + * + * If at some point you call LLSingletonBase::deleteAll(), all remaining + * LLSingleton instances will be destroyed in reverse dependency order. (Or + * call MySubclass::deleteSingleton() to specifically destroy the canonical + * MySubclass instance.) * * That is, consider LLSingleton subclasses C, B and A. A depends on B, which * in turn depends on C. These dependencies are expressed as calls to @@ -280,26 +270,14 @@ class LLParamSingleton; * It shouldn't matter whether these calls appear in A::A() or * A::initSingleton(), likewise B::B() or B::initSingleton(). * - * We promise that if you later call LLSingletonBase::cleanupAll(): - * 1. A::cleanupSingleton() will be called before - * 2. B::cleanupSingleton(), which will be called before - * 3. C::cleanupSingleton(). + * We promise that if you later call LLSingletonBase::deleteAll(): + * 1. A::deleteSingleton() will be called before + * 2. B::deleteSingleton(), which will be called before + * 3. C::deleteSingleton(). * Put differently, if your LLSingleton subclass constructor or * initSingleton() method explicitly depends on some other LLSingleton * subclass, you may continue to rely on that other subclass in your * cleanupSingleton() method. - * - * We introduce a special cleanupSingleton() method because cleanupSingleton() - * operations can involve nontrivial realtime, or might throw an exception. A - * destructor should do neither! - * - * If your cleanupSingleton() method throws an exception, we log that - * exception but proceed with the remaining cleanupSingleton() calls. - * - * Similarly, if at some point you call LLSingletonBase::deleteAll(), all - * remaining LLSingleton instances will be destroyed in dependency order. (Or - * call MySubclass::deleteSingleton() to specifically destroy the canonical - * MySubclass instance.) */ template class LLSingleton : public LLSingletonBase @@ -445,25 +423,18 @@ protected: public: /** - * @brief Immediately delete the singleton. + * @brief Cleanup and destroy the singleton instance. * - * A subsequent call to LLProxy::getInstance() will construct a new - * instance of the class. + * deleteSingleton() calls this instance's cleanupSingleton() method and + * then destroys the instance. * - * Without an explicit call to LLSingletonBase::deleteAll(), LLSingletons - * are implicitly destroyed after main() has exited and the C++ runtime is - * cleaning up statically-constructed objects. Some classes derived from - * LLSingleton have objects that are part of a runtime system that is - * terminated before main() exits. Calling the destructor of those objects - * after the termination of their respective systems can cause crashes and - * other problems during termination of the project. Using this method to - * destroy the singleton early can prevent these crashes. + * A subsequent call to LLSingleton::getInstance() will construct a new + * instance of the class. * - * An example where this is needed is for a LLSingleton that has an APR - * object as a member that makes APR calls on destruction. The APR system is - * shut down explicitly before main() exits. This causes a crash on exit. - * Using this method before the call to apr_terminate() and NOT calling - * getInstance() again will prevent the crash. + * Without an explicit call to LLSingletonBase::deleteAll(), or + * LLSingleton::deleteSingleton(), LLSingleton instances are simply + * leaked. (Allowing implicit destruction at shutdown caused too many + * problems.) */ static void deleteSingleton() { diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index a76ac58724..ed80e7c0ce 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2093,25 +2093,19 @@ bool LLAppViewer::cleanup() removeMarkerFiles(); - // It's not at first obvious where, in this long sequence, generic cleanup - // calls OUGHT to go. So let's say this: as we migrate cleanup from + // It's not at first obvious where, in this long sequence, a generic cleanup + // call OUGHT to go. So let's say this: as we migrate cleanup from // explicit hand-placed calls into the generic mechanism, eventually - // all cleanup will get subsumed into the generic calls. So the calls you + // all cleanup will get subsumed into the generic call. So the calls you // still see above are calls that MUST happen before the generic cleanup // kicks in. - // This calls every remaining LLSingleton's cleanupSingleton() method. - // This method should perform any cleanup that might take significant - // realtime, or might throw an exception. - LLSingletonBase::cleanupAll(); - // The logging subsystem depends on an LLSingleton. Any logging after // LLSingletonBase::deleteAll() won't be recorded. LL_INFOS() << "Goodbye!" << LL_ENDL; - // This calls every remaining LLSingleton's deleteSingleton() method. - // No class destructor should perform any cleanup that might take - // significant realtime, or throw an exception. + // This calls every remaining LLSingleton's cleanupSingleton() and + // deleteSingleton() methods. LLSingletonBase::deleteAll(); removeDumpDir(); -- cgit v1.3 From 66981fab0b3c8dcc3a031d50710a2b24ec6b0603 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 10 May 2018 21:46:07 -0400 Subject: SL-793: Use Boost.Fiber instead of the "dcoroutine" library. Longtime fans will remember that the "dcoroutine" library is a Google Summer of Code project by Giovanni P. Deretta. He originally called it "Boost.Coroutine," and we originally added it to our 3p-boost autobuild package as such. But when the official Boost.Coroutine library came along (with a very different API), and we still needed the API of the GSoC project, we renamed the unofficial one "dcoroutine" to allow coexistence. The "dcoroutine" library had an internal low-level API more or less analogous to Boost.Context. We later introduced an implementation of that internal API based on Boost.Context, a step towards eliminating the GSoC code in favor of official, supported Boost code. However, recent versions of Boost.Context no longer support the API on which we built the shim for "dcoroutine." We started down the path of reimplementing that shim using the current Boost.Context API -- then realized that it's time to bite the bullet and replace the "dcoroutine" API with the Boost.Fiber API, which we've been itching to do for literally years now. Naturally, most of the heavy lifting is in llcoros.{h,cpp} and lleventcoro.{h,cpp} -- which is good: the LLCoros layer abstracts away most of the differences between "dcoroutine" and Boost.Fiber. The one feature Boost.Fiber does not provide is the ability to forcibly terminate some other fiber. Accordingly, disable LLCoros::kill() and LLCoprocedureManager::shutdown(). The only known shutdown() call was in LLCoprocedurePool's destructor. We also took the opportunity to remove postAndSuspend2() and its associated machinery: FutureListener2, LLErrorEvent, errorException(), errorLog(), LLCoroEventPumps. All that dual-LLEventPump stuff was introduced at a time when the Responder pattern was king, and we assumed we'd want to listen on one LLEventPump with the success handler and on another with the error handler. We have never actually used that in practice. Remove associated tests, of course. There is one other semantic difference that necessitates patching a number of tests: with "dcoroutine," fulfilling a future IMMEDIATELY resumes the waiting coroutine. With Boost.Fiber, fulfilling a future merely marks the fiber as ready to resume next time the scheduler gets around to it. To observe the test side effects, we've inserted a number of llcoro::suspend() calls -- also in the main loop. For a long time we retained a single unit test exercising the raw "dcoroutine" API. Remove that. Eliminate llcoro_get_id.{h,cpp}, which provided llcoro::get_id(), which was a hack to emulate fiber-local variables. Since Boost.Fiber has an actual API for that, remove the hack. In fact, use (new alias) LLCoros::local_ptr for LLSingleton's dependency tracking in place of llcoro::get_id(). In CMake land, replace BOOST_COROUTINE_LIBRARY with BOOST_FIBER_LIBRARY. We don't actually use the Boost.Coroutine for anything (though there exist plausible use cases). --- indra/cmake/Boost.cmake | 20 +- indra/cmake/LLAddBuildTest.cmake | 4 +- indra/cmake/LLAppearance.cmake | 2 +- indra/cmake/LLCommon.cmake | 4 +- indra/cmake/LLCoreHttp.cmake | 2 +- indra/linux_crash_logger/CMakeLists.txt | 2 +- indra/llcommon/CMakeLists.txt | 6 +- indra/llcommon/llcoro_get_id.cpp | 32 -- indra/llcommon/llcoro_get_id.h | 30 -- indra/llcommon/llcoros.cpp | 297 +++------- indra/llcommon/llcoros.h | 184 ++----- indra/llcommon/lleventcoro.cpp | 268 +++------ indra/llcommon/lleventcoro.h | 204 +------ indra/llcommon/llsingleton.cpp | 41 +- indra/llcommon/tests/lleventcoro_test.cpp | 598 ++------------------- indra/llmessage/CMakeLists.txt | 8 +- indra/llmessage/llcoproceduremanager.cpp | 6 + indra/llmessage/llcoproceduremanager.h | 2 + indra/llprimitive/CMakeLists.txt | 2 +- indra/llui/CMakeLists.txt | 2 +- indra/mac_crash_logger/CMakeLists.txt | 2 +- indra/newview/CMakeLists.txt | 4 +- indra/newview/llappviewer.cpp | 2 + indra/test/CMakeLists.txt | 2 +- indra/viewer_components/login/CMakeLists.txt | 4 +- .../viewer_components/login/tests/lllogin_test.cpp | 5 + indra/win_crash_logger/CMakeLists.txt | 2 +- 27 files changed, 331 insertions(+), 1404 deletions(-) delete mode 100644 indra/llcommon/llcoro_get_id.cpp delete mode 100644 indra/llcommon/llcoro_get_id.h (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/cmake/Boost.cmake b/indra/cmake/Boost.cmake index 180a84dbcf..e05e3ca0e5 100644 --- a/indra/cmake/Boost.cmake +++ b/indra/cmake/Boost.cmake @@ -8,7 +8,7 @@ if (USESYSTEMLIBS) include(FindBoost) set(BOOST_CONTEXT_LIBRARY boost_context-mt) - set(BOOST_COROUTINE_LIBRARY boost_coroutine-mt) + set(BOOST_FIBER_LIBRARY boost_fiber-mt) set(BOOST_FILESYSTEM_LIBRARY boost_filesystem-mt) set(BOOST_PROGRAM_OPTIONS_LIBRARY boost_program_options-mt) set(BOOST_REGEX_LIBRARY boost_regex-mt) @@ -49,9 +49,9 @@ else (USESYSTEMLIBS) set(BOOST_CONTEXT_LIBRARY optimized libboost_context-mt debug libboost_context-mt-gd) - set(BOOST_COROUTINE_LIBRARY - optimized libboost_coroutine-mt - debug libboost_coroutine-mt-gd) + set(BOOST_FIBER_LIBRARY + optimized libboost_fiber-mt + debug libboost_fiber-mt-gd) set(BOOST_FILESYSTEM_LIBRARY optimized libboost_filesystem-mt debug libboost_filesystem-mt-gd) @@ -75,9 +75,9 @@ else (USESYSTEMLIBS) set(BOOST_CONTEXT_LIBRARY optimized boost_context-mt debug boost_context-mt-d) - set(BOOST_COROUTINE_LIBRARY - optimized boost_coroutine-mt - debug boost_coroutine-mt-d) + set(BOOST_FIBER_LIBRARY + optimized boost_fiber-mt + debug boost_fiber-mt-d) set(BOOST_FILESYSTEM_LIBRARY optimized boost_filesystem-mt debug boost_filesystem-mt-d) @@ -100,9 +100,9 @@ else (USESYSTEMLIBS) set(BOOST_CONTEXT_LIBRARY optimized boost_context-mt debug boost_context-mt-d) - set(BOOST_COROUTINE_LIBRARY - optimized boost_coroutine-mt - debug boost_coroutine-mt-d) + set(BOOST_FIBER_LIBRARY + optimized boost_fiber-mt + debug boost_fiber-mt-d) set(BOOST_FILESYSTEM_LIBRARY optimized boost_filesystem-mt debug boost_filesystem-mt-d) diff --git a/indra/cmake/LLAddBuildTest.cmake b/indra/cmake/LLAddBuildTest.cmake index b3f42c1a5e..ee6396e473 100644 --- a/indra/cmake/LLAddBuildTest.cmake +++ b/indra/cmake/LLAddBuildTest.cmake @@ -53,7 +53,7 @@ INCLUDE(GoogleMock) ${GOOGLEMOCK_INCLUDE_DIRS} ) SET(alltest_LIBRARIES - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${GOOGLEMOCK_LIBRARIES} @@ -201,7 +201,7 @@ FUNCTION(LL_ADD_INTEGRATION_TEST SET(libraries ${library_dependencies} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${GOOGLEMOCK_LIBRARIES} diff --git a/indra/cmake/LLAppearance.cmake b/indra/cmake/LLAppearance.cmake index ae265d07e3..675330ec72 100644 --- a/indra/cmake/LLAppearance.cmake +++ b/indra/cmake/LLAppearance.cmake @@ -18,7 +18,7 @@ endif (BUILD_HEADLESS) set(LLAPPEARANCE_LIBRARIES llappearance llmessage llcorehttp - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) diff --git a/indra/cmake/LLCommon.cmake b/indra/cmake/LLCommon.cmake index 3e29297c58..8900419f9b 100644 --- a/indra/cmake/LLCommon.cmake +++ b/indra/cmake/LLCommon.cmake @@ -19,7 +19,7 @@ if (LINUX) # specify all libraries that llcommon uses. # llcommon uses `clock_gettime' which is provided by librt on linux. set(LLCOMMON_LIBRARIES llcommon - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY} @@ -27,7 +27,7 @@ if (LINUX) ) else (LINUX) set(LLCOMMON_LIBRARIES llcommon - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) diff --git a/indra/cmake/LLCoreHttp.cmake b/indra/cmake/LLCoreHttp.cmake index 379ae207de..613453ab5d 100644 --- a/indra/cmake/LLCoreHttp.cmake +++ b/indra/cmake/LLCoreHttp.cmake @@ -12,6 +12,6 @@ set(LLCOREHTTP_INCLUDE_DIRS ) set(LLCOREHTTP_LIBRARIES llcorehttp - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY}) diff --git a/indra/linux_crash_logger/CMakeLists.txt b/indra/linux_crash_logger/CMakeLists.txt index 315aed8d11..d789c850a0 100644 --- a/indra/linux_crash_logger/CMakeLists.txt +++ b/indra/linux_crash_logger/CMakeLists.txt @@ -69,7 +69,7 @@ target_link_libraries(linux-crash-logger ${LLMATH_LIBRARIES} ${LLCOREHTTP_LIBRARIES} ${LLCOMMON_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${UI_LIBRARIES} ${DB_LIBRARIES} diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 55c44446b4..2f263cd830 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -44,7 +44,6 @@ set(llcommon_SOURCE_FILES llcleanup.cpp llcommon.cpp llcommonutils.cpp - llcoro_get_id.cpp llcoros.cpp llcrc.cpp llcriticaldamp.cpp @@ -146,7 +145,6 @@ set(llcommon_HEADER_FILES llcleanup.h llcommon.h llcommonutils.h - llcoro_get_id.h llcoros.h llcrc.h llcriticaldamp.h @@ -293,7 +291,7 @@ target_link_libraries( ${JSONCPP_LIBRARIES} ${ZLIB_LIBRARIES} ${WINDOWS_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} @@ -322,7 +320,7 @@ if (LL_TESTS) ${LLCOMMON_LIBRARIES} ${WINDOWS_LIBRARIES} ${GOOGLEMOCK_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_THREAD_LIBRARY} ${BOOST_SYSTEM_LIBRARY}) diff --git a/indra/llcommon/llcoro_get_id.cpp b/indra/llcommon/llcoro_get_id.cpp deleted file mode 100644 index 24ed1fe0c9..0000000000 --- a/indra/llcommon/llcoro_get_id.cpp +++ /dev/null @@ -1,32 +0,0 @@ -/** - * @file llcoro_get_id.cpp - * @author Nat Goodspeed - * @date 2016-09-03 - * @brief Implementation for llcoro_get_id. - * - * $LicenseInfo:firstyear=2016&license=viewerlgpl$ - * Copyright (c) 2016, Linden Research, Inc. - * $/LicenseInfo$ - */ - -// Precompiled header -#include "linden_common.h" -// associated header -#include "llcoro_get_id.h" -// STL headers -// std headers -// external library headers -// other Linden headers -#include "llcoros.h" - -namespace llcoro -{ - -id get_id() -{ - // An instance of Current can convert to LLCoros::CoroData*, which can - // implicitly convert to void*, which is an llcoro::id. - return LLCoros::Current(); -} - -} // llcoro diff --git a/indra/llcommon/llcoro_get_id.h b/indra/llcommon/llcoro_get_id.h deleted file mode 100644 index 4c1dca6f19..0000000000 --- a/indra/llcommon/llcoro_get_id.h +++ /dev/null @@ -1,30 +0,0 @@ -/** - * @file llcoro_get_id.h - * @author Nat Goodspeed - * @date 2016-09-03 - * @brief Supplement the functionality in llcoro.h. - * - * This is broken out as a separate header file to resolve - * circularity: LLCoros isa LLSingleton, yet LLSingleton machinery - * requires llcoro::get_id(). - * - * Be very suspicious of anyone else #including this header. - * - * $LicenseInfo:firstyear=2016&license=viewerlgpl$ - * Copyright (c) 2016, Linden Research, Inc. - * $/LicenseInfo$ - */ - -#if ! defined(LL_LLCORO_GET_ID_H) -#define LL_LLCORO_GET_ID_H - -namespace llcoro -{ - -/// Get an opaque, distinct token for the running coroutine (or main). -typedef void* id; -id get_id(); - -} // llcoro - -#endif /* ! defined(LL_LLCORO_GET_ID_H) */ diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index cc775775bf..f5ffd96cec 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -34,6 +34,17 @@ // std headers // external library headers #include +#include +#ifndef BOOST_DISABLE_ASSERTS +#define UNDO_BOOST_DISABLE_ASSERTS +// with Boost 1.65.1, needed for Mac with this specific header +#define BOOST_DISABLE_ASSERTS +#endif +#include +#ifdef UNDO_BOOST_DISABLE_ASSERTS +#undef UNDO_BOOST_DISABLE_ASSERTS +#undef BOOST_DISABLE_ASSERTS +#endif // other Linden headers #include "lltimer.h" #include "llevents.h" @@ -45,176 +56,69 @@ #include #endif -namespace { -void no_op() {} -} // anonymous namespace -// Do nothing, when we need nothing done. This is a static member of LLCoros -// because CoroData is a private nested class. -void LLCoros::no_cleanup(CoroData*) {} - -// CoroData for the currently-running coroutine. Use a thread_specific_ptr -// because each thread potentially has its own distinct pool of coroutines. -LLCoros::Current::Current() +const LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) const { - // Use a function-static instance so this thread_specific_ptr is - // instantiated on demand. Since we happen to know it's consumed by - // LLSingleton, this is likely to happen before the runtime has finished - // initializing module-static data. For the same reason, we can't package - // this pointer in an LLSingleton. - - // This thread_specific_ptr does NOT own the CoroData object! That's owned - // by LLCoros::mCoros. It merely identifies it. For this reason we - // instantiate it with a no-op cleanup function. - static boost::thread_specific_ptr sCurrent(LLCoros::no_cleanup); - - // If this is the first time we're accessing sCurrent for the running - // thread, its get() will be NULL. This could be a problem, in that - // llcoro::get_id() would return the same (NULL) token value for the "main - // coroutine" in every thread, whereas what we really want is a distinct - // value for every distinct stack in the process. So if get() is NULL, - // give it a heap CoroData: this ensures that llcoro::get_id() will return - // distinct values. - // This tactic is "leaky": sCurrent explicitly does not destroy any - // CoroData to which it points, and we do NOT enter these "main coroutine" - // CoroData instances in the LLCoros::mCoros map. They are dummy entries, - // and they will leak at process shutdown: one CoroData per thread. - if (! sCurrent.get()) + CoroData* current = mCurrent.get(); + // For the main() coroutine, the one NOT explicitly launched by launch(), + // we never explicitly set mCurrent. Use a static CoroData instance with + // canonical values. + if (! current) { // It's tempting to provide a distinct name for each thread's "main // coroutine." But as getName() has always returned the empty string - // to mean "not in a coroutine," empty string should suffice here -- - // and truthfully the additional (thread-safe!) machinery to ensure - // uniqueness just doesn't feel worth the trouble. - // We use a no-op callable and a minimal stack size because, although - // CoroData's constructor in fact initializes its mCoro with a - // coroutine with that stack size, no one ever actually enters it by - // calling mCoro(). - sCurrent.reset(new CoroData(0, // no prev - "", // not a named coroutine - no_op, // no-op callable - 1024)); // stacksize moot + // to mean "not in a coroutine," empty string should suffice here. + static CoroData sMain(""); + // We need not reset() the local_ptr to this read-only data: reuse the + // same instance for every thread's main coroutine. + current = &sMain; } - - mCurrent = &sCurrent; + return *current; } -//static LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) { - CoroData* current = Current(); - // With the dummy CoroData set in LLCoros::Current::Current(), this - // pointer should never be NULL. - llassert_always(current); - return *current; + // reuse const implementation, just cast away const-ness of result + return const_cast(const_cast(this)->get_CoroData(caller)); } //static -LLCoros::coro::self& LLCoros::get_self() +LLCoros::coro::id LLCoros::get_self() { - CoroData& current = get_CoroData("get_self()"); - if (! current.mSelf) - { - LL_ERRS("LLCoros") << "Calling get_self() from non-coroutine context!" << LL_ENDL; - } - return *current.mSelf; + return boost::this_fiber::get_id(); } //static void LLCoros::set_consuming(bool consuming) { - get_CoroData("set_consuming()").mConsuming = consuming; + CoroData& data(LLCoros::instance().get_CoroData("set_consuming()")); + // DO NOT call this on the main() coroutine. + llassert_always(! data.mName.empty()); + data.mConsuming = consuming; } //static bool LLCoros::get_consuming() { - return get_CoroData("get_consuming()").mConsuming; -} - -llcoro::Suspending::Suspending() -{ - LLCoros::Current current; - // Remember currently-running coroutine: we're about to suspend it. - mSuspended = current; - // Revert Current to the value it had at the moment we last switched - // into this coroutine. - current.reset(mSuspended->mPrev); -} - -llcoro::Suspending::~Suspending() -{ - LLCoros::Current current; - // Okay, we're back, update our mPrev - mSuspended->mPrev = current; - // and reinstate our Current. - current.reset(mSuspended); + return LLCoros::instance().get_CoroData("get_consuming()").mConsuming; } LLCoros::LLCoros(): // MAINT-2724: default coroutine stack size too small on Windows. // Previously we used // boost::context::guarded_stack_allocator::default_stacksize(); - // empirically this is 64KB on Windows and Linux. Try quadrupling. + // empirically this is insufficient. #if ADDRESS_SIZE == 64 mStackSize(512*1024) #else mStackSize(256*1024) #endif { - // Register our cleanup() method for "mainloop" ticks - LLEventPumps::instance().obtain("mainloop").listen( - "LLCoros", boost::bind(&LLCoros::cleanup, this, _1)); -} - -bool LLCoros::cleanup(const LLSD&) -{ - static std::string previousName; - static int previousCount = 0; - // Walk the mCoros map, checking and removing completed coroutines. - for (CoroMap::iterator mi(mCoros.begin()), mend(mCoros.end()); mi != mend; ) - { - // Has this coroutine exited (normal return, exception, exit() call) - // since last tick? - if (mi->second->mCoro.exited()) - { - if (previousName != mi->first) - { - previousName = mi->first; - previousCount = 1; - } - else - { - ++previousCount; - } - - if ((previousCount < 5) || !(previousCount % 50)) - { - if (previousCount < 5) - LL_DEBUGS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << LL_ENDL; - else - LL_DEBUGS("LLCoros") << "LLCoros: cleaning up coroutine " << mi->first << "("<< previousCount << ")" << LL_ENDL; - - } - // The erase() call will invalidate its passed iterator value -- - // so increment mi FIRST -- but pass its original value to - // erase(). This is what postincrement is all about. - mCoros.erase(mi++); - } - else - { - // Still live, just skip this entry as if incrementing at the top - // of the loop as usual. - ++mi; - } - } - return false; } std::string LLCoros::generateDistinctName(const std::string& prefix) const { - static std::string previousName; - static int previousCount = 0; + static int unique = 0; // Allowing empty name would make getName()'s not-found return ambiguous. if (prefix.empty()) @@ -225,37 +129,15 @@ std::string LLCoros::generateDistinctName(const std::string& prefix) const // If the specified name isn't already in the map, just use that. std::string name(prefix); - // Find the lowest numeric suffix that doesn't collide with an existing - // entry. Start with 2 just to make it more intuitive for any interested - // parties: e.g. "joe", "joe2", "joe3"... - for (int i = 2; ; name = STRINGIZE(prefix << i++)) + // Until we find an unused name, append a numeric suffix for uniqueness. + while (mCoros.find(name) != mCoros.end()) { - if (mCoros.find(name) == mCoros.end()) - { - if (previousName != name) - { - previousName = name; - previousCount = 1; - } - else - { - ++previousCount; - } - - if ((previousCount < 5) || !(previousCount % 50)) - { - if (previousCount < 5) - LL_DEBUGS("LLCoros") << "LLCoros: launching coroutine " << name << LL_ENDL; - else - LL_DEBUGS("LLCoros") << "LLCoros: launching coroutine " << name << "(" << previousCount << ")" << LL_ENDL; - - } - - return name; - } + name = STRINGIZE(prefix << unique++); } + return name; } +/*==========================================================================*| bool LLCoros::kill(const std::string& name) { CoroMap::iterator found = mCoros.find(name); @@ -269,10 +151,11 @@ bool LLCoros::kill(const std::string& name) mCoros.erase(found); return true; } +|*==========================================================================*/ std::string LLCoros::getName() const { - return Current()->mName; + return get_CoroData("getName()").mName; } void LLCoros::setStackSize(S32 stacksize) @@ -300,6 +183,27 @@ void LLCoros::printActiveCoroutines() } } +std::string LLCoros::launch(const std::string& prefix, const callable_t& callable) +{ + std::string name(generateDistinctName(prefix)); + // 'dispatch' means: enter the new fiber immediately, returning here only + // when the fiber yields for whatever reason. + // std::allocator_arg is a flag to indicate that the following argument is + // a StackAllocator. + // protected_fixedsize_stack sets a guard page past the end of the new + // stack so that stack underflow will result in an access violation + // instead of weird, subtle, possibly undiagnosed memory stomps. + boost::fibers::fiber newCoro(boost::fibers::launch::dispatch, + std::allocator_arg, + boost::fibers::protected_fixedsize_stack(mStackSize), + [this, &name, &callable](){ toplevel(name, callable); }); + // You have two choices with a fiber instance: you can join() it or you + // can detach() it. If you try to destroy the instance before doing + // either, the program silently terminates. We don't need this handle. + newCoro.detach(); + return name; +} + #if LL_WINDOWS static const U32 STATUS_MSC_EXCEPTION = 0xE06D7363; // compiler specific @@ -340,10 +244,14 @@ void LLCoros::winlevel(const callable_t& callable) // Top-level wrapper around caller's coroutine callable. This function accepts // the coroutine library's implicit coro::self& parameter and saves it, but // does not pass it down to the caller's callable. -void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& callable) +void LLCoros::toplevel(const std::string& name, const callable_t& callable) { - // capture the 'self' param in CoroData - data->mSelf = &self; + CoroData* corodata = new CoroData(name); + // Store it in our pointer map. Oddly, must cast away const-ness of key. + mCoros.insert(const_cast(name), corodata); + // also set it as current + mCurrent.reset(corodata); + // run the code the caller actually wants in the coroutine try { @@ -358,70 +266,41 @@ void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& calla // Any uncaught exception derived from LLContinueError will be caught // here and logged. This coroutine will terminate but the rest of the // viewer will carry on. - LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << data->mName)); + LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << corodata->mName)); } catch (...) { // Any OTHER kind of uncaught exception will cause the viewer to // crash, hopefully informatively. - CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << data->mName)); + CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << corodata->mName)); } - // This cleanup isn't perfectly symmetrical with the way we initially set - // data->mPrev, but this is our last chance to reset Current. - Current().reset(data->mPrev); } -/***************************************************************************** -* MUST BE LAST -*****************************************************************************/ -// Turn off MSVC optimizations for just LLCoros::launch() -- see -// DEV-32777. But MSVC doesn't support push/pop for optimization flags as it -// does for warning suppression, and we really don't want to force -// optimization ON for other code even in Debug or RelWithDebInfo builds. - -#if LL_MSVC -// work around broken optimizations -#pragma warning(disable: 4748) -#pragma warning(disable: 4355) // 'this' used in initializer list: yes, intentionally -#pragma optimize("", off) -#endif // LL_MSVC - -LLCoros::CoroData::CoroData(CoroData* prev, const std::string& name, - const callable_t& callable, S32 stacksize): - mPrev(prev), +LLCoros::CoroData::CoroData(const std::string& name): mName(name), - // Wrap the caller's callable in our toplevel() function so we can manage - // Current appropriately at startup and shutdown of each coroutine. - mCoro(boost::bind(toplevel, _1, this, callable), stacksize), // don't consume events unless specifically directed mConsuming(false), - mSelf(0), mCreationTime(LLTimer::getTotalSeconds()) { } -std::string LLCoros::launch(const std::string& prefix, const callable_t& callable) +void LLCoros::delete_CoroData(CoroData* cdptr) { - std::string name(generateDistinctName(prefix)); - Current current; - // pass the current value of Current as previous context - CoroData* newCoro = new(std::nothrow) CoroData(current, name, callable, mStackSize); - if (newCoro == NULL) + // This custom cleanup function is necessarily static. Find and bind the + // LLCoros instance. + LLCoros& self(LLCoros::instance()); + // We set mCurrent on entry to a new fiber, expecting that the + // corresponding entry has already been stored in mCoros. It is an + // error if we do not find that entry. + CoroMap::iterator found = self.mCoros.find(cdptr->mName); + if (found == self.mCoros.end()) { - // Out of memory? - printActiveCoroutines(); - LL_ERRS("LLCoros") << "Failed to start coroutine: " << name << " Stacksize: " << mStackSize << " Total coroutines: " << mCoros.size() << LL_ENDL; + LL_ERRS("LLCoros") << "Coroutine '" << cdptr->mName << "' terminated " + << "without being stored in LLCoros::mCoros" + << LL_ENDL; } - // Store it in our pointer map - mCoros.insert(name, newCoro); - // also set it as current - current.reset(newCoro); - /* Run the coroutine until its first wait, then return here */ - (newCoro->mCoro)(std::nothrow); - return name; -} -#if LL_MSVC -// reenable optimizations -#pragma optimize("", on) -#endif // LL_MSVC + // Oh good, we found the mCoros entry. Erase it. Because it's a ptr_map, + // that will implicitly delete this CoroData. + self.mCoros.erase(found); +} diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index c551413811..678633497d 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -29,22 +29,13 @@ #if ! defined(LL_LLCOROS_H) #define LL_LLCOROS_H -#include -#include +#include +#include +#include #include "llsingleton.h" #include #include -#include -#include #include -#include -#include "llcoro_get_id.h" // for friend declaration - -// forward-declare helper class -namespace llcoro -{ -class Suspending; -} /** * Registry of named Boost.Coroutine instances @@ -76,19 +67,20 @@ class Suspending; * name prefix; from your prefix it generates a distinct name, registers the * new coroutine and returns the actual name. * - * The name can be used to kill off the coroutine prematurely, if needed. It - * can also provide diagnostic info: we can look up the name of the + * The name + * can provide diagnostic info: we can look up the name of the * currently-running coroutine. - * - * Finally, the next frame ("mainloop" event) after the coroutine terminates, - * LLCoros will notice its demise and destroy it. */ class LL_COMMON_API LLCoros: public LLSingleton { LLSINGLETON(LLCoros); public: - /// Canonical boost::dcoroutines::coroutine signature we use - typedef boost::dcoroutines::coroutine coro; + /// The viewer's use of the term "coroutine" became deeply embedded before + /// the industry term "fiber" emerged to distinguish userland threads from + /// simpler, more transient kinds of coroutines. Semantically they've + /// always been fibers. But at this point in history, we're pretty much + /// stuck with the term "coroutine." + typedef boost::fibers::fiber coro; /// Canonical callable type typedef boost::function callable_t; @@ -119,10 +111,10 @@ public: * DEV-32777 comments for an explanation. * * Pass a nullary callable. It works to directly pass a nullary free - * function (or static method); for all other cases use boost::bind(). Of - * course, for a non-static class method, the first parameter must be the - * class instance. Any other parameters should be passed via the bind() - * expression. + * function (or static method); for other cases use a lambda expression, + * std::bind() or boost::bind(). Of course, for a non-static class method, + * the first parameter must be the class instance. Any other parameters + * should be passed via the enclosing expression. * * launch() tweaks the suggested name so it won't collide with any * existing coroutine instance, creates the coroutine instance, registers @@ -138,7 +130,7 @@ public: * one prematurely. Returns @c true if the specified name was found and * still running at the time. */ - bool kill(const std::string& name); +// bool kill(const std::string& name); /** * From within a coroutine, look up the (tweaked) name string by which @@ -148,14 +140,18 @@ public: */ std::string getName() const; - /// for delayed initialization + /** + * For delayed initialization. To be clear, this will only affect + * coroutines launched @em after this point. The underlying facility + * provides no way to alter the stack size of any running coroutine. + */ void setStackSize(S32 stacksize); /// for delayed initialization void printActiveCoroutines(); - /// get the current coro::self& for those who really really care - static coro::self& get_self(); + /// get the current coro::id for those who really really care + static coro::id get_self(); /** * Most coroutines, most of the time, don't "consume" the events for which @@ -190,141 +186,57 @@ public: }; /** - * Please do NOT directly use boost::dcoroutines::future! It is essential - * to maintain the "current" coroutine at every context switch. This - * Future wraps the essential boost::dcoroutines::future functionality - * with that maintenance. + * Aliases for promise and future. An older underlying future implementation + * required us to wrap future; that's no longer needed. However -- if it's + * important to restore kill() functionality, we might need to provide a + * proxy, so continue using the aliases. */ template - class Future; + using Promise = boost::fibers::promise; + template + using Future = boost::fibers::future; + template + static Future getFuture(Promise& promise) { return promise.get_future(); } + + /// for data local to each running coroutine + template + using local_ptr = boost::fibers::fiber_specific_ptr; private: - friend class llcoro::Suspending; - friend llcoro::id llcoro::get_id(); std::string generateDistinctName(const std::string& prefix) const; - bool cleanup(const LLSD&); + void toplevel(const std::string& name, const callable_t& callable); struct CoroData; - static void no_cleanup(CoroData*); #if LL_WINDOWS static void winlevel(const callable_t& callable); #endif - static void toplevel(coro::self& self, CoroData* data, const callable_t& callable); - static CoroData& get_CoroData(const std::string& caller); + CoroData& get_CoroData(const std::string& caller); + const CoroData& get_CoroData(const std::string& caller) const; S32 mStackSize; // coroutine-local storage, as it were: one per coro we track struct CoroData { - CoroData(CoroData* prev, const std::string& name, - const callable_t& callable, S32 stacksize); + CoroData(const std::string& name); - // The boost::dcoroutines library supports asymmetric coroutines. Every - // time we context switch out of a coroutine, we pass control to the - // previously-active one (or to the non-coroutine stack owned by the - // thread). So our management of the "current" coroutine must be able to - // restore the previous value when we're about to switch away. - CoroData* mPrev; // tweaked name of the current coroutine const std::string mName; - // the actual coroutine instance - LLCoros::coro mCoro; // set_consuming() state bool mConsuming; - // When the dcoroutine library calls a top-level callable, it implicitly - // passes coro::self& as the first parameter. All our consumer code used - // to explicitly pass coro::self& down through all levels of call stack, - // because at the leaf level we need it for context-switching. But since - // coroutines are based on cooperative switching, we can cause the - // top-level entry point to stash a pointer to the currently-running - // coroutine, and manage it appropriately as we switch out and back in. - // That eliminates the need to pass it as an explicit parameter down - // through every level, which is unfortunately viral in nature. Finding it - // implicitly rather than explicitly allows minor maintenance in which a - // leaf-level function adds a new async I/O call that suspends the calling - // coroutine, WITHOUT having to propagate coro::self& through every - // function signature down to that point -- and of course through every - // other caller of every such function. - LLCoros::coro::self* mSelf; F64 mCreationTime; // since epoch }; typedef boost::ptr_map CoroMap; CoroMap mCoros; - // Identify the current coroutine's CoroData. Use a little helper class so - // a caller can either use a temporary instance, or instantiate a named - // variable and access it multiple times. - class Current - { - public: - Current(); - - operator LLCoros::CoroData*() { return get(); } - LLCoros::CoroData* operator->() { return get(); } - LLCoros::CoroData* get() { return mCurrent->get(); } - void reset(LLCoros::CoroData* ptr) { mCurrent->reset(ptr); } - - private: - boost::thread_specific_ptr* mCurrent; - }; -}; - -namespace llcoro -{ + // Identify the current coroutine's CoroData. This local_ptr isn't static + // because it's a member of an LLSingleton, and we rely on it being + // cleaned up in proper dependency order. + // As each coroutine terminates, use our custom cleanup function to remove + // the corresponding entry from mCoros. + local_ptr mCurrent{delete_CoroData}; -/// Instantiate one of these in a block surrounding any leaf point when -/// control literally switches away from this coroutine. -class Suspending: boost::noncopyable -{ -public: - Suspending(); - ~Suspending(); - -private: - LLCoros::CoroData* mSuspended; -}; - -} // namespace llcoro - -template -class LLCoros::Future -{ - typedef boost::dcoroutines::future dfuture; - -public: - Future(): - mFuture(get_self()) - {} - - typedef typename boost::dcoroutines::make_callback_result::type callback_t; - - callback_t make_callback() - { - return boost::dcoroutines::make_callback(mFuture); - } - -#ifndef LL_LINUX - explicit -#endif - operator bool() const - { - return bool(mFuture); - } - - bool operator!() const - { - return ! mFuture; - } - - T get() - { - // instantiate Suspending to manage the "current" coroutine - llcoro::Suspending suspended; - return *mFuture; - } - -private: - dfuture mFuture; + // Cleanup function for each fiber's instance of mCurrent. + static void delete_CoroData(CoroData* cdptr); }; #endif /* ! defined(LL_LLCOROS_H) */ diff --git a/indra/llcommon/lleventcoro.cpp b/indra/llcommon/lleventcoro.cpp index 43e41f250d..47d99f0050 100644 --- a/indra/llcommon/lleventcoro.cpp +++ b/indra/llcommon/lleventcoro.cpp @@ -31,18 +31,15 @@ // associated header #include "lleventcoro.h" // STL headers -#include +#include // std headers // external library headers +#include // other Linden headers #include "llsdserialize.h" #include "llsdutil.h" #include "llerror.h" #include "llcoros.h" -#include "llmake.h" -#include "llexception.h" - -#include "lleventfilter.h" namespace { @@ -105,65 +102,47 @@ void storeToLLSDPath(LLSD& dest, const LLSD& path, const LLSD& value) llsd::drill(dest, path) = value; } -/// For LLCoros::Future::make_callback(), the callback has a signature -/// like void callback(LLSD), which isn't a valid LLEventPump listener: such -/// listeners must return bool. -template -class FutureListener -{ -public: - // FutureListener is instantiated on the coroutine stack: the stack, in - // other words, that wants to suspend. - FutureListener(const LISTENER& listener): - mListener(listener), - // Capture the suspending coroutine's flag as a consuming or - // non-consuming listener. - mConsume(LLCoros::get_consuming()) - {} - - // operator()() is called on the main stack: the stack on which the - // expected event is fired. - bool operator()(const LLSD& event) - { - mListener(event); - // tell upstream LLEventPump whether listener consumed - return mConsume; - } - -protected: - LISTENER mListener; - bool mConsume; -}; - } // anonymous void llcoro::suspend() { - // By viewer convention, we post an event on the "mainloop" LLEventPump - // each iteration of the main event-handling loop. So waiting for a single - // event on "mainloop" gives us a one-frame suspend. - suspendUntilEventOn("mainloop"); + boost::this_fiber::yield(); } void llcoro::suspendUntilTimeout(float seconds) { - LLEventTimeout timeout; - - timeout.eventAfter(seconds, LLSD()); - llcoro::suspendUntilEventOn(timeout); + // The fact that we accept non-integer seconds means we should probably + // use granularity finer than one second. However, given the overhead of + // the rest of our processing, it seems silly to use granularity finer + // than a millisecond. + boost::this_fiber::sleep_for(std::chrono::milliseconds(long(seconds * 1000))); } -LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump, const LLSD& replyPumpNamePath) +namespace { - // declare the future - LLCoros::Future future; + +LLBoundListener postAndSuspendSetup(const std::string& callerName, + const std::string& listenerName, + LLCoros::Promise& promise, + const LLSD& event, + const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, + const LLSD& replyPumpNamePath) +{ + // Get the consuming attribute for THIS coroutine, the one that's about to + // suspend. Don't call get_consuming() in the lambda body: that would + // return the consuming attribute for some other coroutine, most likely + // the main routine. + bool consuming(LLCoros::get_consuming()); // make a callback that will assign a value to the future, and listen on // the specified LLEventPump with that callback - std::string listenerName(listenerNameForCoro()); - LLTempBoundListener connection( + LLBoundListener connection( replyPump.getPump().listen(listenerName, - llmake(future.make_callback()))); + [&promise, consuming](const LLSD& result) + { + promise.set_value(result); + return consuming; + })); // skip the "post" part if requestPump is default-constructed if (requestPump) { @@ -171,7 +150,7 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ // request event. LLSD modevent(event); storeToLLSDPath(modevent, replyPumpNamePath, replyPump.getPump().getName()); - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName + LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName << " posting to " << requestPump.getPump().getName() << LL_ENDL; @@ -179,158 +158,73 @@ LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requ // << ": " << modevent << LL_ENDL; requestPump.getPump().post(modevent); } - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName + LL_DEBUGS("lleventcoro") << callerName << ": coroutine " << listenerName << " about to wait on LLEventPump " << replyPump.getPump().getName() << LL_ENDL; - // calling get() on the future makes us wait for it - LLSD value(future.get()); - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName - << " resuming with " << value << LL_ENDL; - // returning should disconnect the connection - return value; -} - -LLSD llcoro::suspendUntilEventOnWithTimeout(const LLEventPumpOrPumpName& suspendPumpOrName, - F32 timeoutin, const LLSD &timeoutResult) -{ - /** - * The timeout pump is attached upstream of of the waiting pump and will - * pass the timeout event through it. We CAN NOT attach downstream since - * doing so will cause the suspendPump to fire any waiting events immediately - * and they will be lost. This becomes especially problematic with the - * LLEventTimeout(pump) constructor which will also attempt to fire those - * events using the virtual listen_impl method in the not yet fully constructed - * timeoutPump. - */ - LLEventTimeout timeoutPump; - LLEventPump &suspendPump = suspendPumpOrName.getPump(); - - LLTempBoundListener timeoutListener(timeoutPump.listen(suspendPump.getName(), - boost::bind(&LLEventPump::post, &suspendPump, _1))); - - timeoutPump.eventAfter(timeoutin, timeoutResult); - return llcoro::suspendUntilEventOn(suspendPump); + return connection; } -namespace -{ - -/** - * This helper is specifically for postAndSuspend2(). We use a single future - * object, but we want to listen on two pumps with it. Since we must still - * adapt from the callable constructed by boost::dcoroutines::make_callback() - * (void return) to provide an event listener (bool return), we've adapted - * FutureListener for the purpose. The basic idea is that we construct a - * distinct instance of FutureListener2 -- binding different instance data -- - * for each of the pumps. Then, when a pump delivers an LLSD value to either - * FutureListener2, it can combine that LLSD with its discriminator to feed - * the future object. - * - * DISCRIM is a template argument so we can use llmake() rather than - * having to write our own argument-deducing helper function. - */ -template -class FutureListener2: public FutureListener -{ - typedef FutureListener super; - -public: - // instantiated on coroutine stack: the stack about to suspend - FutureListener2(const LISTENER& listener, DISCRIM discriminator): - super(listener), - mDiscrim(discriminator) - {} - - // called on main stack: the stack on which event is fired - bool operator()(const LLSD& event) - { - // our future object is defined to accept LLEventWithID - super::mListener(LLEventWithID(event, mDiscrim)); - // tell LLEventPump whether or not event was consumed - return super::mConsume; - } - -private: - const DISCRIM mDiscrim; -}; - } // anonymous -namespace llcoro +LLSD llcoro::postAndSuspend(const LLSD& event, const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, const LLSD& replyPumpNamePath) { + LLCoros::Promise promise; + std::string listenerName(listenerNameForCoro()); + + // Store connection into an LLTempBoundListener so we implicitly + // disconnect on return from this function. + LLTempBoundListener connection = + postAndSuspendSetup("postAndSuspend()", listenerName, promise, + event, requestPump, replyPump, replyPumpNamePath); -LLEventWithID postAndSuspend2(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump0, - const LLEventPumpOrPumpName& replyPump1, - const LLSD& replyPump0NamePath, - const LLSD& replyPump1NamePath) -{ // declare the future - LLCoros::Future future; - // either callback will assign a value to this future; listen on - // each specified LLEventPump with a callback - std::string name(listenerNameForCoro()); - LLTempBoundListener connection0( - replyPump0.getPump().listen( - name + "a", - llmake(future.make_callback(), 0))); - LLTempBoundListener connection1( - replyPump1.getPump().listen( - name + "b", - llmake(future.make_callback(), 1))); - // skip the "post" part if requestPump is default-constructed - if (requestPump) - { - // If either replyPumpNamePath is non-empty, store the corresponding - // replyPump name in the request event. - LLSD modevent(event); - storeToLLSDPath(modevent, replyPump0NamePath, - replyPump0.getPump().getName()); - storeToLLSDPath(modevent, replyPump1NamePath, - replyPump1.getPump().getName()); - LL_DEBUGS("lleventcoro") << "postAndSuspend2(): coroutine " << name - << " posting to " << requestPump.getPump().getName() - << ": " << modevent << LL_ENDL; - requestPump.getPump().post(modevent); - } - LL_DEBUGS("lleventcoro") << "postAndSuspend2(): coroutine " << name - << " about to wait on LLEventPumps " << replyPump0.getPump().getName() - << ", " << replyPump1.getPump().getName() << LL_ENDL; + LLCoros::Future future = LLCoros::getFuture(promise); // calling get() on the future makes us wait for it - LLEventWithID value(future.get()); - LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << name - << " resuming with (" << value.first << ", " << value.second << ")" - << LL_ENDL; - // returning should disconnect both connections + LLSD value(future.get()); + LL_DEBUGS("lleventcoro") << "postAndSuspend(): coroutine " << listenerName + << " resuming with " << value << LL_ENDL; + // returning should disconnect the connection return value; } -LLSD errorException(const LLEventWithID& result, const std::string& desc) +LLSD llcoro::postAndSuspendWithTimeout(const LLSD& event, + const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, + const LLSD& replyPumpNamePath, + F32 timeout, const LLSD& timeoutResult) { - // If the result arrived on the error pump (pump 1), instead of - // returning it, deliver it via exception. - if (result.second) + LLCoros::Promise promise; + std::string listenerName(listenerNameForCoro()); + + // Store connection into an LLTempBoundListener so we implicitly + // disconnect on return from this function. + LLTempBoundListener connection = + postAndSuspendSetup("postAndSuspendWithTimeout()", listenerName, promise, + event, requestPump, replyPump, replyPumpNamePath); + + // declare the future + LLCoros::Future future = LLCoros::getFuture(promise); + // wait for specified timeout + boost::fibers::future_status status = + future.wait_for(std::chrono::milliseconds(long(timeout * 1000))); + // if the future is NOT yet ready, return timeoutResult instead + if (status == boost::fibers::future_status::timeout) { - LLTHROW(LLErrorEvent(desc, result.first)); + LL_DEBUGS("lleventcoro") << "postAndSuspendWithTimeout(): coroutine " << listenerName + << " timed out after " << timeout << " seconds," + << " resuming with " << timeoutResult << LL_ENDL; + return timeoutResult; } - // That way, our caller knows a simple return must be from the reply - // pump (pump 0). - return result.first; -} - -LLSD errorLog(const LLEventWithID& result, const std::string& desc) -{ - // If the result arrived on the error pump (pump 1), log it as a fatal - // error. - if (result.second) + else { - LL_ERRS("errorLog") << desc << ":" << std::endl; - LLSDSerialize::toPrettyXML(result.first, LL_CONT); - LL_CONT << LL_ENDL; + llassert_always(status == boost::fibers::future_status::ready); + + // future is now ready, no more waiting + LLSD value(future.get()); + LL_DEBUGS("lleventcoro") << "postAndSuspendWithTimeout(): coroutine " << listenerName + << " resuming with " << value << LL_ENDL; + // returning should disconnect the connection + return value; } - // A simple return must therefore be from the reply pump (pump 0). - return result.first; } - -} // namespace llcoro diff --git a/indra/llcommon/lleventcoro.h b/indra/llcommon/lleventcoro.h index 84827aab4a..c0fe8b094f 100644 --- a/indra/llcommon/lleventcoro.h +++ b/indra/llcommon/lleventcoro.h @@ -29,12 +29,8 @@ #if ! defined(LL_LLEVENTCORO_H) #define LL_LLEVENTCORO_H -#include #include -#include // std::pair #include "llevents.h" -#include "llerror.h" -#include "llexception.h" /** * Like LLListenerOrPumpName, this is a class intended for parameter lists: @@ -147,117 +143,29 @@ LLSD suspendUntilEventOn(const LLEventPumpOrPumpName& pump) return postAndSuspend(LLSD(), LLEventPumpOrPumpName(), pump); } +/// Like postAndSuspend(), but if we wait longer than @a timeout seconds, +/// stop waiting and return @a timeoutResult instead. +LLSD postAndSuspendWithTimeout(const LLSD& event, + const LLEventPumpOrPumpName& requestPump, + const LLEventPumpOrPumpName& replyPump, + const LLSD& replyPumpNamePath, + F32 timeout, const LLSD& timeoutResult); + /// Suspend the coroutine until an event is fired on the identified pump /// or the timeout duration has elapsed. If the timeout duration /// elapses the specified LLSD is returned. -LLSD suspendUntilEventOnWithTimeout(const LLEventPumpOrPumpName& suspendPumpOrName, F32 timeoutin, const LLSD &timeoutResult); - -} // namespace llcoro - -/// return type for two-pump variant of suspendUntilEventOn() -typedef std::pair LLEventWithID; - -namespace llcoro -{ - -/** - * This function waits for a reply on either of two specified LLEventPumps. - * Otherwise, it closely resembles postAndSuspend(); please see the documentation - * for that function for detailed parameter info. - * - * While we could have implemented the single-pump variant in terms of this - * one, there's enough added complexity here to make it worthwhile to give the - * single-pump variant its own straightforward implementation. Conversely, - * though we could use preprocessor logic to generate n-pump overloads up to - * BOOST_COROUTINE_WAIT_MAX, we don't foresee a use case. This two-pump - * overload exists because certain event APIs are defined in terms of a reply - * LLEventPump and an error LLEventPump. - * - * The LLEventWithID return value provides not only the received event, but - * the index of the pump on which it arrived (0 or 1). - * - * @note - * I'd have preferred to overload the name postAndSuspend() for both signatures. - * But consider the following ambiguous call: - * @code - * postAndSuspend(LLSD(), requestPump, replyPump, "someString"); - * @endcode - * "someString" could be converted to either LLSD (@a replyPumpNamePath for - * the single-pump function) or LLEventOrPumpName (@a replyPump1 for two-pump - * function). - * - * It seems less burdensome to write postAndSuspend2() than to write either - * LLSD("someString") or LLEventOrPumpName("someString"). - */ -LLEventWithID postAndSuspend2(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLEventPumpOrPumpName& replyPump0, - const LLEventPumpOrPumpName& replyPump1, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()); - -/** - * Wait for the next event on either of two specified LLEventPumps. - */ inline -LLEventWithID -suspendUntilEventOn(const LLEventPumpOrPumpName& pump0, const LLEventPumpOrPumpName& pump1) +LLSD suspendUntilEventOnWithTimeout(const LLEventPumpOrPumpName& suspendPumpOrName, + F32 timeoutin, const LLSD &timeoutResult) { - // This is now a convenience wrapper for postAndSuspend2(). - return postAndSuspend2(LLSD(), LLEventPumpOrPumpName(), pump0, pump1); + return postAndSuspendWithTimeout(LLSD(), // event + LLEventPumpOrPumpName(), // requestPump + suspendPumpOrName, // replyPump + LLSD(), // replyPumpNamePath + timeoutin, + timeoutResult); } -/** - * Helper for the two-pump variant of suspendUntilEventOn(), e.g.: - * - * @code - * LLSD reply = errorException(suspendUntilEventOn(replyPump, errorPump), - * "error response from login.cgi"); - * @endcode - * - * Examines an LLEventWithID, assuming that the second pump (pump 1) is - * listening for an error indication. If the incoming data arrived on pump 1, - * throw an LLErrorEvent exception. If the incoming data arrived on pump 0, - * just return it. Since a normal return can only be from pump 0, we no longer - * need the LLEventWithID's discriminator int; we can just return the LLSD. - * - * @note I'm not worried about introducing the (fairly generic) name - * errorException() into global namespace, because how many other overloads of - * the same name are going to accept an LLEventWithID parameter? - */ -LLSD errorException(const LLEventWithID& result, const std::string& desc); - -} // namespace llcoro - -/** - * Exception thrown by errorException(). We don't call this LLEventError - * because it's not an error in event processing: rather, this exception - * announces an event that bears error information (for some other API). - */ -class LL_COMMON_API LLErrorEvent: public LLException -{ -public: - LLErrorEvent(const std::string& what, const LLSD& data): - LLException(what), - mData(data) - {} - virtual ~LLErrorEvent() throw() {} - - LLSD getData() const { return mData; } - -private: - LLSD mData; -}; - -namespace llcoro -{ - -/** - * Like errorException(), save that this trips a fatal error using LL_ERRS - * rather than throwing an exception. - */ -LL_COMMON_API LLSD errorLog(const LLEventWithID& result, const std::string& desc); - } // namespace llcoro /** @@ -304,84 +212,4 @@ private: LLEventStream mPump; }; -/** - * Other event APIs require the names of two different LLEventPumps: one for - * success response, the other for error response. Extend LLCoroEventPump - * for the two-pump use case. - */ -class LL_COMMON_API LLCoroEventPumps -{ -public: - LLCoroEventPumps(const std::string& name="coro", - const std::string& suff0="Reply", - const std::string& suff1="Error"): - mPump0(name + suff0, true), // allow tweaking the pump instance name - mPump1(name + suff1, true) - {} - /// request pump 0's name - std::string getName0() const { return mPump0.getName(); } - /// request pump 1's name - std::string getName1() const { return mPump1.getName(); } - /// request both names - std::pair getNames() const - { - return std::pair(mPump0.getName(), mPump1.getName()); - } - - /// request pump 0 - LLEventPump& getPump0() { return mPump0; } - /// request pump 1 - LLEventPump& getPump1() { return mPump1; } - - /// suspendUntilEventOn(either of our two LLEventPumps) - LLEventWithID suspend() - { - return llcoro::suspendUntilEventOn(mPump0, mPump1); - } - - /// errorException(suspend()) - LLSD suspendWithException() - { - return llcoro::errorException(suspend(), std::string("Error event on ") + getName1()); - } - - /// errorLog(suspend()) - LLSD suspendWithLog() - { - return llcoro::errorLog(suspend(), std::string("Error event on ") + getName1()); - } - - LLEventWithID postAndSuspend(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()) - { - return llcoro::postAndSuspend2(event, requestPump, mPump0, mPump1, - replyPump0NamePath, replyPump1NamePath); - } - - LLSD postAndSuspendWithException(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()) - { - return llcoro::errorException(postAndSuspend(event, requestPump, - replyPump0NamePath, replyPump1NamePath), - std::string("Error event on ") + getName1()); - } - - LLSD postAndSuspendWithLog(const LLSD& event, - const LLEventPumpOrPumpName& requestPump, - const LLSD& replyPump0NamePath=LLSD(), - const LLSD& replyPump1NamePath=LLSD()) - { - return llcoro::errorLog(postAndSuspend(event, requestPump, - replyPump0NamePath, replyPump1NamePath), - std::string("Error event on ") + getName1()); - } - -private: - LLEventStream mPump0, mPump1; -}; - #endif /* ! defined(LL_LLEVENTCORO_H) */ diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index f5f3aec270..356b896163 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -30,10 +30,9 @@ #include "llerror.h" #include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" -#include "llcoro_get_id.h" #include "llexception.h" +#include "llcoros.h" #include -#include #include #include // std::cerr in dire emergency #include @@ -115,19 +114,10 @@ private: // initialized, either in the constructor or in initSingleton(). However, // managing that as a stack depends on having a DISTINCT 'initializing' // stack for every C++ stack in the process! And we have a distinct C++ - // stack for every running coroutine. It would be interesting and cool to - // implement a generic coroutine-local-storage mechanism and use that - // here. The trouble is that LLCoros is itself an LLSingleton, so - // depending on LLCoros functionality could dig us into infinite - // recursion. (Moreover, when we reimplement LLCoros on top of - // Boost.Fiber, that library already provides fiber_specific_ptr -- so - // it's not worth a great deal of time and energy implementing a generic - // equivalent on top of boost::dcoroutine, which is on its way out.) - // Instead, use a map of llcoro::id to select the appropriate - // coro-specific 'initializing' stack. llcoro::get_id() is carefully - // implemented to avoid requiring LLCoros. - typedef boost::unordered_map InitializingMap; - InitializingMap mInitializing; + // stack for every running coroutine. Therefore this stack must be based + // on a coroutine-local pointer. + // This local_ptr isn't static because it's a member of an LLSingleton. + LLCoros::local_ptr mInitializing; public: // Instantiate this to obtain a reference to the coroutine-specific @@ -166,18 +156,23 @@ public: private: list_t& get_initializing_() { - // map::operator[] has find-or-create semantics, exactly what we need - // here. It returns a reference to the selected mapped_type instance. - return mInitializing[llcoro::get_id()]; + LLSingletonBase::list_t* current = mInitializing.get(); + if (! current) + { + // If the running coroutine doesn't already have an initializing + // stack, allocate a new one and save it for future reference. + current = new LLSingletonBase::list_t(); + mInitializing.reset(current); + } + return *current; } + // By the time mInitializing is destroyed, its value for every coroutine + // except the running one must have been reset() to nullptr. So every time + // we pop the list to empty, reset() the running coroutine's local_ptr. void cleanup_initializing_() { - InitializingMap::iterator found = mInitializing.find(llcoro::get_id()); - if (found != mInitializing.end()) - { - mInitializing.erase(found); - } + mInitializing.reset(nullptr); } }; diff --git a/indra/llcommon/tests/lleventcoro_test.cpp b/indra/llcommon/tests/lleventcoro_test.cpp index fa02d2bb1a..2e4b6ba823 100644 --- a/indra/llcommon/tests/lleventcoro_test.cpp +++ b/indra/llcommon/tests/lleventcoro_test.cpp @@ -26,50 +26,12 @@ * $/LicenseInfo$ */ -/*****************************************************************************/ -// test<1>() is cloned from a Boost.Coroutine example program whose copyright -// info is reproduced here: -/*---------------------------------------------------------------------------*/ -// Copyright (c) 2006, Giovanni P. Deretta -// -// This code may be used under either of the following two licences: -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL -// THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. OF SUCH DAMAGE. -// -// Or: -// -// Distributed under the Boost Software License, Version 1.0. -// (See accompanying file LICENSE_1_0.txt or copy at -// http://www.boost.org/LICENSE_1_0.txt) -/*****************************************************************************/ - #define BOOST_RESULT_OF_USE_TR1 1 -// On some platforms, Boost.Coroutine must #define magic symbols before -// #including platform-API headers. Naturally, that's ineffective unless the -// Boost.Coroutine #include is the *first* #include of the platform header. -// That means that client code must generally #include Boost.Coroutine headers -// before anything else. -#include #include #include #include #include +#include #include "linden_common.h" @@ -80,47 +42,12 @@ #include "llsd.h" #include "llsdutil.h" #include "llevents.h" -#include "tests/wrapllerrs.h" -#include "stringize.h" #include "llcoros.h" #include "lleventcoro.h" #include "../test/debug.h" using namespace llcoro; -/***************************************************************************** -* from the banana.cpp example program borrowed for test<1>() -*****************************************************************************/ -namespace coroutines = boost::dcoroutines; -using coroutines::coroutine; - -template -bool match(Iter first, Iter last, std::string match) { - std::string::iterator i = match.begin(); - for(; (first != last) && (i != match.end()); ++i) { - if (*first != *i) - return false; - ++first; - } - return i == match.end(); -} - -template -BidirectionalIterator -match_substring(BidirectionalIterator begin, - BidirectionalIterator end, - std::string xmatch, - BOOST_DEDUCED_TYPENAME coroutine::self& self) { -//BidirectionalIterator begin_ = begin; - for(; begin != end; ++begin) - if(match(begin, end, xmatch)) { - self.yield(begin); - } - return end; -} - -typedef coroutine match_coroutine_type; - /***************************************************************************** * Test helpers *****************************************************************************/ @@ -150,6 +77,8 @@ public: LLSD::Integer value(event["value"]); LLSD::String replyPumpName(event.has("fail")? "error" : "reply"); LLEventPumps::instance().obtain(event[replyPumpName]).post(value + 1); + // give listener a chance to process + llcoro::suspend(); return false; } @@ -167,51 +96,6 @@ namespace tut typedef coroutine_group::object object; coroutine_group coroutinegrp("coroutine"); - template<> template<> - void object::test<1>() - { - set_test_name("From banana.cpp example program in Boost.Coroutine distro"); - std::string buffer = "banananana"; - std::string match = "nana"; - std::string::iterator begin = buffer.begin(); - std::string::iterator end = buffer.end(); - -#if defined(BOOST_CORO_POSIX_IMPL) -// std::cout << "Using Boost.Coroutine " << BOOST_CORO_POSIX_IMPL << '\n'; -#else -// std::cout << "Using non-Posix Boost.Coroutine implementation" << std::endl; -#endif - - typedef std::string::iterator signature(std::string::iterator, - std::string::iterator, - std::string, - match_coroutine_type::self&); - - coroutine matcher - (boost::bind(static_cast(match_substring), - begin, - end, - match, - _1)); - - std::string::iterator i = matcher(); -/*==========================================================================*| - while(matcher && i != buffer.end()) { - std::cout <<"Match at: "<< std::distance(buffer.begin(), i)<<'\n'; - i = matcher(); - } -|*==========================================================================*/ - size_t matches[] = { 2, 4, 6 }; - for (size_t *mi(boost::begin(matches)), *mend(boost::end(matches)); - mi != mend; ++mi, i = matcher()) - { - ensure("more", matcher); - ensure("found", i != buffer.end()); - ensure_equals("value", std::distance(buffer.begin(), i), *mi); - } - ensure("done", ! matcher); - } - // use static data so we can intersperse coroutine functions with the // tests that engage them ImmediateAPI immediateAPI; @@ -231,7 +115,7 @@ namespace tut which = 0; } - void explicit_wait(boost::shared_ptr::callback_t>& cbp) + void explicit_wait(boost::shared_ptr>& cbp) { BEGIN { @@ -241,44 +125,40 @@ namespace tut // provides a callback-style notification (and prove that it // works). - LLCoros::Future future; - // get the callback from that future - LLCoros::Future::callback_t callback(future.make_callback()); - // Perhaps we would send a request to a remote server and arrange - // for 'callback' to be called on response. Of course that might - // involve an adapter object from the actual callback signature to - // the signature of 'callback' -- in this case, void(std::string). - // For test purposes, instead of handing 'callback' (or the + // for cbp->set_value() to be called on response. + // For test purposes, instead of handing 'callback' (or an // adapter) off to some I/O subsystem, we'll just pass it back to // our caller. - cbp.reset(new LLCoros::Future::callback_t(callback)); + cbp = boost::make_shared>(); + LLCoros::Future future = LLCoros::getFuture(*cbp); - ensure("Not yet", ! future); // calling get() on the future causes us to suspend debug("about to suspend"); stringdata = future.get(); - ensure("Got it", bool(future)); + ensure_equals("Got it", stringdata, "received"); } END } template<> template<> - void object::test<2>() + void object::test<1>() { clear(); set_test_name("explicit_wait"); DEBUG; // Construct the coroutine instance that will run explicit_wait. - boost::shared_ptr::callback_t> respond; - LLCoros::instance().launch("test<2>", + boost::shared_ptr> respond; + LLCoros::instance().launch("test<1>", boost::bind(explicit_wait, boost::ref(respond))); // When the coroutine waits for the future, it returns here. debug("about to respond"); - // Now we're the I/O subsystem delivering a result. This immediately - // transfers control back to the coroutine. - (*respond)("received"); + // Now we're the I/O subsystem delivering a result. This should make + // the coroutine ready. + respond->set_value("received"); + // but give it a chance to wake up + llcoro::suspend(); // ensure the coroutine ran and woke up again with the intended result ensure_equals(stringdata, "received"); } @@ -293,60 +173,20 @@ namespace tut } template<> template<> - void object::test<3>() + void object::test<2>() { clear(); set_test_name("waitForEventOn1"); DEBUG; - LLCoros::instance().launch("test<3>", waitForEventOn1); + LLCoros::instance().launch("test<2>", waitForEventOn1); debug("about to send"); LLEventPumps::instance().obtain("source").post("received"); + // give waitForEventOn1() a chance to run + llcoro::suspend(); debug("back from send"); ensure_equals(result.asString(), "received"); } - void waitForEventOn2() - { - BEGIN - { - LLEventWithID pair = suspendUntilEventOn("reply", "error"); - result = pair.first; - which = pair.second; - debug(STRINGIZE("result = " << result << ", which = " << which)); - } - END - } - - template<> template<> - void object::test<4>() - { - clear(); - set_test_name("waitForEventOn2 reply"); - { - DEBUG; - LLCoros::instance().launch("test<4>", waitForEventOn2); - debug("about to send"); - LLEventPumps::instance().obtain("reply").post("received"); - debug("back from send"); - } - ensure_equals(result.asString(), "received"); - ensure_equals("which pump", which, 0); - } - - template<> template<> - void object::test<5>() - { - clear(); - set_test_name("waitForEventOn2 error"); - DEBUG; - LLCoros::instance().launch("test<5>", waitForEventOn2); - debug("about to send"); - LLEventPumps::instance().obtain("error").post("badness"); - debug("back from send"); - ensure_equals(result.asString(), "badness"); - ensure_equals("which pump", which, 1); - } - void coroPump() { BEGIN @@ -359,175 +199,20 @@ namespace tut } template<> template<> - void object::test<6>() + void object::test<3>() { clear(); set_test_name("coroPump"); DEBUG; - LLCoros::instance().launch("test<6>", coroPump); - debug("about to send"); - LLEventPumps::instance().obtain(replyName).post("received"); - debug("back from send"); - ensure_equals(result.asString(), "received"); - } - - void coroPumps() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - LLEventWithID pair(waiter.suspend()); - result = pair.first; - which = pair.second; - } - END - } - - template<> template<> - void object::test<7>() - { - clear(); - set_test_name("coroPumps reply"); - DEBUG; - LLCoros::instance().launch("test<7>", coroPumps); - debug("about to send"); - LLEventPumps::instance().obtain(replyName).post("received"); - debug("back from send"); - ensure_equals(result.asString(), "received"); - ensure_equals("which pump", which, 0); - } - - template<> template<> - void object::test<8>() - { - clear(); - set_test_name("coroPumps error"); - DEBUG; - LLCoros::instance().launch("test<8>", coroPumps); - debug("about to send"); - LLEventPumps::instance().obtain(errorName).post("badness"); - debug("back from send"); - ensure_equals(result.asString(), "badness"); - ensure_equals("which pump", which, 1); - } - - void coroPumpsNoEx() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - result = waiter.suspendWithException(); - } - END - } - - template<> template<> - void object::test<9>() - { - clear(); - set_test_name("coroPumpsNoEx"); - DEBUG; - LLCoros::instance().launch("test<9>", coroPumpsNoEx); - debug("about to send"); - LLEventPumps::instance().obtain(replyName).post("received"); - debug("back from send"); - ensure_equals(result.asString(), "received"); - } - - void coroPumpsEx() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - try - { - result = waiter.suspendWithException(); - debug("no exception"); - } - catch (const LLErrorEvent& e) - { - debug(STRINGIZE("exception " << e.what())); - errordata = e.getData(); - } - } - END - } - - template<> template<> - void object::test<10>() - { - clear(); - set_test_name("coroPumpsEx"); - DEBUG; - LLCoros::instance().launch("test<10>", coroPumpsEx); - debug("about to send"); - LLEventPumps::instance().obtain(errorName).post("badness"); - debug("back from send"); - ensure("no result", result.isUndefined()); - ensure_equals("got error", errordata.asString(), "badness"); - } - - void coroPumpsNoLog() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - result = waiter.suspendWithLog(); - } - END - } - - template<> template<> - void object::test<11>() - { - clear(); - set_test_name("coroPumpsNoLog"); - DEBUG; - LLCoros::instance().launch("test<11>", coroPumpsNoLog); + LLCoros::instance().launch("test<3>", coroPump); debug("about to send"); LLEventPumps::instance().obtain(replyName).post("received"); + // give coroPump() a chance to run + llcoro::suspend(); debug("back from send"); ensure_equals(result.asString(), "received"); } - void coroPumpsLog() - { - BEGIN - { - LLCoroEventPumps waiter; - replyName = waiter.getName0(); - errorName = waiter.getName1(); - WrapLLErrs capture; - threw = capture.catch_llerrs([&waiter, &debug](){ - result = waiter.suspendWithLog(); - debug("no exception"); - }); - } - END - } - - template<> template<> - void object::test<12>() - { - clear(); - set_test_name("coroPumpsLog"); - DEBUG; - LLCoros::instance().launch("test<12>", coroPumpsLog); - debug("about to send"); - LLEventPumps::instance().obtain(errorName).post("badness"); - debug("back from send"); - ensure("no result", result.isUndefined()); - ensure_contains("got error", threw, "badness"); - } - void postAndWait1() { BEGIN @@ -541,71 +226,17 @@ namespace tut } template<> template<> - void object::test<13>() + void object::test<4>() { clear(); set_test_name("postAndWait1"); DEBUG; - LLCoros::instance().launch("test<13>", postAndWait1); + LLCoros::instance().launch("test<4>", postAndWait1); + // give postAndWait1() a chance to run + llcoro::suspend(); ensure_equals(result.asInteger(), 18); } - void postAndWait2() - { - BEGIN - { - LLEventWithID pair = ::postAndSuspend2(LLSDMap("value", 18), - immediateAPI.getPump(), - "reply2", - "error2", - "reply", - "error"); - result = pair.first; - which = pair.second; - debug(STRINGIZE("result = " << result << ", which = " << which)); - } - END - } - - template<> template<> - void object::test<14>() - { - clear(); - set_test_name("postAndWait2"); - DEBUG; - LLCoros::instance().launch("test<14>", postAndWait2); - ensure_equals(result.asInteger(), 19); - ensure_equals(which, 0); - } - - void postAndWait2_1() - { - BEGIN - { - LLEventWithID pair = ::postAndSuspend2(LLSDMap("value", 18)("fail", LLSD()), - immediateAPI.getPump(), - "reply2", - "error2", - "reply", - "error"); - result = pair.first; - which = pair.second; - debug(STRINGIZE("result = " << result << ", which = " << which)); - } - END - } - - template<> template<> - void object::test<15>() - { - clear(); - set_test_name("postAndWait2_1"); - DEBUG; - LLCoros::instance().launch("test<15>", postAndWait2_1); - ensure_equals(result.asInteger(), 19); - ensure_equals(which, 1); - } - void coroPumpPost() { BEGIN @@ -618,177 +249,14 @@ namespace tut } template<> template<> - void object::test<16>() + void object::test<5>() { clear(); set_test_name("coroPumpPost"); DEBUG; - LLCoros::instance().launch("test<16>", coroPumpPost); + LLCoros::instance().launch("test<5>", coroPumpPost); + // give coroPumpPost() a chance to run + llcoro::suspend(); ensure_equals(result.asInteger(), 18); } - - void coroPumpsPost() - { - BEGIN - { - LLCoroEventPumps waiter; - LLEventWithID pair(waiter.postAndSuspend(LLSDMap("value", 23), - immediateAPI.getPump(), "reply", "error")); - result = pair.first; - which = pair.second; - } - END - } - - template<> template<> - void object::test<17>() - { - clear(); - set_test_name("coroPumpsPost reply"); - DEBUG; - LLCoros::instance().launch("test<17>", coroPumpsPost); - ensure_equals(result.asInteger(), 24); - ensure_equals("which pump", which, 0); - } - - void coroPumpsPost_1() - { - BEGIN - { - LLCoroEventPumps waiter; - LLEventWithID pair( - waiter.postAndSuspend(LLSDMap("value", 23)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error")); - result = pair.first; - which = pair.second; - } - END - } - - template<> template<> - void object::test<18>() - { - clear(); - set_test_name("coroPumpsPost error"); - DEBUG; - LLCoros::instance().launch("test<18>", coroPumpsPost_1); - ensure_equals(result.asInteger(), 24); - ensure_equals("which pump", which, 1); - } - - void coroPumpsPostNoEx() - { - BEGIN - { - LLCoroEventPumps waiter; - result = waiter.postAndSuspendWithException(LLSDMap("value", 8), - immediateAPI.getPump(), "reply", "error"); - } - END - } - - template<> template<> - void object::test<19>() - { - clear(); - set_test_name("coroPumpsPostNoEx"); - DEBUG; - LLCoros::instance().launch("test<19>", coroPumpsPostNoEx); - ensure_equals(result.asInteger(), 9); - } - - void coroPumpsPostEx() - { - BEGIN - { - LLCoroEventPumps waiter; - try - { - result = waiter.postAndSuspendWithException( - LLSDMap("value", 9)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error"); - debug("no exception"); - } - catch (const LLErrorEvent& e) - { - debug(STRINGIZE("exception " << e.what())); - errordata = e.getData(); - } - } - END - } - - template<> template<> - void object::test<20>() - { - clear(); - set_test_name("coroPumpsPostEx"); - DEBUG; - LLCoros::instance().launch("test<20>", coroPumpsPostEx); - ensure("no result", result.isUndefined()); - ensure_equals("got error", errordata.asInteger(), 10); - } - - void coroPumpsPostNoLog() - { - BEGIN - { - LLCoroEventPumps waiter; - result = waiter.postAndSuspendWithLog(LLSDMap("value", 30), - immediateAPI.getPump(), "reply", "error"); - } - END - } - - template<> template<> - void object::test<21>() - { - clear(); - set_test_name("coroPumpsPostNoLog"); - DEBUG; - LLCoros::instance().launch("test<21>", coroPumpsPostNoLog); - ensure_equals(result.asInteger(), 31); - } - - void coroPumpsPostLog() - { - BEGIN - { - LLCoroEventPumps waiter; - WrapLLErrs capture; - threw = capture.catch_llerrs( - [&waiter, &debug](){ - result = waiter.postAndSuspendWithLog( - LLSDMap("value", 31)("fail", LLSD()), - immediateAPI.getPump(), "reply", "error"); - debug("no exception"); - }); - } - END - } - - template<> template<> - void object::test<22>() - { - clear(); - set_test_name("coroPumpsPostLog"); - DEBUG; - LLCoros::instance().launch("test<22>", coroPumpsPostLog); - ensure("no result", result.isUndefined()); - ensure_contains("got error", threw, "32"); - } } - -/*==========================================================================*| -#include - -namespace tut -{ - template<> template<> - void object::test<23>() - { - set_test_name("stacksize"); - std::cout << "default_stacksize: " << boost::context::guarded_stack_allocator::default_stacksize() << '\n'; - } -} // namespace tut -|*==========================================================================*/ diff --git a/indra/llmessage/CMakeLists.txt b/indra/llmessage/CMakeLists.txt index e0922c0667..a2a57ad740 100644 --- a/indra/llmessage/CMakeLists.txt +++ b/indra/llmessage/CMakeLists.txt @@ -217,7 +217,7 @@ target_link_libraries( ${NGHTTP2_LIBRARIES} ${XMLRPCEPI_LIBRARIES} ${LLCOREHTTP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} rt @@ -235,7 +235,7 @@ target_link_libraries( ${NGHTTP2_LIBRARIES} ${XMLRPCEPI_LIBRARIES} ${LLCOREHTTP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) @@ -264,7 +264,7 @@ if (LINUX) ${LLMESSAGE_LIBRARIES} ${LLCOREHTTP_LIBRARIES} ${JSONCPP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} rt ${GOOGLEMOCK_LIBRARIES} @@ -280,7 +280,7 @@ else (LINUX) ${LLMESSAGE_LIBRARIES} ${LLCOREHTTP_LIBRARIES} ${JSONCPP_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${GOOGLEMOCK_LIBRARIES} ) diff --git a/indra/llmessage/llcoproceduremanager.cpp b/indra/llmessage/llcoproceduremanager.cpp index 74cdff2b00..4c85dd999a 100644 --- a/indra/llmessage/llcoproceduremanager.cpp +++ b/indra/llmessage/llcoproceduremanager.cpp @@ -203,6 +203,7 @@ void LLCoprocedureManager::cancelCoprocedure(const LLUUID &id) LL_INFOS() << "Coprocedure not found." << LL_ENDL; } +/*==========================================================================*| void LLCoprocedureManager::shutdown(bool hardShutdown) { for (poolMap_t::const_iterator it = mPoolMap.begin(); it != mPoolMap.end(); ++it) @@ -211,6 +212,7 @@ void LLCoprocedureManager::shutdown(bool hardShutdown) } mPoolMap.clear(); } +|*==========================================================================*/ void LLCoprocedureManager::setPropertyMethods(SettingQuery_t queryfn, SettingUpdate_t updatefn) { @@ -303,10 +305,13 @@ LLCoprocedurePool::LLCoprocedurePool(const std::string &poolName, size_t size): LLCoprocedurePool::~LLCoprocedurePool() { +/*==========================================================================*| shutdown(); +|*==========================================================================*/ } //------------------------------------------------------------------------- +/*==========================================================================*| void LLCoprocedurePool::shutdown(bool hardShutdown) { CoroAdapterMap_t::iterator it; @@ -327,6 +332,7 @@ void LLCoprocedurePool::shutdown(bool hardShutdown) mCoroMapping.clear(); mPendingCoprocs.clear(); } +|*==========================================================================*/ //------------------------------------------------------------------------- LLUUID LLCoprocedurePool::enqueueCoprocedure(const std::string &name, LLCoprocedurePool::CoProcedure_t proc) diff --git a/indra/llmessage/llcoproceduremanager.h b/indra/llmessage/llcoproceduremanager.h index 7d0e83180c..ba6f97355c 100644 --- a/indra/llmessage/llcoproceduremanager.h +++ b/indra/llmessage/llcoproceduremanager.h @@ -59,9 +59,11 @@ public: /// If it has not yet been dequeued it is simply removed from the queue. void cancelCoprocedure(const LLUUID &id); +/*==========================================================================*| /// Requests a shutdown of the upload manager. Passing 'true' will perform /// an immediate kill on the upload coroutine. void shutdown(bool hardShutdown = false); +|*==========================================================================*/ void setPropertyMethods(SettingQuery_t queryfn, SettingUpdate_t updatefn); diff --git a/indra/llprimitive/CMakeLists.txt b/indra/llprimitive/CMakeLists.txt index dd2e806dda..7b6d04b096 100644 --- a/indra/llprimitive/CMakeLists.txt +++ b/indra/llprimitive/CMakeLists.txt @@ -80,7 +80,7 @@ target_link_libraries(llprimitive ${LLXML_LIBRARIES} ${LLPHYSICSEXTENSIONS_LIBRARIES} ${LLCHARACTER_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ) diff --git a/indra/llui/CMakeLists.txt b/indra/llui/CMakeLists.txt index e44f57fa9f..2d2fa6588f 100644 --- a/indra/llui/CMakeLists.txt +++ b/indra/llui/CMakeLists.txt @@ -299,7 +299,7 @@ if(LL_TESTS) set(test_libs llui llmessage llcorehttp llcommon ${HUNSPELL_LIBRARY} ${LLCOMMON_LIBRARIES} - ${BOOST_COROUTINE_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${WINDOWS_LIBRARIES}) if(NOT LINUX) LL_ADD_INTEGRATION_TEST(llurlentry llurlentry.cpp "${test_libs}") diff --git a/indra/mac_crash_logger/CMakeLists.txt b/indra/mac_crash_logger/CMakeLists.txt index f6c4dfb59d..95637c9a28 100644 --- a/indra/mac_crash_logger/CMakeLists.txt +++ b/indra/mac_crash_logger/CMakeLists.txt @@ -77,7 +77,7 @@ target_link_libraries(mac-crash-logger ${LLCOREHTTP_LIBRARIES} ${LLCOMMON_LIBRARIES} ${BOOST_CONTEXT_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ) add_custom_command( diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index dc0d737540..45f4cb269c 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2004,7 +2004,7 @@ target_link_libraries(${VIEWER_BINARY_NAME} ${viewer_LIBRARIES} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${DBUSGLIB_LIBRARIES} ${OPENGL_LIBRARIES} @@ -2484,7 +2484,7 @@ if (LL_TESTS) ${OPENSSL_LIBRARIES} ${CRYPTO_LIBRARIES} ${LIBRT_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index af70751b37..db2db43ee1 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -1422,6 +1422,8 @@ bool LLAppViewer::doFrame() // canonical per-frame event mainloop.post(newFrame); + // give listeners a chance to run + llcoro::suspend(); if (!LLApp::isExiting()) { diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 8344cead57..4187076030 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -98,7 +98,7 @@ target_link_libraries(lltest ${WINDOWS_LIBRARIES} ${BOOST_PROGRAM_OPTIONS_LIBRARY} ${BOOST_REGEX_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ${DL_LIBRARY} diff --git a/indra/viewer_components/login/CMakeLists.txt b/indra/viewer_components/login/CMakeLists.txt index 3bedeb7292..23518b791c 100644 --- a/indra/viewer_components/login/CMakeLists.txt +++ b/indra/viewer_components/login/CMakeLists.txt @@ -50,7 +50,7 @@ target_link_libraries(lllogin ${LLMATH_LIBRARIES} ${LLXML_LIBRARIES} ${BOOST_THREAD_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${BOOST_CONTEXT_LIBRARY} ${BOOST_SYSTEM_LIBRARY} ) @@ -62,7 +62,7 @@ if(LL_TESTS) set_source_files_properties( lllogin.cpp PROPERTIES - LL_TEST_ADDITIONAL_LIBRARIES "${LLMESSAGE_LIBRARIES};${LLCOREHTTP_LIBRARIES};${LLCOMMON_LIBRARIES};${BOOST_COROUTINE_LIBRARY};${BOOST_CONTEXT_LIBRARY};${BOOST_THREAD_LIBRARY};${BOOST_SYSTEM_LIBRARY}" + LL_TEST_ADDITIONAL_LIBRARIES "${LLMESSAGE_LIBRARIES};${LLCOREHTTP_LIBRARIES};${LLCOMMON_LIBRARIES};${BOOST_FIBER_LIBRARY};${BOOST_CONTEXT_LIBRARY};${BOOST_THREAD_LIBRARY};${BOOST_SYSTEM_LIBRARY}" ) LL_ADD_PROJECT_UNIT_TESTS(lllogin "${lllogin_TEST_SOURCE_FILES}") diff --git a/indra/viewer_components/login/tests/lllogin_test.cpp b/indra/viewer_components/login/tests/lllogin_test.cpp index e96c495446..774823d735 100644 --- a/indra/viewer_components/login/tests/lllogin_test.cpp +++ b/indra/viewer_components/login/tests/lllogin_test.cpp @@ -44,6 +44,7 @@ //#define DEBUG_ON #include "../../../test/debug.h" #include "llevents.h" +#include "lleventcoro.h" #include "stringize.h" #if LL_WINDOWS @@ -199,6 +200,7 @@ namespace tut credentials["passwd"] = "secret"; login.connect("login.bar.com", credentials); + llcoro::suspend(); ensure_equals("Online state", listener.lastEvent()["state"].asString(), "online"); } @@ -226,6 +228,7 @@ namespace tut credentials["passwd"] = "badpasswd"; login.connect("login.bar.com", credentials); + llcoro::suspend(); ensure_equals("Auth state", listener.lastEvent()["change"].asString(), "authenticating"); @@ -265,6 +268,7 @@ namespace tut credentials["passwd"] = "matter"; login.connect("login.bar.com", credentials); + llcoro::suspend(); ensure_equals("Auth state", listener.lastEvent()["change"].asString(), "authenticating"); @@ -300,6 +304,7 @@ namespace tut credentials["cfg_srv_timeout"] = 0.0f; login.connect("login.bar.com", credentials); + llcoro::suspend(); // Get the mainloop eventpump, which needs a pinging in order to drive the // SRV timeout. diff --git a/indra/win_crash_logger/CMakeLists.txt b/indra/win_crash_logger/CMakeLists.txt index 4fba26ab2f..1c3479bf69 100644 --- a/indra/win_crash_logger/CMakeLists.txt +++ b/indra/win_crash_logger/CMakeLists.txt @@ -83,7 +83,7 @@ target_link_libraries(windows-crash-logger ${LLCOREHTTP_LIBRARIES} ${LLCOMMON_LIBRARIES} ${BOOST_CONTEXT_LIBRARY} - ${BOOST_COROUTINE_LIBRARY} + ${BOOST_FIBER_LIBRARY} ${WINDOWS_LIBRARIES} ${DXGUID_LIBRARY} ${GOOGLE_PERFTOOLS_LIBRARIES} -- cgit v1.3 From 1efb4aefed4724c74a2579ee99bf21037ab6aaf1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 10 Dec 2019 11:09:02 -0500 Subject: DRTVWR-476: LLTHROW() requires LLException or subclass. --- indra/llcommon/llsingleton.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 356b896163..2439a6b5fd 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -135,8 +135,8 @@ public: { if (! mList) { - LLTHROW(std::runtime_error("Trying to use LockedInitializing " - "after cleanup_initializing()")); + LLTHROW(LLException("Trying to use LockedInitializing " + "after cleanup_initializing()")); } return *mList; } -- cgit v1.3 From c50db833656c424bb398cd91a46f478ca8b72b23 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 13 Dec 2019 14:12:42 -0500 Subject: DRTVWR-476: Fix merge glitch. --- indra/llcommon/llsingleton.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 2439a6b5fd..9846031512 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -117,7 +117,7 @@ private: // stack for every running coroutine. Therefore this stack must be based // on a coroutine-local pointer. // This local_ptr isn't static because it's a member of an LLSingleton. - LLCoros::local_ptr mInitializing; + LLCoros::local_ptr mInitializing; public: // Instantiate this to obtain a reference to the coroutine-specific -- cgit v1.3 From d1175e8fa1eb873e6accbb2f220686077cad38dc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 13 Dec 2019 15:24:56 -0500 Subject: DRTVWR-476: Log calls to LLParamSingleton::initParamSingleton(). --- indra/llcommon/llsingleton.cpp | 12 ++++++------ indra/llcommon/llsingleton.h | 8 +++++--- 2 files changed, 11 insertions(+), 9 deletions(-) (limited to 'indra/llcommon/llsingleton.cpp') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 9846031512..d3d25201b2 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -42,8 +42,6 @@ namespace { void log(LLError::ELevel level, const char* p1, const char* p2, const char* p3, const char* p4); -void logdebugs(const char* p1="", const char* p2="", const char* p3="", const char* p4=""); - bool oktolog(); } // anonymous namespace @@ -486,10 +484,6 @@ void log(LLError::ELevel level, } } -void logdebugs(const char* p1, const char* p2, const char* p3, const char* p4) -{ - log(LLError::LEVEL_DEBUG, p1, p2, p3, p4); -} } // anonymous namespace //static @@ -504,6 +498,12 @@ void LLSingletonBase::loginfos(const char* p1, const char* p2, const char* p3, c log(LLError::LEVEL_INFO, p1, p2, p3, p4); } +//static +void LLSingletonBase::logdebugs(const char* p1, const char* p2, const char* p3, const char* p4) +{ + log(LLError::LEVEL_DEBUG, p1, p2, p3, p4); +} + //static void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) { diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 65dd332afb..27c2ceb3b6 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -110,15 +110,15 @@ protected: // A::initSingleton(), record that A directly depends on B. void capture_dependency(); - // delegate LL_ERRS() logging to llsingleton.cpp + // delegate logging calls to llsingleton.cpp static void logerrs(const char* p1, const char* p2="", const char* p3="", const char* p4=""); - // delegate LL_WARNS() logging to llsingleton.cpp static void logwarns(const char* p1, const char* p2="", const char* p3="", const char* p4=""); - // delegate LL_INFOS() logging to llsingleton.cpp static void loginfos(const char* p1, const char* p2="", const char* p3="", const char* p4=""); + static void logdebugs(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); static std::string demangle(const char* mangled); template static std::string classname() { return demangle(typeid(T).name()); } @@ -647,6 +647,8 @@ private: else if (on_main_thread()) { // on the main thread, simply construct instance while holding lock + super::logdebugs(super::template classname().c_str(), + "::initParamSingleton()"); super::constructSingleton(lk, std::forward(args)...); return lk->mInstance; } -- cgit v1.3