From 107b9bcb70e785c2d12515e38b8b296eea7ab8d8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 20 May 2015 10:56:09 -0400 Subject: MAINT-5232: Introduce SUBSYSTEM_CLEANUP() macro and use it for existing LLSomeClass::cleanupClass() calls. This logs the fact of making the call, as well as making it. --- indra/llcommon/llapp.cpp | 3 ++- indra/llcommon/llcleanup.h | 30 ++++++++++++++++++++++++++++++ indra/llcommon/llcommon.cpp | 5 +++-- 3 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 indra/llcommon/llcleanup.h (limited to 'indra/llcommon') diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 5a40845e7d..2c52b11594 100755 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -48,6 +48,7 @@ #include "lleventtimer.h" #include "google_breakpad/exception_handler.h" #include "stringize.h" +#include "llcleanup.h" // // Signal handling @@ -177,7 +178,7 @@ LLApp::~LLApp() if(mExceptionHandler != 0) delete mExceptionHandler; - LLCommon::cleanupClass(); + SUBSYSTEM_CLEANUP(LLCommon); } // static diff --git a/indra/llcommon/llcleanup.h b/indra/llcommon/llcleanup.h new file mode 100644 index 0000000000..8eda9a7fb3 --- /dev/null +++ b/indra/llcommon/llcleanup.h @@ -0,0 +1,30 @@ +/** + * @file llcleanup.h + * @author Nat Goodspeed + * @date 2015-05-20 + * @brief Mechanism for cleaning up subsystem resources + * + * $LicenseInfo:firstyear=2015&license=viewerlgpl$ + * Copyright (c) 2015, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLCLEANUP_H) +#define LL_LLCLEANUP_H + +#include "llerror.h" + +// Instead of directly calling SomeClass::cleanupClass(), use +// SUBSYSTEM_CLEANUP(SomeClass); +// This logs the call as well as performing it. That gives us a baseline +// subsystem shutdown order against which to compare subsequent dynamic +// shutdown schemes. +#define SUBSYSTEM_CLEANUP(CLASSNAME) \ + do { \ + LL_INFOS("Cleanup") << "Calling " #CLASSNAME "::cleanupClass()" << LL_ENDL; \ + CLASSNAME::cleanupClass(); \ + } while (0) +// Use ancient do { ... } while (0) macro trick to permit a block of +// statements with the same syntax as a single statement. + +#endif /* ! defined(LL_LLCLEANUP_H) */ diff --git a/indra/llcommon/llcommon.cpp b/indra/llcommon/llcommon.cpp index 19642b0982..439ff4e628 100755 --- a/indra/llcommon/llcommon.cpp +++ b/indra/llcommon/llcommon.cpp @@ -31,6 +31,7 @@ #include "llthread.h" #include "lltrace.h" #include "lltracethreadrecorder.h" +#include "llcleanup.h" //static BOOL LLCommon::sAprInitialized = FALSE; @@ -63,11 +64,11 @@ void LLCommon::cleanupClass() sMasterThreadRecorder = NULL; LLTrace::set_master_thread_recorder(NULL); LLThreadSafeRefCount::cleanupThreadSafeRefCount(); - LLTimer::cleanupClass(); + SUBSYSTEM_CLEANUP(LLTimer); if (sAprInitialized) { ll_cleanup_apr(); sAprInitialized = FALSE; } - LLMemory::cleanupClass(); + SUBSYSTEM_CLEANUP(LLMemory); } -- cgit v1.2.3 From 331e932857e1156a68b6d39d3ea2d8c1f39ec7ae Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 22 May 2015 14:02:24 -0400 Subject: MAINT-5232: Clean up some dubious LLSingleton methods. Remove evil getIfExists() method, used by no one. Remove evil destroyed() method, used in exactly three places -- one of which is a test. Replace with equally evil instanceExists() method, which is used EVERYWHERE -- sigh. --- indra/llcommon/llregistry.h | 2 +- indra/llcommon/llsingleton.h | 18 +++--------------- indra/llcommon/tests/llsingleton_test.cpp | 1 - 3 files changed, 4 insertions(+), 17 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llregistry.h b/indra/llcommon/llregistry.h index 29950c108d..fde729f8f9 100755 --- a/indra/llcommon/llregistry.h +++ b/indra/llcommon/llregistry.h @@ -269,7 +269,7 @@ public: ~ScopedRegistrar() { - if (!singleton_t::destroyed()) + if (singleton_t::instanceExists()) { popScope(); } diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 6e6291a165..a4877eed1f 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -166,31 +166,19 @@ public: return NULL; } - static DERIVED_TYPE* getIfExists() - { - return sData.mInstance; - } - // Reference version of getInstance() // Preferred over getInstance() as it disallows checking for NULL static DERIVED_TYPE& instance() { return *getInstance(); } - - // Has this singleton been created uet? - // Use this to avoid accessing singletons before the can safely be constructed + + // Has this singleton been created yet? + // Use this to avoid accessing singletons before they can safely be constructed. static bool instanceExists() { return sData.mInitState == INITIALIZED; } - - // Has this singleton already been deleted? - // Use this to avoid accessing singletons from a static object's destructor - static bool destroyed() - { - return sData.mInitState == DELETED; - } private: diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp index 385289aefe..bed436283a 100755 --- a/indra/llcommon/tests/llsingleton_test.cpp +++ b/indra/llcommon/tests/llsingleton_test.cpp @@ -65,7 +65,6 @@ namespace tut //Delete the instance LLSingletonTest::deleteSingleton(); - ensure(LLSingletonTest::destroyed()); ensure(!LLSingletonTest::instanceExists()); //Construct it again. -- cgit v1.2.3 From df3da846243cce973468b15d1f1752db2009d4ba Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 22 May 2015 22:05:16 -0400 Subject: MAINT-5232: Add LLPounceable template for delayed registrations. LLMuteList, an LLSingleton, overrides its getInstance() method to intercept control every time a consumer wants LLMuteList. This "polling" is to notice when gMessageSystem becomes non-NULL, and register a couple callbacks on it. Unfortunately there are a couple ways to request the LLMuteList instance without specifically calling the subclass getInstance(), which would bypass that logic. Moreover, the polling feels a bit dubious to start with. LLPounceable presents an idiom in which you can callWhenReady(callable) on the LLPounceable instance. If the T* is already non-NULL, it calls the callable immediately; otherwise it enqueues it for when the T* is set non-NULL. (This lets you "pounce" on the T* as soon as it becomes available, hence the name.) So if gMessageSystem were an LLPounceable, LLMuteList's constructor could simply call gMessageSystem.callWhenReady() and relax: the callbacks would be registered either on LLMuteList construction or LLMessageSystem initialization, whichever comes later. LLPounceable comes with its very own set of unit tests. However, as of this commit it is not yet used in actual viewer code. --- indra/llcommon/CMakeLists.txt | 2 + indra/llcommon/llpounceable.h | 217 +++++++++++++++++++++++++++++ indra/llcommon/tests/llpounceable_test.cpp | 200 ++++++++++++++++++++++++++ 3 files changed, 419 insertions(+) create mode 100644 indra/llcommon/llpounceable.h create mode 100644 indra/llcommon/tests/llpounceable_test.cpp (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 1459b9ada2..d2d507d676 100755 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -178,6 +178,7 @@ set(llcommon_HEADER_FILES llmortician.h llnametable.h llpointer.h + llpounceable.h llpredicate.h llpreprocessor.h llpriqueuemap.h @@ -310,6 +311,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llstreamqueue "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") # *TODO - reenable these once tcmalloc libs no longer break the build. #ADD_BUILD_TEST(llallocator llcommon) diff --git a/indra/llcommon/llpounceable.h b/indra/llcommon/llpounceable.h new file mode 100644 index 0000000000..94d508d917 --- /dev/null +++ b/indra/llcommon/llpounceable.h @@ -0,0 +1,217 @@ +/** + * @file llpounceable.h + * @author Nat Goodspeed + * @date 2015-05-22 + * @brief LLPounceable is tangentially related to a future: it's a holder for + * a value that may or may not exist yet. Unlike a future, though, + * LLPounceable freely allows reading the held value. (If the held + * type T does not have a distinguished "empty" value, consider using + * LLPounceable>.) + * + * LLPounceable::callWhenReady() is this template's claim to fame. It + * allows its caller to "pounce" on the held value as soon as it + * becomes non-empty. Call callWhenReady() with any C++ callable + * accepting T. If the held value is already non-empty, callWhenReady() + * will immediately call the callable with the held value. If the held + * value is empty, though, callWhenReady() will enqueue the callable + * for later. As soon as LLPounceable is assigned a non-empty held + * value, it will flush the queue of deferred callables. + * + * Consider a global LLMessageSystem* gMessageSystem. Message system + * initialization happens at a very specific point during viewer + * initialization. Other subsystems want to register callbacks on the + * LLMessageSystem instance as soon as it's initialized, but their own + * initialization may precede that. If we define gMessageSystem to be + * an LLPounceable, a subsystem can use + * callWhenReady() to either register immediately (if gMessageSystem + * is already up and runnning) or register as soon as gMessageSystem + * is set with a new, initialized instance. + * + * $LicenseInfo:firstyear=2015&license=viewerlgpl$ + * Copyright (c) 2015, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLPOUNCEABLE_H) +#define LL_LLPOUNCEABLE_H + +#include "llsingleton.h" +#include +#include +#include +#include +#include +#include + +// Forward declare the user template, since we want to be able to point to it +// in some of its implementation classes. +template +class LLPounceable; + +template +struct LLPounceableTraits +{ + // Call callWhenReady() with any callable accepting T. + typedef boost::function::param_type)> func_t; + // Our actual queue is a simple queue of such callables. + typedef std::queue queue_t; + // owner pointer type + typedef LLPounceable* owner_ptr; +}; + +// Tag types distinguish the two different implementations of LLPounceable's +// queue. +struct LLPounceableQueue {}; +struct LLPounceableStatic {}; + +// generic LLPounceableQueueImpl deliberately omitted: only the above tags are +// legal +template +class LLPounceableQueueImpl; + +// The implementation selected by LLPounceableStatic uses an LLSingleton +// because we can't count on a data member queue being initialized at the time +// we start getting callWhenReady() calls. This is that LLSingleton. +template +class LLPounceableQueueSingleton: + public LLSingleton > +{ +private: + typedef LLPounceableTraits traits; + typedef typename traits::owner_ptr owner_ptr; + typedef typename traits::queue_t queue_t; + + // For a given held type T, every LLPounceable + // instance will call on the SAME LLPounceableQueueSingleton instance -- + // given how class statics work. We must keep a separate queue for each + // LLPounceable instance. Use a hash map for that. + typedef boost::unordered_map map_t; + +public: + // Disambiguate queues belonging to different LLPounceables. + queue_t& get(owner_ptr owner) + { + // operator[] has find-or-create semantics -- just what we want! + return mMap[owner]; + } + +private: + map_t mMap; +}; + +// LLPounceableQueueImpl that uses the above LLSingleton +template +class LLPounceableQueueImpl +{ +public: + typedef LLPounceableTraits traits; + typedef typename traits::owner_ptr owner_ptr; + typedef typename traits::queue_t queue_t; + + queue_t& get(owner_ptr owner) const + { + // this Impl contains nothing; it delegates to the Singleton + return LLPounceableQueueSingleton::instance().get(owner); + } +}; + +// The implementation selected by LLPounceableQueue directly contains the +// queue of interest, suitable for an LLPounceable we can trust to be fully +// initialized when it starts getting callWhenReady() calls. +template +class LLPounceableQueueImpl +{ +public: + typedef LLPounceableTraits traits; + typedef typename traits::owner_ptr owner_ptr; + typedef typename traits::queue_t queue_t; + + queue_t& get(owner_ptr) + { + return mQueue; + } + +private: + queue_t mQueue; +}; + +// LLPounceable is for an LLPounceable instance on the heap or the stack. +// LLPounceable is for a static LLPounceable instance. +template +class LLPounceable +{ +private: + typedef LLPounceableTraits traits; + typedef typename traits::owner_ptr owner_ptr; + typedef typename traits::queue_t queue_t; + +public: + typedef typename traits::func_t func_t; + + // By default, both the initial value and the distinguished empty value + // are a default-constructed T instance. However you can explicitly + // specify each. + LLPounceable(typename boost::call_traits::value_type init =boost::value_initialized(), + typename boost::call_traits::param_type empty=boost::value_initialized()): + mHeld(init), + mEmpty(empty) + {} + + // make read access to mHeld as cheap and transparent as possible + operator T () const { return mHeld; } + typename boost::remove_pointer::type operator*() const { return *mHeld; } + typename boost::call_traits::value_type operator->() const { return mHeld; } + // uncomment 'explicit' as soon as we allow C++11 compilation + /*explicit*/ operator bool() const { return bool(mHeld); } + bool operator!() const { return ! mHeld; } + + // support both assignment (dumb ptr idiom) and reset() (smart ptr) + void operator=(typename boost::call_traits::param_type value) + { + reset(value); + } + + void reset(typename boost::call_traits::param_type value) + { + mHeld = value; + // If this new value is non-empty, flush anything pending in the queue. + if (mHeld != mEmpty) + { + queue_t& queue(get_queue()); + while (! queue.empty()) + { + queue.front()(mHeld); + queue.pop(); + } + } + } + + // our claim to fame + void callWhenReady(const func_t& func) + { + if (mHeld != mEmpty) + { + // If the held value is already non-empty, immediately call func() + func(mHeld); + } + else + { + // held value still empty, queue func() for later + get_queue().push(func); + } + } + +private: + queue_t& get_queue() { return mQueue.get(this); } + + // Store both the current and the empty value. + // MAYBE: Might be useful to delegate to LLPounceableTraits the meaning of + // testing for "empty." For some types we want operator!(); for others we + // want to compare to a distinguished value. + typename boost::call_traits::value_type mHeld, mEmpty; + // This might either contain the queue (LLPounceableQueue) or delegate to + // an LLSingleton (LLPounceableStatic). + LLPounceableQueueImpl mQueue; +}; + +#endif /* ! defined(LL_LLPOUNCEABLE_H) */ diff --git a/indra/llcommon/tests/llpounceable_test.cpp b/indra/llcommon/tests/llpounceable_test.cpp new file mode 100644 index 0000000000..1f8cdca145 --- /dev/null +++ b/indra/llcommon/tests/llpounceable_test.cpp @@ -0,0 +1,200 @@ +/** + * @file llpounceable_test.cpp + * @author Nat Goodspeed + * @date 2015-05-22 + * @brief Test for llpounceable. + * + * $LicenseInfo:firstyear=2015&license=viewerlgpl$ + * Copyright (c) 2015, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llpounceable.h" +// STL headers +// std headers +// external library headers +#include +// other Linden headers +#include "../test/lltut.h" + +struct Data +{ + Data(const std::string& data): + mData(data) + {} + const std::string mData; +}; + +void setter(Data** dest, Data* ptr) +{ + *dest = ptr; +} + +static Data* static_check = 0; + +// Set up an extern pointer to an LLPounceableStatic so the linker will fill +// in the forward reference from below, before runtime. +extern LLPounceable gForward; + +struct EnqueueCall +{ + EnqueueCall() + { + // Intentionally use a forward reference to an LLPounceableStatic that + // we believe is NOT YET CONSTRUCTED. This models the scenario in + // which a constructor in another translation unit runs before + // constructors in this one. We very specifically want callWhenReady() + // to work even in that case: we need the LLPounceableQueueImpl to be + // initialized even if the LLPounceable itself is not. + gForward.callWhenReady(boost::bind(setter, &static_check, _1)); + } +} nqcall; +// When this declaration is processed, we should enqueue the +// setter(&static_check, _1) call for when gForward is set non-NULL. Needless +// to remark, we want this call not to crash. + +// Now declare gForward. Its constructor should not run until after nqcall's. +LLPounceable gForward; + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llpounceable_data + { + }; + typedef test_group llpounceable_group; + typedef llpounceable_group::object object; + llpounceable_group llpounceablegrp("llpounceable"); + + template<> template<> + void object::test<1>() + { + set_test_name("LLPounceableStatic out-of-order test"); + // LLPounceable::callWhenReady() must work even + // before LLPounceable's constructor runs. That's the whole point of + // implementing it with an LLSingleton queue. This models (say) + // LLPounceableStatic. + ensure("static_check should still be null", ! static_check); + Data myData("test<1>"); + gForward = &myData; // should run setter + ensure_equals("static_check should be &myData", static_check, &myData); + } + + template<> template<> + void object::test<2>() + { + set_test_name("LLPounceableQueue different queues"); + // We expect that LLPounceable should have + // different queues because that specialization stores the queue + // directly in the LLPounceable instance. + Data *aptr = 0, *bptr = 0; + LLPounceable a, b; + a.callWhenReady(boost::bind(setter, &aptr, _1)); + b.callWhenReady(boost::bind(setter, &bptr, _1)); + ensure("aptr should be null", ! aptr); + ensure("bptr should be null", ! bptr); + Data adata("a"), bdata("b"); + a = &adata; + ensure_equals("aptr should be &adata", aptr, &adata); + // but we haven't yet set b + ensure("bptr should still be null", !bptr); + b = &bdata; + ensure_equals("bptr should be &bdata", bptr, &bdata); + } + + template<> template<> + void object::test<3>() + { + set_test_name("LLPounceableStatic different queues"); + // LLPounceable should also have a distinct + // queue for each instance, but that engages an additional map lookup + // because there's only one LLSingleton for each T. + Data *aptr = 0, *bptr = 0; + LLPounceable a, b; + a.callWhenReady(boost::bind(setter, &aptr, _1)); + b.callWhenReady(boost::bind(setter, &bptr, _1)); + ensure("aptr should be null", ! aptr); + ensure("bptr should be null", ! bptr); + Data adata("a"), bdata("b"); + a = &adata; + ensure_equals("aptr should be &adata", aptr, &adata); + // but we haven't yet set b + ensure("bptr should still be null", !bptr); + b = &bdata; + ensure_equals("bptr should be &bdata", bptr, &bdata); + } + + template<> template<> + void object::test<4>() + { + set_test_name("LLPounceable looks like T"); + // We want LLPounceable to be drop-in replaceable for a plain + // T for read constructs. In particular, it should behave like a dumb + // pointer -- and with zero abstraction cost for such usage. + Data* aptr = 0; + Data a("a"); + // should be able to initialize a pounceable (when its constructor + // runs) + LLPounceable pounceable = &a; + // should be able to pass LLPounceable to function accepting T + setter(&aptr, pounceable); + ensure_equals("aptr should be &a", aptr, &a); + // should be able to dereference with * + ensure_equals("deref with *", (*pounceable).mData, "a"); + // should be able to dereference with -> + ensure_equals("deref with ->", pounceable->mData, "a"); + // bool operations + ensure("test with operator bool()", pounceable); + ensure("test with operator !()", ! (! pounceable)); + } + + template<> template<> + void object::test<5>() + { + set_test_name("Multiple callWhenReady() queue items"); + Data *p1 = 0, *p2 = 0, *p3 = 0; + Data a("a"); + LLPounceable pounceable; + // queue up a couple setter() calls for later + pounceable.callWhenReady(boost::bind(setter, &p1, _1)); + pounceable.callWhenReady(boost::bind(setter, &p2, _1)); + // should still be pending + ensure("p1 should be null", !p1); + ensure("p2 should be null", !p2); + ensure("p3 should be null", !p3); + pounceable = 0; + // assigning a new empty value shouldn't flush the queue + ensure("p1 should still be null", !p1); + ensure("p2 should still be null", !p2); + ensure("p3 should still be null", !p3); + // using whichever syntax + pounceable.reset(0); + // try to make ensure messages distinct... tough to pin down which + // ensure() failed if multiple ensure() calls in the same test have + // the same message! + ensure("p1 should again be null", !p1); + ensure("p2 should again be null", !p2); + ensure("p3 should again be null", !p3); + pounceable.reset(&a); // should flush queue + ensure_equals("p1 should be &a", p1, &a); + ensure_equals("p2 should be &a", p2, &a); + ensure("p3 still not set", !p3); + // immediate call + pounceable.callWhenReady(boost::bind(setter, &p3, _1)); + ensure_equals("p3 should be &a", p3, &a); + } + + template<> template<> + void object::test<6>() + { + set_test_name("compile-fail test, uncomment to check"); + // The following declaration should fail: only LLPounceableQueue and + // LLPounceableStatic should work as tags. +// LLPounceable pounceable; + } +} // namespace tut -- cgit v1.2.3 From 9f962c03bf9080d67a8ea10aa53289c841fea781 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 27 May 2015 16:41:10 -0400 Subject: MAINT-5232: Extract LLInitClass, LLDestroyClass from llui/llui.h to a new llcommon/llinitdestroyclass.h. This mechanism is so general -- but has so many related moving parts -- that (a) it deserves to be in a header file all its own, instead of conflated with llui.h, and (b) it should be in llcommon where anyone can use it. It has no dependencies whatsoever on llui or anything viewer-specific. In this very changeset we changed one #include "llui.h" whose comment admits that it was only dragged in for LLDestroyClass. --- indra/llcommon/CMakeLists.txt | 1 + indra/llcommon/llinitdestroyclass.h | 134 ++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 indra/llcommon/llinitdestroyclass.h (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index d2d507d676..de5aa0fde4 100755 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -162,6 +162,7 @@ set(llcommon_HEADER_FILES llhash.h llheartbeat.h llindexedvector.h + llinitdestroyclass.h llinitparam.h llinstancetracker.h llkeythrottle.h diff --git a/indra/llcommon/llinitdestroyclass.h b/indra/llcommon/llinitdestroyclass.h new file mode 100644 index 0000000000..ca5c3f07de --- /dev/null +++ b/indra/llcommon/llinitdestroyclass.h @@ -0,0 +1,134 @@ +/** + * @file llinitdestroyclass.h + * @author Nat Goodspeed + * @date 2015-05-27 + * @brief LLInitClass / LLDestroyClass mechanism + * + * The LLInitClass template, extracted from llui.h, ensures that control will + * reach a static initClass() method. LLDestroyClass does the same for a + * static destroyClass() method. + * + * The distinguishing characteristics of these templates are: + * + * - All LLInitClass::initClass() methods are triggered by an explicit call + * to LLInitClassList::instance().fireCallbacks(). Presumably this call + * happens sometime after all static objects in the program have been + * initialized. In other words, each LLInitClass::initClass() method + * should be able to make some assumptions about global program state. + * + * - Similarly, LLDestroyClass::destroyClass() methods are triggered by + * LLDestroyClassList::instance().fireCallbacks(). Again, presumably this + * happens at a well-defined moment in the program's shutdown sequence. + * + * - The initClass() calls happen in an unspecified sequence. You may not rely + * on the relative ordering of LLInitClass::initClass() versus another + * LLInitClass::initClass() method. If you need such a guarantee, use + * LLSingleton instead and make the dependency explicit. + * + * - Similarly, LLDestroyClass::destroyClass() may happen either before or + * after LLDestroyClass::destroyClass(). You cannot rely on that order. + * + * $LicenseInfo:firstyear=2015&license=viewerlgpl$ + * Copyright (c) 2015, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLINITDESTROYCLASS_H) +#define LL_LLINITDESTROYCLASS_H + +#include "llerror.h" +#include "llsingleton.h" +#include +#include +#include + +class LLCallbackRegistry +{ +public: + typedef boost::signals2::signal callback_signal_t; + + void registerCallback(const callback_signal_t::slot_type& slot) + { + mCallbacks.connect(slot); + } + + void fireCallbacks() + { + mCallbacks(); + } + +private: + callback_signal_t mCallbacks; +}; + +class LLInitClassList : + public LLCallbackRegistry, + public LLSingleton +{ + friend class LLSingleton; +private: + LLInitClassList() {} +}; + +class LLDestroyClassList : + public LLCallbackRegistry, + public LLSingleton +{ + friend class LLSingleton; +private: + LLDestroyClassList() {} +}; + +template +class LLRegisterWith +{ +public: + LLRegisterWith(boost::function func) + { + T::instance().registerCallback(func); + } + + // this avoids a MSVC bug where non-referenced static members are "optimized" away + // even if their constructors have side effects + S32 reference() + { + S32 dummy; + dummy = 0; + return dummy; + } +}; + +template +class LLInitClass +{ +public: + LLInitClass() { sRegister.reference(); } + + static LLRegisterWith sRegister; +private: + + static void initClass() + { + LL_ERRS() << "No static initClass() method defined for " << typeid(T).name() << LL_ENDL; + } +}; + +template +class LLDestroyClass +{ +public: + LLDestroyClass() { sRegister.reference(); } + + static LLRegisterWith sRegister; +private: + + static void destroyClass() + { + LL_ERRS() << "No static destroyClass() method defined for " << typeid(T).name() << LL_ENDL; + } +}; + +template LLRegisterWith LLInitClass::sRegister(&T::initClass); +template LLRegisterWith LLDestroyClass::sRegister(&T::destroyClass); + +#endif /* ! defined(LL_LLINITDESTROYCLASS_H) */ -- cgit v1.2.3 From d15ec90bc958a810c7129a66b4924b91267f4fa1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 28 May 2015 11:57:46 -0400 Subject: MAINT-5232: Provide better commentation for llinitdestroyclass.h. --- indra/llcommon/llinitdestroyclass.h | 56 +++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llinitdestroyclass.h b/indra/llcommon/llinitdestroyclass.h index ca5c3f07de..49bcefc33d 100644 --- a/indra/llcommon/llinitdestroyclass.h +++ b/indra/llcommon/llinitdestroyclass.h @@ -42,6 +42,11 @@ #include #include +/** + * LLCallbackRegistry is an implementation detail base class for + * LLInitClassList and LLDestroyClassList. It's a very thin wrapper around a + * Boost.Signals2 signal object. + */ class LLCallbackRegistry { public: @@ -61,6 +66,13 @@ private: callback_signal_t mCallbacks; }; +/** + * LLInitClassList is the LLCallbackRegistry for LLInitClass. It stores the + * registered initClass() methods. It must be an LLSingleton because + * LLInitClass registers its initClass() method at static construction time + * (before main()), requiring LLInitClassList to be fully constructed on + * demand regardless of module initialization order. + */ class LLInitClassList : public LLCallbackRegistry, public LLSingleton @@ -70,6 +82,13 @@ private: LLInitClassList() {} }; +/** + * LLDestroyClassList is the LLCallbackRegistry for LLDestroyClass. It stores + * the registered destroyClass() methods. It must be an LLSingleton because + * LLDestroyClass registers its destroyClass() method at static construction + * time (before main()), requiring LLDestroyClassList to be fully constructed + * on demand regardless of module initialization order. + */ class LLDestroyClassList : public LLCallbackRegistry, public LLSingleton @@ -79,6 +98,12 @@ private: LLDestroyClassList() {} }; +/** + * LLRegisterWith is an implementation detail for LLInitClass and + * LLDestroyClass. It is intended to be used as a static class member whose + * constructor registers the specified callback with the LLMumbleClassList + * singleton registry specified as the template argument. + */ template class LLRegisterWith { @@ -98,37 +123,68 @@ public: } }; +/** + * Derive MyClass from LLInitClass (the Curiously Recurring Template + * Pattern) to ensure that the static method MyClass::initClass() will be + * called (along with all other LLInitClass subclass initClass() methods) + * when someone calls LLInitClassList::instance().fireCallbacks(). This gives + * the application specific control over the timing of all such + * initializations, without having to insert calls for every such class into + * generic application code. + */ template class LLInitClass { public: LLInitClass() { sRegister.reference(); } + // When this static member is initialized, the subclass initClass() method + // is registered on LLInitClassList. See sRegister definition below. static LLRegisterWith sRegister; private: + // Provide a default initClass() method in case subclass misspells (or + // omits) initClass(). This turns a potential build error into a fatal + // runtime error. static void initClass() { LL_ERRS() << "No static initClass() method defined for " << typeid(T).name() << LL_ENDL; } }; +/** + * Derive MyClass from LLDestroyClass (the Curiously Recurring + * Template Pattern) to ensure that the static method MyClass::destroyClass() + * will be called (along with other LLDestroyClass subclass destroyClass() + * methods) when someone calls LLDestroyClassList::instance().fireCallbacks(). + * This gives the application specific control over the timing of all such + * cleanup calls, without having to insert calls for every such class into + * generic application code. + */ template class LLDestroyClass { public: LLDestroyClass() { sRegister.reference(); } + // When this static member is initialized, the subclass destroyClass() + // method is registered on LLInitClassList. See sRegister definition + // below. static LLRegisterWith sRegister; private: + // Provide a default destroyClass() method in case subclass misspells (or + // omits) destroyClass(). This turns a potential build error into a fatal + // runtime error. static void destroyClass() { LL_ERRS() << "No static destroyClass() method defined for " << typeid(T).name() << LL_ENDL; } }; +// Here's where LLInitClass specifies the subclass initClass() method. template LLRegisterWith LLInitClass::sRegister(&T::initClass); +// Here's where LLDestroyClass specifies the subclass destroyClass() method. template LLRegisterWith LLDestroyClass::sRegister(&T::destroyClass); #endif /* ! defined(LL_LLINITDESTROYCLASS_H) */ -- cgit v1.2.3 From df8da8c013dfa7fc1c51b483208001cdd97269ba Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 28 May 2015 17:03:30 -0400 Subject: MAINT-5232: Stop documenting deprecated alternative LLSingleton usage. --- indra/llcommon/llsingleton.h | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index a4877eed1f..5d2a26cae5 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -30,30 +30,20 @@ #include #include -// LLSingleton implements the getInstance() method part of the Singleton -// pattern. It can't make the derived class constructors protected, though, so -// you have to do that yourself. -// -// There are two ways to use LLSingleton. The first way is to inherit from it -// while using the typename that you'd like to be static as the template -// parameter, like so: -// -// class Foo: public LLSingleton{}; -// -// Foo& instance = Foo::instance(); -// -// The second way is to use the singleton class directly, without inheritance: -// -// typedef LLSingleton FooSingleton; -// -// Foo& instance = FooSingleton::instance(); -// -// In this case, the class being managed as a singleton needs to provide an -// initSingleton() method since the LLSingleton virtual method won't be -// available -// -// As currently written, it is not thread-safe. - +/** + * LLSingleton implements the getInstance() method part of the Singleton + * pattern. It can't make the derived class constructors protected, though, so + * you have to do that yourself. + * + * Derive your class from LLSingleton, passing your subclass name as + * LLSingleton's template parameter, like so: + * + * class Foo: public LLSingleton{}; + * + * Foo& instance = Foo::instance(); + * + * As currently written, LLSingleton is not thread-safe. + */ template class LLSingleton : private boost::noncopyable { -- cgit v1.2.3 From a9650ba22219d91438d91fd9371966f510884cbf Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 29 May 2015 15:10:54 -0400 Subject: MAINT-5232: Per Vir review, use Boost.Signals2 for LLPounceable. Vir points out that "queue of callables" is pretty much exactly what a signal does. Add unit tests to verify chronological order, also queue reset when fired. --- indra/llcommon/llpounceable.h | 43 ++++++++++++++---------------- indra/llcommon/tests/llpounceable_test.cpp | 30 +++++++++++++++++++++ 2 files changed, 50 insertions(+), 23 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llpounceable.h b/indra/llcommon/llpounceable.h index 94d508d917..9c8aba2154 100644 --- a/indra/llcommon/llpounceable.h +++ b/indra/llcommon/llpounceable.h @@ -40,8 +40,7 @@ #include #include #include -#include -#include +#include // Forward declare the user template, since we want to be able to point to it // in some of its implementation classes. @@ -51,10 +50,10 @@ class LLPounceable; template struct LLPounceableTraits { + // Our "queue" is a signal object with correct signature. + typedef boost::signals2::signal::param_type)> signal_t; // Call callWhenReady() with any callable accepting T. - typedef boost::function::param_type)> func_t; - // Our actual queue is a simple queue of such callables. - typedef std::queue queue_t; + typedef typename signal_t::slot_type func_t; // owner pointer type typedef LLPounceable* owner_ptr; }; @@ -79,17 +78,17 @@ class LLPounceableQueueSingleton: private: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; // For a given held type T, every LLPounceable // instance will call on the SAME LLPounceableQueueSingleton instance -- // given how class statics work. We must keep a separate queue for each // LLPounceable instance. Use a hash map for that. - typedef boost::unordered_map map_t; + typedef boost::unordered_map map_t; public: // Disambiguate queues belonging to different LLPounceables. - queue_t& get(owner_ptr owner) + signal_t& get(owner_ptr owner) { // operator[] has find-or-create semantics -- just what we want! return mMap[owner]; @@ -106,9 +105,9 @@ class LLPounceableQueueImpl public: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; - queue_t& get(owner_ptr owner) const + signal_t& get(owner_ptr owner) const { // this Impl contains nothing; it delegates to the Singleton return LLPounceableQueueSingleton::instance().get(owner); @@ -124,15 +123,15 @@ class LLPounceableQueueImpl public: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; - queue_t& get(owner_ptr) + signal_t& get(owner_ptr) { return mQueue; } private: - queue_t mQueue; + signal_t mQueue; }; // LLPounceable is for an LLPounceable instance on the heap or the stack. @@ -143,7 +142,7 @@ class LLPounceable private: typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; - typedef typename traits::queue_t queue_t; + typedef typename traits::signal_t signal_t; public: typedef typename traits::func_t func_t; @@ -177,12 +176,9 @@ public: // If this new value is non-empty, flush anything pending in the queue. if (mHeld != mEmpty) { - queue_t& queue(get_queue()); - while (! queue.empty()) - { - queue.front()(mHeld); - queue.pop(); - } + signal_t& signal(get_signal()); + signal(mHeld); + signal.disconnect_all_slots(); } } @@ -196,13 +192,14 @@ public: } else { - // held value still empty, queue func() for later - get_queue().push(func); + // Held value still empty, queue func() for later. By default, + // connect() enqueues slots in FIFO order. + get_signal().connect(func); } } private: - queue_t& get_queue() { return mQueue.get(this); } + signal_t& get_signal() { return mQueue.get(this); } // Store both the current and the empty value. // MAYBE: Might be useful to delegate to LLPounceableTraits the meaning of diff --git a/indra/llcommon/tests/llpounceable_test.cpp b/indra/llcommon/tests/llpounceable_test.cpp index 1f8cdca145..25458865a6 100644 --- a/indra/llcommon/tests/llpounceable_test.cpp +++ b/indra/llcommon/tests/llpounceable_test.cpp @@ -20,6 +20,13 @@ // other Linden headers #include "../test/lltut.h" +/*----------------------------- string testing -----------------------------*/ +void append(std::string* dest, const std::string& src) +{ + dest->append(src); +} + +/*-------------------------- Data-struct testing ---------------------------*/ struct Data { Data(const std::string& data): @@ -191,6 +198,29 @@ namespace tut template<> template<> void object::test<6>() + { + set_test_name("queue order"); + std::string data; + LLPounceable pounceable; + pounceable.callWhenReady(boost::bind(append, _1, "a")); + pounceable.callWhenReady(boost::bind(append, _1, "b")); + pounceable.callWhenReady(boost::bind(append, _1, "c")); + pounceable = &data; + ensure_equals("callWhenReady() must preserve chronological order", + data, "abc"); + + std::string data2; + pounceable = NULL; + pounceable.callWhenReady(boost::bind(append, _1, "d")); + pounceable.callWhenReady(boost::bind(append, _1, "e")); + pounceable.callWhenReady(boost::bind(append, _1, "f")); + pounceable = &data2; + ensure_equals("LLPounceable must reset queue when fired", + data2, "def"); + } + + template<> template<> + void object::test<7>() { set_test_name("compile-fail test, uncomment to check"); // The following declaration should fail: only LLPounceableQueue and -- cgit v1.2.3 From ccc55255c57f8449aa46cd8847dbda1ced1e851f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 29 May 2015 16:37:15 -0400 Subject: MAINT-5232: Make LLPounceable noncopyable. Changing the queue-of-callables implementation to boost::signals2::signal, which is noncopyable, means that LLPounceable itself should be noncopyable. --- indra/llcommon/llpounceable.h | 3 ++- indra/llcommon/tests/llpounceable_test.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llpounceable.h b/indra/llcommon/llpounceable.h index 9c8aba2154..77b711bdc6 100644 --- a/indra/llcommon/llpounceable.h +++ b/indra/llcommon/llpounceable.h @@ -36,6 +36,7 @@ #define LL_LLPOUNCEABLE_H #include "llsingleton.h" +#include #include #include #include @@ -137,7 +138,7 @@ private: // LLPounceable is for an LLPounceable instance on the heap or the stack. // LLPounceable is for a static LLPounceable instance. template -class LLPounceable +class LLPounceable: public boost::noncopyable { private: typedef LLPounceableTraits traits; diff --git a/indra/llcommon/tests/llpounceable_test.cpp b/indra/llcommon/tests/llpounceable_test.cpp index 25458865a6..2f4915ce11 100644 --- a/indra/llcommon/tests/llpounceable_test.cpp +++ b/indra/llcommon/tests/llpounceable_test.cpp @@ -147,7 +147,7 @@ namespace tut Data a("a"); // should be able to initialize a pounceable (when its constructor // runs) - LLPounceable pounceable = &a; + LLPounceable pounceable(&a); // should be able to pass LLPounceable to function accepting T setter(&aptr, pounceable); ensure_equals("aptr should be &a", aptr, &a); -- cgit v1.2.3 From d792baf9f7220a374788b35789335a7ae2e62369 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 24 Jun 2015 21:14:55 -0400 Subject: MAINT-5232: Introduce inter-LLSingleton dependency tracking. Introduce LLSingleton::cleanupSingleton() canonical method as the place to put any subclass cleanup logic that might take nontrivial realtime or throw an exception. Neither is appropriate in a destructor. Track all extant LLSingleton subclass instances on a master list, which permits adding LLSingletonBase::cleanupAll() and deleteAll() methods. Also notice when any LLSingleton subclass constructor (or initSingleton() method) calls instance() or getInstance() for another LLSingleton, and capture that other LLSingleton instance as a dependency of the first. This permits cleanupAll() and deleteAll() to perform a dependency sort on the master list, thus cleaning up (or deleting) leaf LLSingletons AFTER the LLSingletons that depend on them. Make C++ runtime's final static destructor call LLSingletonBase::deleteAll() instead of deleting individual LLSingleton instances in arbitrary order. Eliminate "llerror.h" from llsingleton.h, a longstanding TODO. --- indra/llcommon/llsingleton.cpp | 290 ++++++++++++++++++++++++- indra/llcommon/llsingleton.h | 480 ++++++++++++++++++++++++++++++----------- 2 files changed, 639 insertions(+), 131 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 9b49e52377..204c0d24d0 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -25,7 +25,295 @@ */ #include "linden_common.h" - #include "llsingleton.h" +#include "llerror.h" +#include "lldependencies.h" +#include +#include +#include +#include + +// Our master list of all LLSingletons is itself an LLSingleton. We used to +// store it in a function-local static, but that could get destroyed before +// the last of the LLSingletons -- and ~LLSingletonBase() definitely wants to +// remove itself from the master list. Since the whole point of this master +// list is to help track inter-LLSingleton dependencies, and since we have +// this implicit dependency from every LLSingleton to the master list, make it +// an LLSingleton. +class LLSingletonBase::MasterList: + public LLSingleton +{ +private: + friend class LLSingleton; + +public: + // No need to make this private with accessors; nobody outside this source + // file can see it. + LLSingletonBase::list_t mList; +}; + +//static +LLSingletonBase::list_t& LLSingletonBase::get_master() +{ + return LLSingletonBase::MasterList::instance().mList; +} + +void LLSingletonBase::add_master() +{ + // As each new LLSingleton is constructed, add to the master list. + get_master().push_back(this); +} + +void LLSingletonBase::remove_master() +{ + // When an LLSingleton is destroyed, remove from master list. + // add_master() used to capture the iterator to the newly-added list item + // so we could directly erase() it from the master list. Unfortunately + // that runs afoul of destruction-dependency order problems. So search the + // 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); +} + +// Wrapping our initializing list in a static method ensures that it will be +// constructed on demand. This list doesn't also need to be in an LLSingleton +// because (a) it should be empty by program shutdown and (b) none of our +// destructors reference it. +//static +LLSingletonBase::list_t& LLSingletonBase::get_initializing() +{ + static list_t sList; + return sList; +} + +LLSingletonBase::LLSingletonBase(): + mCleaned(false), + mDeleteSingleton(NULL) +{ + // Make this the currently-initializing LLSingleton. + push_initializing(); +} + +LLSingletonBase::~LLSingletonBase() {} + +void LLSingletonBase::push_initializing() +{ + get_initializing().push_back(this); +} + +void LLSingletonBase::pop_initializing() +{ + list_t& list(get_initializing()); + if (list.empty()) + { + LL_ERRS() << "Underflow in stack of currently-initializing LLSingletons at " + << typeid(*this).name() << "::getInstance()" << LL_ENDL; + } + if (list.back() != this) + { + LL_ERRS() << "Push/pop mismatch in stack of currently-initializing LLSingletons: " + << typeid(*this).name() << "::getInstance() trying to pop " + << typeid(*list.back()).name() << LL_ENDL; + } + // Here we're sure that list.back() == this. Whew, pop it. + list.pop_back(); +} + +void LLSingletonBase::capture_dependency() +{ + // 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 + // empty(). + list_t& initializing(get_initializing()); + if (! initializing.empty()) + { + // getInstance() is being called by some other LLSingleton. But -- is + // this a circularity? That is, does 'this' already appear in the + // initializing stack? + // For what it's worth, normally 'initializing' should contain very + // few elements. + list_t::const_iterator found = + std::find(initializing.begin(), initializing.end(), this); + if (found != initializing.end()) + { + // Report the circularity. Requiring the coder to dig through the + // logic to diagnose exactly how we got here is less than helpful. + std::ostringstream out; + for ( ; found != initializing.end(); ++found) + { + // 'found' is an iterator; *found is an LLSingletonBase*; **found + // is the actual LLSingletonBase instance. + out << typeid(**found).name() << " -> "; + } + // DEBUGGING: Initially, make this crump. We want to know how bad + // the problem is before we add it to the long, sad list of + // ominous warnings that everyone always ignores. + LL_ERRS() << "LLSingleton circularity: " << out.str() + << typeid(*this).name() << LL_ENDL; + } + else + { + // Here 'this' is NOT already in the 'initializing' stack. Great! + // Record the dependency. + // initializing.back() is the LLSingletonBase* currently being + // initialized. Store 'this' in its mDepends set. + initializing.back()->mDepends.insert(this); + } + } +} + +//static +LLSingletonBase::vec_t LLSingletonBase::dep_sort() +{ + // While it would theoretically be possible to maintain a static + // 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(). + typedef LLDependencies SingletonDeps; + SingletonDeps sdeps; + list_t& master(get_master()); + BOOST_FOREACH(LLSingletonBase* sp, master) + { + // Build the SingletonDeps structure by adding, for each + // LLSingletonBase* sp in the master list, sp itself. It has no + // associated value type in our SingletonDeps, hence the 0. We don't + // record the LLSingletons it must follow; rather, we record the ones + // it must precede. Copy its mDepends to a KeyList to express that. + sdeps.add(sp, 0, + SingletonDeps::KeyList(), + SingletonDeps::KeyList(sp->mDepends.begin(), sp->mDepends.end())); + } + vec_t ret; + ret.reserve(master.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 + // straightforward, as long as we remember the above reserve() call! + BOOST_FOREACH(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()); + 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; + + try + { + sp->cleanupSingleton(); + } + catch (const std::exception& e) + { + LL_WARNS() << "Exception in " << typeid(*sp).name() + << "::cleanupSingleton(): " << e.what() << LL_ENDL; + } + catch (...) + { + LL_WARNS() << "Unknown exception in " << typeid(*sp).name() + << "::cleanupSingleton()" << LL_ENDL; + } + } + } +} + +//static +void LLSingletonBase::deleteAll() +{ + // It's essential to traverse these in dependency order. + BOOST_FOREACH(LLSingletonBase* sp, dep_sort()) + { + // Capture the class name first: in case of exception, don't count on + // being able to extract it later. + const char* name = typeid(*sp).name(); + try + { + // Call static method through instance function pointer. + if (! sp->mDeleteSingleton) + { + // This Should Not Happen... but carry on. + LL_WARNS() << name << "::mDeleteSingleton not initialized!" << LL_ENDL; + } + else + { + // properly initialized: call it. + // From this point on, DO NOT DEREFERENCE sp! + sp->mDeleteSingleton(); + } + } + catch (const std::exception& e) + { + LL_WARNS() << "Exception in " << name + << "::deleteSingleton(): " << e.what() << LL_ENDL; + } + catch (...) + { + LL_WARNS() << "Unknown exception in " << name + << "::deleteSingleton()" << LL_ENDL; + } + } +} + +/*------------------------ Final cleanup management ------------------------*/ +class LLSingletonBase::MasterRefcount +{ +public: + // store a POD int so it will be statically initialized to 0 + int refcount; +}; +static LLSingletonBase::MasterRefcount sMasterRefcount; + +LLSingletonBase::ref_ptr_t LLSingletonBase::get_master_refcount() +{ + // Calling this method constructs a new ref_ptr_t, which implicitly calls + // intrusive_ptr_add_ref(MasterRefcount*). + return &sMasterRefcount; +} + +void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount* mrc) +{ + // Count outstanding SingletonLifetimeManager instances. + ++mrc->refcount; +} + +void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) +{ + // Notice when each SingletonLifetimeManager instance is destroyed. + if (! --mrc->refcount) + { + // The last instance was destroyed. Time to kill any remaining + // LLSingletons -- but in dependency order. + LLSingletonBase::deleteAll(); + } +} + +/*---------------------------- Logging helpers -----------------------------*/ +//static +void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3) +{ + LL_ERRS() << p1 << p2 << p3 << LL_ENDL; +} +//static +void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3) +{ + LL_WARNS() << p1 << p2 << p3 << LL_ENDL; +} diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 5d2a26cae5..7706ed53f2 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -25,10 +25,151 @@ #ifndef LLSINGLETON_H #define LLSINGLETON_H -#include "llerror.h" // *TODO: eliminate this - -#include #include +#include +#include +#include +#include +#include + +// TODO: +// Tests for all this! +class LLSingletonBase: private boost::noncopyable +{ +public: + class MasterList; + class MasterRefcount; + typedef boost::intrusive_ptr ref_ptr_t; + +private: + // All existing LLSingleton instances are tracked in this master list. + typedef std::list list_t; + static list_t& get_master(); + // This, on the other hand, is a stack whose top indicates the LLSingleton + // currently being initialized. + static list_t& get_initializing(); + // Produce a vector of master list, in dependency order. + 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; + +protected: + // Base-class constructor should only be invoked by the DERIVED_TYPE + // constructor. + LLSingletonBase(); + virtual ~LLSingletonBase(); + + // Every new LLSingleton should be added to/removed from the master list + void add_master(); + void remove_master(); + // with a little help from our friends. + template friend class LLSingleton_manage_master; + + // Maintain a stack of the LLSingleton subclass instance currently being + // initialized. We use this to notice direct dependencies: we want to know + // if A requires B. We deduce that if while initializing A, control + // reaches B::getInstance(). + // We want &A to be at the top of that stack during both A::A() and + // A::initSingleton(), since a call to B::getInstance() might occur during + // either. + // Unfortunately the desired timespan does not correspond neatly with a + // single C++ scope, else we'd use RAII to track it. But we do know that + // LLSingletonBase's constructor definitely runs just before + // LLSingleton's, which runs just before the specific subclass's. + void push_initializing(); + // LLSingleton is, and must remain, the only caller to initSingleton(). + // That being the case, we control exactly when it happens -- and we can + // pop the stack immediately thereafter. + void pop_initializing(); + // 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(); + + // delegate LL_ERRS() logging to llsingleton.cpp + static void logerrs(const char* p1, const char* p2="", const char* p3=""); + // delegate LL_WARNS() logging to llsingleton.cpp + static void logwarns(const char* p1, const char* p2="", const char* p3=""); + + // obtain canonical ref_ptr_t + static ref_ptr_t get_master_refcount(); + + // Default methods in case subclass doesn't declare them. + virtual void initSingleton() {} + virtual void cleanupSingleton() {} + + // 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 + // deleteSingleton()) store a pointer here. Since we know it's a static + // class method, a classic-C function pointer will do. + void (*mDeleteSingleton)(); + +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. + * + * The most important property of deleteAll() is that deleteSingleton() + * 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 deleteSingleton() method throws an exception, the exception is + * logged, but deleteAll() attempts to continue calling the rest of the + * deleteSingleton() methods. + */ + static void deleteAll(); +}; + +// support ref_ptr_t +void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount*); +void intrusive_ptr_release(LLSingletonBase::MasterRefcount*); + +// Most of the time, we want LLSingleton_manage_master() to forward its +// methods to LLSingletonBase::add_master() and remove_master(). +template +struct LLSingleton_manage_master +{ + void add(LLSingletonBase* sb) { sb->add_master(); } + void remove(LLSingletonBase* sb) { sb->remove_master(); } +}; + +// But for the specific case of LLSingletonBase::MasterList, don't. +template <> +struct LLSingleton_manage_master +{ + void add(LLSingletonBase*) {} + void remove(LLSingletonBase*) {} +}; /** * LLSingleton implements the getInstance() method part of the Singleton @@ -42,146 +183,225 @@ * * Foo& instance = Foo::instance(); * + * LLSingleton recognizes a couple special methods in your derived class. + * + * If you override LLSingleton::initSingleton(), your method will be called + * immediately after the instance is constructed. This is useful for breaking + * circular dependencies: if you find that your LLSingleton subclass + * constructor references other LLSingleton subclass instances in a chain + * 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. + * + * 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 + * B::instance() or B::getInstance(), and C::instance() or C::getInstance(). + * 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(). + * 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.) + * * As currently written, LLSingleton is not thread-safe. */ template -class LLSingleton : private boost::noncopyable +class LLSingleton : public LLSingletonBase { - private: - typedef enum e_init_state - { - UNINITIALIZED, - CONSTRUCTING, - INITIALIZING, - INITIALIZED, - DELETED - } EInitState; - + typedef enum e_init_state + { + UNINITIALIZED = 0, // must be default-initialized state + CONSTRUCTING, + INITIALIZING, + INITIALIZED, + DELETED + } EInitState; + static DERIVED_TYPE* constructSingleton() { return new DERIVED_TYPE(); } - - // stores pointer to singleton instance - struct SingletonLifetimeManager - { - SingletonLifetimeManager() - { - construct(); - } - - static void construct() - { - sData.mInitState = CONSTRUCTING; - sData.mInstance = constructSingleton(); - sData.mInitState = INITIALIZING; - } - - ~SingletonLifetimeManager() - { - if (sData.mInitState != DELETED) - { - deleteSingleton(); - } - } - }; - + + // stores pointer to singleton instance + struct SingletonLifetimeManager + { + SingletonLifetimeManager(): + mMasterRefcount(LLSingletonBase::get_master_refcount()) + { + construct(); + } + + static void construct() + { + sData.mInitState = CONSTRUCTING; + sData.mInstance = constructSingleton(); + sData.mInitState = INITIALIZING; + } + + ~SingletonLifetimeManager() + { + // The dependencies between LLSingletons, and the arbitrary order + // of static-object destruction, mean that we DO NOT WANT this + // destructor to delete this LLSingleton. This destructor will run + // without regard to any other LLSingleton whose cleanup might + // depend on its existence. What we really want is to count the + // runtime's attempts to cleanup LLSingleton static data -- and on + // the very last one, call LLSingletonBase::deleteAll(). That + // method will properly honor cross-LLSingleton dependencies. This + // is why we store an intrusive_ptr to a MasterRefcount: our + // ref_ptr_t member counts SingletonLifetimeManager instances. + // Once the runtime destroys the last of these, THEN we can delete + // every remaining LLSingleton. + } + + LLSingletonBase::ref_ptr_t mMasterRefcount; + }; + +protected: + LLSingleton() + { + // populate base-class function pointer with the static + // deleteSingleton() function for this particular specialization + mDeleteSingleton = &deleteSingleton; + + // add this new instance to the master list + LLSingleton_manage_master().add(this); + } + public: - virtual ~LLSingleton() - { - sData.mInstance = NULL; - sData.mInitState = DELETED; - } - - /** - * @brief Immediately delete the singleton. - * - * A subsequent call to LLProxy::getInstance() will construct a new - * instance of the class. - * - * LLSingletons are normally 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. - * - * 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. - */ - static void deleteSingleton() - { - delete sData.mInstance; - sData.mInstance = NULL; - sData.mInitState = DELETED; - } - - - static DERIVED_TYPE* getInstance() - { - static SingletonLifetimeManager sLifeTimeMgr; - - switch (sData.mInitState) - { - case UNINITIALIZED: - // should never be uninitialized at this point - llassert(false); - return NULL; - case CONSTRUCTING: - LL_ERRS() << "Tried to access singleton " << typeid(DERIVED_TYPE).name() << " from singleton constructor!" << LL_ENDL; - return NULL; - case INITIALIZING: - // go ahead and flag ourselves as initialized so we can be reentrant during initialization - sData.mInitState = INITIALIZED; - // 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(); - return sData.mInstance; - case INITIALIZED: - return sData.mInstance; - case DELETED: - LL_WARNS() << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << LL_ENDL; - SingletonLifetimeManager::construct(); - // same as first time construction - sData.mInitState = INITIALIZED; - sData.mInstance->initSingleton(); - return sData.mInstance; - } - - return NULL; - } - - // Reference version of getInstance() - // Preferred over getInstance() as it disallows checking for NULL - static DERIVED_TYPE& instance() - { - return *getInstance(); - } - - // Has this singleton been created yet? - // Use this to avoid accessing singletons before they can safely be constructed. - static bool instanceExists() - { - return sData.mInitState == INITIALIZED; - } + virtual ~LLSingleton() + { + // remove this instance from the master list + LLSingleton_manage_master().remove(this); + sData.mInstance = NULL; + sData.mInitState = DELETED; + } -private: + /** + * @brief Immediately delete the singleton. + * + * A subsequent call to LLProxy::getInstance() will construct a new + * instance of the class. + * + * 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. + * + * 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. + */ + static void deleteSingleton() + { + delete sData.mInstance; + sData.mInstance = NULL; + sData.mInitState = DELETED; + } - virtual void initSingleton() {} + static DERIVED_TYPE* getInstance() + { + static SingletonLifetimeManager sLifeTimeMgr; + + switch (sData.mInitState) + { + case UNINITIALIZED: + // should never be uninitialized at this point + logerrs("Uninitialized singleton ", typeid(DERIVED_TYPE).name()); + return NULL; + + case CONSTRUCTING: + logerrs("Tried to access singleton ", typeid(DERIVED_TYPE).name(), + " from singleton constructor!"); + return NULL; + + case INITIALIZING: + // go ahead and flag ourselves as initialized so we can be + // reentrant during initialization + sData.mInitState = INITIALIZED; + // 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(); + // pop this off stack of initializing singletons + sData.mInstance->pop_initializing(); + break; + + case INITIALIZED: + break; + + case DELETED: + logwarns("Trying to access deleted singleton ", typeid(DERIVED_TYPE).name(), + " -- creating new instance"); + SingletonLifetimeManager::construct(); + // same as first time construction + sData.mInitState = INITIALIZED; + sData.mInstance->initSingleton(); + // pop this off stack of initializing singletons + sData.mInstance->pop_initializing(); + break; + } + + // 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 this call + // came from another LLSingleton, rather than from vanilla application + // code, record the dependency. + sData.mInstance->capture_dependency(); + return sData.mInstance; + } - 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; + // Reference version of getInstance() + // Preferred over getInstance() as it disallows checking for NULL + static DERIVED_TYPE& instance() + { + return *getInstance(); + } + + // Has this singleton been created yet? + // Use this to avoid accessing singletons before they can safely be constructed. + static bool instanceExists() + { + return sData.mInitState == INITIALIZED; + } + +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 -- cgit v1.2.3 From 0ea1b2a164e0d985c80c8a1afea4b31670efc907 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 25 Jun 2015 16:04:01 -0400 Subject: MAINT-5232: Try to avoid circularity between LLError and LLSingleton. Part of LLError's logging infrastructure is implemented with an LLSingleton. Therefore, attempts to log from within LLSingleton machinery could potentially go south if LLError's LLSingleton is not yet initialized. Introduce LLError::is_available() in llerrorcontrol.h and llerror.cpp. Make LLSingletonBase::logwarns() and logerrs() consult LLError::is_available() before attempting to use LL_WARNS or LL_ERRS, respectively. Moreover, make all LLSingleton internal logging use logwarns() and logerrs() instead of directly engaging LL_ERRS or LL_WARNS. --- indra/llcommon/llerror.cpp | 5 ++++ indra/llcommon/llerrorcontrol.h | 5 ++++ indra/llcommon/llsingleton.cpp | 64 ++++++++++++++++++++++++++++------------- indra/llcommon/llsingleton.h | 6 ++-- 4 files changed, 58 insertions(+), 22 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2100989316..54524bbe8e 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -402,6 +402,11 @@ namespace namespace LLError { + bool is_available() + { + return Globals::instanceExists(); + } + class SettingsConfig : public LLRefCount { friend class Settings; diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 56ac52e5de..56e84f7172 100755 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -189,6 +189,11 @@ namespace LLError LL_COMMON_API std::string abbreviateFile(const std::string& filePath); LL_COMMON_API int shouldLogCallCount(); + + // Check whether Globals exists. This should only be used by LLSingleton + // infrastructure to avoid trying to log when our internal LLSingleton is + // unavailable -- circularity ensues. + LL_COMMON_API bool is_available(); }; #endif // LL_LLERRORCONTROL_H diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 204c0d24d0..a3edf925ad 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -28,9 +28,11 @@ #include "llsingleton.h" #include "llerror.h" +#include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" #include #include +#include // std::cerr in dire emergency #include #include @@ -108,14 +110,14 @@ void LLSingletonBase::pop_initializing() list_t& list(get_initializing()); if (list.empty()) { - LL_ERRS() << "Underflow in stack of currently-initializing LLSingletons at " - << typeid(*this).name() << "::getInstance()" << LL_ENDL; + logerrs("Underflow in stack of currently-initializing LLSingletons at ", + typeid(*this).name(), "::getInstance()"); } if (list.back() != this) { - LL_ERRS() << "Push/pop mismatch in stack of currently-initializing LLSingletons: " - << typeid(*this).name() << "::getInstance() trying to pop " - << typeid(*list.back()).name() << LL_ENDL; + logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", + typeid(*this).name(), "::getInstance() trying to pop ", + typeid(*list.back()).name()); } // Here we're sure that list.back() == this. Whew, pop it. list.pop_back(); @@ -151,8 +153,8 @@ void LLSingletonBase::capture_dependency() // DEBUGGING: Initially, make this crump. We want to know how bad // the problem is before we add it to the long, sad list of // ominous warnings that everyone always ignores. - LL_ERRS() << "LLSingleton circularity: " << out.str() - << typeid(*this).name() << LL_ENDL; + logerrs("LLSingleton circularity: ", out.str().c_str(), + typeid(*this).name()); } else { @@ -223,13 +225,13 @@ void LLSingletonBase::cleanupAll() } catch (const std::exception& e) { - LL_WARNS() << "Exception in " << typeid(*sp).name() - << "::cleanupSingleton(): " << e.what() << LL_ENDL; + logwarns("Exception in ", typeid(*sp).name(), + "::cleanupSingleton(): ", e.what()); } catch (...) { - LL_WARNS() << "Unknown exception in " << typeid(*sp).name() - << "::cleanupSingleton()" << LL_ENDL; + logwarns("Unknown exception in ", typeid(*sp).name(), + "::cleanupSingleton()"); } } } @@ -250,7 +252,7 @@ void LLSingletonBase::deleteAll() if (! sp->mDeleteSingleton) { // This Should Not Happen... but carry on. - LL_WARNS() << name << "::mDeleteSingleton not initialized!" << LL_ENDL; + logwarns(name, "::mDeleteSingleton not initialized!"); } else { @@ -261,13 +263,11 @@ void LLSingletonBase::deleteAll() } catch (const std::exception& e) { - LL_WARNS() << "Exception in " << name - << "::deleteSingleton(): " << e.what() << LL_ENDL; + logwarns("Exception in ", name, "::deleteSingleton(): ", e.what()); } catch (...) { - LL_WARNS() << "Unknown exception in " << name - << "::deleteSingleton()" << LL_ENDL; + logwarns("Unknown exception in ", name, "::deleteSingleton()"); } } } @@ -307,13 +307,37 @@ void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) /*---------------------------- Logging helpers -----------------------------*/ //static -void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3) +void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) { - LL_ERRS() << p1 << p2 << p3 << LL_ENDL; + // Check LLError::is_available() because some of LLError's infrastructure + // is itself an LLSingleton. If that LLSingleton has not yet been + // initialized, trying to log will engage LLSingleton machinery... and + // around and around we go. + if (LLError::is_available()) + { + LL_ERRS() << p1 << p2 << p3 << p4 << LL_ENDL; + } + else + { + // Caller may be a test program, or something else whose stderr is + // visible to the user. + std::cerr << p1 << p2 << p3 << p4 << std::endl; + // The other important side effect of LL_ERRS() is + // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) + LLError::crashAndLoop(std::string()); + } } //static -void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3) +void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, const char* p4) { - LL_WARNS() << p1 << p2 << p3 << LL_ENDL; + // See logerrs() remarks about is_available(). + if (LLError::is_available()) + { + LL_WARNS() << p1 << p2 << p3 << p4 << LL_ENDL; + } + else + { + std::cerr << p1 << p2 << p3 << p4 << std::endl; + } } diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 7706ed53f2..d66ea33c0c 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -90,9 +90,11 @@ protected: void capture_dependency(); // delegate LL_ERRS() logging to llsingleton.cpp - static void logerrs(const char* p1, const char* p2="", const char* p3=""); + 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=""); + static void logwarns(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); // obtain canonical ref_ptr_t static ref_ptr_t get_master_refcount(); -- cgit v1.2.3 From 0727519d9307ed09877073ef17b754526ee3f5b1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 25 Jun 2015 16:07:27 -0400 Subject: MAINT-5232: Correct forward declaration of LLSingleton_manage_master. The forward declaration said it was a 'friend class', whereas the actual definition is a struct. MSVC dislikes that. --- indra/llcommon/llsingleton.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index d66ea33c0c..25bdba0a0d 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -67,7 +67,7 @@ protected: void add_master(); void remove_master(); // with a little help from our friends. - template friend class LLSingleton_manage_master; + template friend struct LLSingleton_manage_master; // Maintain a stack of the LLSingleton subclass instance currently being // initialized. We use this to notice direct dependencies: we want to know -- cgit v1.2.3 From 687efd84eabc524e339e61458b0cbf53f9a38f8a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Jun 2015 13:03:59 -0400 Subject: MAINT-5232: Loosen LLSingleton circularity constraints slightly. LLSingleton explicitly supports circular dependencies: initialization performed during an LLSingleton subclass's initSingleton() method may recursively call that same subclass's getInstance() method. On the other hand, circularity from a subclass constructor cannot be permitted, else getInstance() would have to return a partially-constructed object. Our dependency tracking circularity check initially forbade both. Loosen it to permit references from within initSingleton(). --- indra/llcommon/llsingleton.cpp | 19 +++++++++++++------ indra/llcommon/llsingleton.h | 22 +++++++++++----------- 2 files changed, 24 insertions(+), 17 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index a3edf925ad..2813814ae1 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -123,7 +123,7 @@ void LLSingletonBase::pop_initializing() list.pop_back(); } -void LLSingletonBase::capture_dependency() +void LLSingletonBase::capture_dependency(EInitState initState) { // Did this getInstance() call come from another LLSingleton, or from // vanilla application code? Note that although this is a nontrivial @@ -150,11 +150,18 @@ void LLSingletonBase::capture_dependency() // is the actual LLSingletonBase instance. out << typeid(**found).name() << " -> "; } - // DEBUGGING: Initially, make this crump. We want to know how bad - // the problem is before we add it to the long, sad list of - // ominous warnings that everyone always ignores. - logerrs("LLSingleton circularity: ", out.str().c_str(), - typeid(*this).name()); + // 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 based on initState. They have + // identical signatures. + ((initState == CONSTRUCTING)? logerrs : logwarns) + ("LLSingleton circularity: ", out.str().c_str(), typeid(*this).name(), ""); } else { diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 25bdba0a0d..a82101c367 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -58,6 +58,15 @@ private: set_t mDepends; protected: + typedef enum e_init_state + { + UNINITIALIZED = 0, // must be default-initialized state + CONSTRUCTING, + INITIALIZING, + INITIALIZED, + DELETED + } EInitState; + // Base-class constructor should only be invoked by the DERIVED_TYPE // constructor. LLSingletonBase(); @@ -87,7 +96,7 @@ protected: void pop_initializing(); // 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(); + void capture_dependency(EInitState); // delegate LL_ERRS() logging to llsingleton.cpp static void logerrs(const char* p1, const char* p2="", @@ -232,15 +241,6 @@ template class LLSingleton : public LLSingletonBase { private: - typedef enum e_init_state - { - UNINITIALIZED = 0, // must be default-initialized state - CONSTRUCTING, - INITIALIZING, - INITIALIZED, - DELETED - } EInitState; - static DERIVED_TYPE* constructSingleton() { return new DERIVED_TYPE(); @@ -377,7 +377,7 @@ public: // an LLSingleton that directly depends on DERIVED_TYPE. If this call // came from another LLSingleton, rather than from vanilla application // code, record the dependency. - sData.mInstance->capture_dependency(); + sData.mInstance->capture_dependency(sData.mInitState); return sData.mInstance; } -- cgit v1.2.3 From 33c88a1c6037290924691db59c6538a370946ea4 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Jun 2015 15:27:06 -0400 Subject: MAINT-5232: Publish class name demangler as LLError::Log::demangle(). We've had this functionality buried in llerror.cpp for years. Make it available for callers outside llerror.cpp too. --- indra/llcommon/llerror.cpp | 21 ++++++++++++++++----- indra/llcommon/llerror.h | 1 + 2 files changed, 17 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2100989316..f2c647c2d9 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -237,6 +237,14 @@ namespace { namespace { std::string className(const std::type_info& type) + { + return LLError::Log::demangle(type.name()); + } +} // anonymous + +namespace LLError +{ + std::string Log::demangle(const char* mangled) { #ifdef __GNUC__ // GCC: type_info::name() returns a mangled class name,st demangle @@ -251,18 +259,18 @@ namespace // but gcc 3.3 libstc++'s implementation of demangling is broken // and fails without. - char* name = abi::__cxa_demangle(type.name(), + char* name = abi::__cxa_demangle(mangled, abi_name_buf, &abi_name_len, &status); // this call can realloc the abi_name_buf pointer (!) - return name ? name : type.name(); + return name ? name : mangled; #elif LL_WINDOWS // DevStudio: type_info::name() includes the text "class " at the start static const std::string class_prefix = "class "; - std::string name = type.name(); + std::string name = mangled; std::string::size_type p = name.find(class_prefix); if (p == std::string::npos) { @@ -271,11 +279,14 @@ namespace return name.substr(p + class_prefix.size()); -#else - return type.name(); +#else + return mangled; #endif } +} // LLError +namespace +{ std::string functionName(const std::string& preprocessor_name) { #if LL_WINDOWS diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 63040e1772..d0ddb5e8e9 100755 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -189,6 +189,7 @@ namespace LLError static std::ostringstream* out(); static void flush(std::ostringstream* out, char* message); static void flush(std::ostringstream*, const CallSite&); + static std::string demangle(const char* mangled); }; struct LL_COMMON_API CallSite -- cgit v1.2.3 From 8fee1565eb310081a7f3e26237ddd776c5a9aaaa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Jun 2015 15:28:57 -0400 Subject: MAINT-5232: Use LLError::Log::demangle() to log LLSingleton classes. --- indra/llcommon/llsingleton.cpp | 28 +++++++++++++++++----------- indra/llcommon/llsingleton.h | 10 +++++++--- 2 files changed, 24 insertions(+), 14 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 2813814ae1..b78110296f 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -111,13 +111,13 @@ void LLSingletonBase::pop_initializing() if (list.empty()) { logerrs("Underflow in stack of currently-initializing LLSingletons at ", - typeid(*this).name(), "::getInstance()"); + demangle(typeid(*this).name()).c_str(), "::getInstance()"); } if (list.back() != this) { logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", - typeid(*this).name(), "::getInstance() trying to pop ", - typeid(*list.back()).name()); + demangle(typeid(*this).name()).c_str(), "::getInstance() trying to pop ", + demangle(typeid(*list.back()).name()).c_str()); } // Here we're sure that list.back() == this. Whew, pop it. list.pop_back(); @@ -148,7 +148,7 @@ void LLSingletonBase::capture_dependency(EInitState initState) { // 'found' is an iterator; *found is an LLSingletonBase*; **found // is the actual LLSingletonBase instance. - out << typeid(**found).name() << " -> "; + out << demangle(typeid(**found).name()) << " -> "; } // We promise to capture dependencies from both the constructor // and the initSingleton() method, so an LLSingleton's instance @@ -161,7 +161,8 @@ void LLSingletonBase::capture_dependency(EInitState initState) // Decide which log helper to call based on initState. They have // identical signatures. ((initState == CONSTRUCTING)? logerrs : logwarns) - ("LLSingleton circularity: ", out.str().c_str(), typeid(*this).name(), ""); + ("LLSingleton circularity: ", out.str().c_str(), + demangle(typeid(*this).name()).c_str(), ""); } else { @@ -232,12 +233,12 @@ void LLSingletonBase::cleanupAll() } catch (const std::exception& e) { - logwarns("Exception in ", typeid(*sp).name(), + logwarns("Exception in ", demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton(): ", e.what()); } catch (...) { - logwarns("Unknown exception in ", typeid(*sp).name(), + logwarns("Unknown exception in ", demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton()"); } } @@ -252,14 +253,14 @@ void LLSingletonBase::deleteAll() { // Capture the class name first: in case of exception, don't count on // being able to extract it later. - const char* name = typeid(*sp).name(); + const std::string name = demangle(typeid(*sp).name()); try { // Call static method through instance function pointer. if (! sp->mDeleteSingleton) { // This Should Not Happen... but carry on. - logwarns(name, "::mDeleteSingleton not initialized!"); + logwarns(name.c_str(), "::mDeleteSingleton not initialized!"); } else { @@ -270,11 +271,11 @@ void LLSingletonBase::deleteAll() } catch (const std::exception& e) { - logwarns("Exception in ", name, "::deleteSingleton(): ", e.what()); + logwarns("Exception in ", name.c_str(), "::deleteSingleton(): ", e.what()); } catch (...) { - logwarns("Unknown exception in ", name, "::deleteSingleton()"); + logwarns("Unknown exception in ", name.c_str(), "::deleteSingleton()"); } } } @@ -348,3 +349,8 @@ void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, c std::cerr << p1 << p2 << p3 << p4 << std::endl; } } + +std::string LLSingletonBase::demangle(const char* mangled) +{ + return LLError::Log::demangle(mangled); +} diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index a82101c367..253e0b9a6b 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -104,6 +104,7 @@ protected: // delegate LL_WARNS() logging to llsingleton.cpp static void logwarns(const char* p1, const char* p2="", const char* p3="", const char* p4=""); + static std::string demangle(const char* mangled); // obtain canonical ref_ptr_t static ref_ptr_t get_master_refcount(); @@ -337,11 +338,13 @@ public: { case UNINITIALIZED: // should never be uninitialized at this point - logerrs("Uninitialized singleton ", typeid(DERIVED_TYPE).name()); + logerrs("Uninitialized singleton ", + demangle(typeid(DERIVED_TYPE).name()).c_str()); return NULL; case CONSTRUCTING: - logerrs("Tried to access singleton ", typeid(DERIVED_TYPE).name(), + logerrs("Tried to access singleton ", + demangle(typeid(DERIVED_TYPE).name()).c_str(), " from singleton constructor!"); return NULL; @@ -361,7 +364,8 @@ public: break; case DELETED: - logwarns("Trying to access deleted singleton ", typeid(DERIVED_TYPE).name(), + logwarns("Trying to access deleted singleton ", + demangle(typeid(DERIVED_TYPE).name()).c_str(), " -- creating new instance"); SingletonLifetimeManager::construct(); // same as first time construction -- cgit v1.2.3 From d4fb82c217bccda536f7a7b2ca1809bb8c2dba40 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 30 Jun 2015 14:49:18 -0400 Subject: MAINT-5232: Add tests for new LLSingleton dependency functionality. --- indra/llcommon/llsingleton.h | 6 +- indra/llcommon/tests/llsingleton_test.cpp | 205 ++++++++++++++++++++++++------ 2 files changed, 167 insertions(+), 44 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 253e0b9a6b..6a7f27bed4 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -32,8 +32,6 @@ #include #include -// TODO: -// Tests for all this! class LLSingletonBase: private boost::noncopyable { public: @@ -80,8 +78,8 @@ protected: // Maintain a stack of the LLSingleton subclass instance currently being // initialized. We use this to notice direct dependencies: we want to know - // if A requires B. We deduce that if while initializing A, control - // reaches B::getInstance(). + // if A requires B. We deduce a dependency if while initializing A, + // control reaches B::getInstance(). // We want &A to be at the top of that stack during both A::A() and // A::initSingleton(), since a call to B::getInstance() might occur during // either. diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp index bed436283a..a05f650f25 100755 --- a/indra/llcommon/tests/llsingleton_test.cpp +++ b/indra/llcommon/tests/llsingleton_test.cpp @@ -30,46 +30,171 @@ #include "llsingleton.h" #include "../test/lltut.h" + +// Capture execution sequence by appending to log string. +std::string sLog; + +#define DECLARE_CLASS(CLS) \ +struct CLS: public LLSingleton \ +{ \ + static enum dep_flag { \ + DEP_NONE, /* no dependency */ \ + DEP_CTOR, /* dependency in ctor */ \ + DEP_INIT /* dependency in initSingleton */ \ + } sDepFlag; \ + \ + CLS(); \ + void initSingleton(); \ + void cleanupSingleton(); \ + ~CLS(); \ +}; \ + \ +CLS::dep_flag CLS::sDepFlag = DEP_NONE + +DECLARE_CLASS(A); +DECLARE_CLASS(B); + +#define DEFINE_MEMBERS(CLS, OTHER) \ +CLS::CLS() \ +{ \ + sLog.append(#CLS); \ + if (sDepFlag == DEP_CTOR) \ + { \ + (void)OTHER::instance(); \ + } \ +} \ + \ +void CLS::initSingleton() \ +{ \ + sLog.append("i" #CLS); \ + if (sDepFlag == DEP_INIT) \ + { \ + (void)OTHER::instance(); \ + } \ +} \ + \ +void CLS::cleanupSingleton() \ +{ \ + sLog.append("x" #CLS); \ +} \ + \ +CLS::~CLS() \ +{ \ + sLog.append("~" #CLS); \ +} + +DEFINE_MEMBERS(A, B) +DEFINE_MEMBERS(B, A) + namespace tut { - struct singleton - { - // We need a class created with the LLSingleton template to test with. - class LLSingletonTest: public LLSingleton - { - - }; - }; - - typedef test_group singleton_t; - typedef singleton_t::object singleton_object_t; - tut::singleton_t tut_singleton("LLSingleton"); - - template<> template<> - void singleton_object_t::test<1>() - { - - } - template<> template<> - void singleton_object_t::test<2>() - { - LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); - ensure(singleton_test); - } - template<> template<> - void singleton_object_t::test<3>() - { - //Construct the instance - LLSingletonTest::getInstance(); - ensure(LLSingletonTest::instanceExists()); - - //Delete the instance - LLSingletonTest::deleteSingleton(); - ensure(!LLSingletonTest::instanceExists()); - - //Construct it again. - LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); - ensure(singleton_test); - ensure(LLSingletonTest::instanceExists()); - } + struct singleton + { + // We need a class created with the LLSingleton template to test with. + class LLSingletonTest: public LLSingleton + { + + }; + }; + + typedef test_group singleton_t; + typedef singleton_t::object singleton_object_t; + tut::singleton_t tut_singleton("LLSingleton"); + + template<> template<> + void singleton_object_t::test<1>() + { + + } + template<> template<> + void singleton_object_t::test<2>() + { + LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); + ensure(singleton_test); + } + + template<> template<> + void singleton_object_t::test<3>() + { + //Construct the instance + LLSingletonTest::getInstance(); + ensure(LLSingletonTest::instanceExists()); + + //Delete the instance + LLSingletonTest::deleteSingleton(); + ensure(!LLSingletonTest::instanceExists()); + + //Construct it again. + LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); + ensure(singleton_test); + ensure(LLSingletonTest::instanceExists()); + } + +#define TESTS(CLS, OTHER, N0, N1, N2, N3) \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name("just " #CLS); \ + CLS::sDepFlag = CLS::DEP_NONE; \ + OTHER::sDepFlag = OTHER::DEP_NONE; \ + sLog.clear(); \ + \ + (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); \ + } \ + \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name(#CLS " ctor depends " #OTHER); \ + CLS::sDepFlag = CLS::DEP_CTOR; \ + OTHER::sDepFlag = OTHER::DEP_NONE; \ + sLog.clear(); \ + \ + (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); \ + } \ + \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name(#CLS " init depends " #OTHER); \ + CLS::sDepFlag = CLS::DEP_INIT; \ + OTHER::sDepFlag = OTHER::DEP_NONE; \ + sLog.clear(); \ + \ + (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); \ + } \ + \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name(#CLS " circular init"); \ + CLS::sDepFlag = CLS::DEP_INIT; \ + OTHER::sDepFlag = OTHER::DEP_CTOR; \ + sLog.clear(); \ + \ + (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); \ + } + + TESTS(A, B, 4, 5, 6, 7) + TESTS(B, A, 8, 9, 10, 11) } -- cgit v1.2.3 From eddce74206328f213d83c9a76432645a7d3dbc22 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 30 Aug 2016 14:08:32 -0400 Subject: MAINT-5232: Do less work inside typeid() calls. clang gets nervous about expressions that call functions inside typeid(), even though these particular typeid() calls are runtime expressions on runtime values. Extract the offending calls to a previous statement. --- indra/llcommon/llsingleton.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index b78110296f..8c8ded0e51 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -115,9 +115,10 @@ void LLSingletonBase::pop_initializing() } if (list.back() != this) { + LLSingletonBase* back(list.back()); logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", demangle(typeid(*this).name()).c_str(), "::getInstance() trying to pop ", - demangle(typeid(*list.back()).name()).c_str()); + demangle(typeid(*back).name()).c_str()); } // Here we're sure that list.back() == this. Whew, pop it. list.pop_back(); @@ -148,7 +149,8 @@ void LLSingletonBase::capture_dependency(EInitState initState) { // 'found' is an iterator; *found is an LLSingletonBase*; **found // is the actual LLSingletonBase instance. - out << demangle(typeid(**found).name()) << " -> "; + LLSingletonBase* foundp(*found); + out << demangle(typeid(*foundp).name()) << " -> "; } // We promise to capture dependencies from both the constructor // and the initSingleton() method, so an LLSingleton's instance -- cgit v1.2.3 From dcdccb3ef1b42b36042fa6f3fe61849124e1728f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 30 Aug 2016 16:28:21 -0400 Subject: MAINT-5232: Move "llerror.h" out of llcleanup.h, llinitdestroyclass.h Introduce corresponding llcleanup.cpp, llinitdestroyclass.cpp modules to contain code that performs logging calls. Track class::method names for LLInitClass and LLDestroyClass subclasses, and log them when called. The order in which these calls occur could be relevant to bugs, and could surface the need to convert to LLSingleton dependencies. --- indra/llcommon/CMakeLists.txt | 3 ++ indra/llcommon/llcleanup.cpp | 28 +++++++++++++++++ indra/llcommon/llcleanup.h | 7 +++-- indra/llcommon/llinitdestroyclass.cpp | 30 ++++++++++++++++++ indra/llcommon/llinitdestroyclass.h | 59 ++++++++++++++--------------------- 5 files changed, 90 insertions(+), 37 deletions(-) create mode 100644 indra/llcommon/llcleanup.cpp create mode 100644 indra/llcommon/llinitdestroyclass.cpp (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 556f879634..e1261ee17d 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -40,6 +40,7 @@ set(llcommon_SOURCE_FILES llbase64.cpp llbitpack.cpp llcallbacklist.cpp + llcleanup.cpp llcommon.cpp llcommonutils.cpp llcoros.cpp @@ -66,6 +67,7 @@ set(llcommon_SOURCE_FILES llframetimer.cpp llheartbeat.cpp llinitparam.cpp + llinitdestroyclass.cpp llinstancetracker.cpp llleap.cpp llleaplistener.cpp @@ -134,6 +136,7 @@ set(llcommon_HEADER_FILES llbitpack.h llboost.h llcallbacklist.h + llcleanup.h llcommon.h llcommonutils.h llcoros.h diff --git a/indra/llcommon/llcleanup.cpp b/indra/llcommon/llcleanup.cpp new file mode 100644 index 0000000000..f45f4925ce --- /dev/null +++ b/indra/llcommon/llcleanup.cpp @@ -0,0 +1,28 @@ +/** + * @file llcleanup.cpp + * @author Nat Goodspeed + * @date 2016-08-30 + * @brief Implementation for llcleanup. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llcleanup.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "llerror.h" + +void log_subsystem_cleanup(const char* file, int line, const char* function, + const char* classname) +{ + LL_INFOS("Cleanup") << file << "(" << line << "): calling " + << classname << "::cleanupClass() in " + << function << LL_ENDL; +} diff --git a/indra/llcommon/llcleanup.h b/indra/llcommon/llcleanup.h index 8eda9a7fb3..a319171b5f 100644 --- a/indra/llcommon/llcleanup.h +++ b/indra/llcommon/llcleanup.h @@ -12,7 +12,7 @@ #if ! defined(LL_LLCLEANUP_H) #define LL_LLCLEANUP_H -#include "llerror.h" +#include // Instead of directly calling SomeClass::cleanupClass(), use // SUBSYSTEM_CLEANUP(SomeClass); @@ -21,10 +21,13 @@ // shutdown schemes. #define SUBSYSTEM_CLEANUP(CLASSNAME) \ do { \ - LL_INFOS("Cleanup") << "Calling " #CLASSNAME "::cleanupClass()" << LL_ENDL; \ + log_subsystem_cleanup(__FILE__, __LINE__, BOOST_CURRENT_FUNCTION, #CLASSNAME); \ CLASSNAME::cleanupClass(); \ } while (0) // Use ancient do { ... } while (0) macro trick to permit a block of // statements with the same syntax as a single statement. +void log_subsystem_cleanup(const char* file, int line, const char* function, + const char* classname); + #endif /* ! defined(LL_LLCLEANUP_H) */ diff --git a/indra/llcommon/llinitdestroyclass.cpp b/indra/llcommon/llinitdestroyclass.cpp new file mode 100644 index 0000000000..e6382a7924 --- /dev/null +++ b/indra/llcommon/llinitdestroyclass.cpp @@ -0,0 +1,30 @@ +/** + * @file llinitdestroyclass.cpp + * @author Nat Goodspeed + * @date 2016-08-30 + * @brief Implementation for llinitdestroyclass. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llinitdestroyclass.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "llerror.h" + +void LLCallbackRegistry::fireCallbacks() const +{ + for (FuncList::const_iterator fi = mCallbacks.begin(), fe = mCallbacks.end(); + fi != fe; ++fi) + { + LL_INFOS("LLInitDestroyClass") << "calling " << fi->first << "()" << LL_ENDL; + fi->second(); + } +} diff --git a/indra/llcommon/llinitdestroyclass.h b/indra/llcommon/llinitdestroyclass.h index 49bcefc33d..9c66211475 100644 --- a/indra/llcommon/llinitdestroyclass.h +++ b/indra/llcommon/llinitdestroyclass.h @@ -36,34 +36,35 @@ #if ! defined(LL_LLINITDESTROYCLASS_H) #define LL_LLINITDESTROYCLASS_H -#include "llerror.h" #include "llsingleton.h" #include -#include #include +#include +#include // std::pair /** * LLCallbackRegistry is an implementation detail base class for - * LLInitClassList and LLDestroyClassList. It's a very thin wrapper around a - * Boost.Signals2 signal object. + * LLInitClassList and LLDestroyClassList. It accumulates the initClass() or + * destroyClass() callbacks for registered classes. */ class LLCallbackRegistry { public: - typedef boost::signals2::signal callback_signal_t; - - void registerCallback(const callback_signal_t::slot_type& slot) - { - mCallbacks.connect(slot); - } + typedef boost::function func_t; - void fireCallbacks() + void registerCallback(const std::string& name, const func_t& func) { - mCallbacks(); + mCallbacks.push_back(FuncList::value_type(name, func)); } + void fireCallbacks() const; + private: - callback_signal_t mCallbacks; + // Arguably this should be a boost::signals2::signal, which is, after all, + // a sequence of callables. We manage it by hand so we can log a name for + // each registered function we call. + typedef std::vector< std::pair > FuncList; + FuncList mCallbacks; }; /** @@ -108,9 +109,9 @@ template class LLRegisterWith { public: - LLRegisterWith(boost::function func) + LLRegisterWith(const std::string& name, const LLCallbackRegistry::func_t& func) { - T::instance().registerCallback(func); + T::instance().registerCallback(name, func); } // this avoids a MSVC bug where non-referenced static members are "optimized" away @@ -141,15 +142,6 @@ public: // When this static member is initialized, the subclass initClass() method // is registered on LLInitClassList. See sRegister definition below. static LLRegisterWith sRegister; -private: - - // Provide a default initClass() method in case subclass misspells (or - // omits) initClass(). This turns a potential build error into a fatal - // runtime error. - static void initClass() - { - LL_ERRS() << "No static initClass() method defined for " << typeid(T).name() << LL_ENDL; - } }; /** @@ -171,20 +163,17 @@ public: // method is registered on LLInitClassList. See sRegister definition // below. static LLRegisterWith sRegister; -private: - - // Provide a default destroyClass() method in case subclass misspells (or - // omits) destroyClass(). This turns a potential build error into a fatal - // runtime error. - static void destroyClass() - { - LL_ERRS() << "No static destroyClass() method defined for " << typeid(T).name() << LL_ENDL; - } }; // Here's where LLInitClass specifies the subclass initClass() method. -template LLRegisterWith LLInitClass::sRegister(&T::initClass); +template +LLRegisterWith +LLInitClass::sRegister(std::string(typeid(T).name()) + "::initClass", + &T::initClass); // Here's where LLDestroyClass specifies the subclass destroyClass() method. -template LLRegisterWith LLDestroyClass::sRegister(&T::destroyClass); +template +LLRegisterWith +LLDestroyClass::sRegister(std::string(typeid(T).name()) + "::destroyClass", + &T::destroyClass); #endif /* ! defined(LL_LLINITDESTROYCLASS_H) */ -- cgit v1.2.3 From 37d3993a59f01bae55049b08d196733121b54f7f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 30 Aug 2016 16:42:24 -0400 Subject: MAINT-5232: Consolidate special LLSingletonBase logging logic. --- indra/llcommon/llsingleton.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 8c8ded0e51..c3e8f6c7c7 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -316,8 +316,9 @@ void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) } /*---------------------------- Logging helpers -----------------------------*/ -//static -void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) +namespace { +void log(LLError::ELevel level, + const char* p1, const char* p2, const char* p3, const char* p4) { // Check LLError::is_available() because some of LLError's infrastructure // is itself an LLSingleton. If that LLSingleton has not yet been @@ -325,31 +326,30 @@ void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, co // around and around we go. if (LLError::is_available()) { - LL_ERRS() << p1 << p2 << p3 << p4 << LL_ENDL; + lllog(level, false) << p1 << p2 << p3 << p4 << LL_ENDL; } else { // Caller may be a test program, or something else whose stderr is // visible to the user. std::cerr << p1 << p2 << p3 << p4 << std::endl; - // The other important side effect of LL_ERRS() is - // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) - LLError::crashAndLoop(std::string()); } } +} // anonymous namespace //static void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, const char* p4) { - // See logerrs() remarks about is_available(). - if (LLError::is_available()) - { - LL_WARNS() << p1 << p2 << p3 << p4 << LL_ENDL; - } - else - { - std::cerr << p1 << p2 << p3 << p4 << std::endl; - } + log(LLError::LEVEL_WARN, p1, p2, p3, p4); +} + +//static +void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) +{ + log(LLError::LEVEL_ERROR, p1, p2, p3, p4); + // The other important side effect of LL_ERRS() is + // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) + LLError::crashAndLoop(std::string()); } std::string LLSingletonBase::demangle(const char* mangled) -- cgit v1.2.3 From f5ccfff54299becdbff81661622375d319e5ea7b Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 31 Aug 2016 14:25:55 -0400 Subject: MAINT-5232: Add DEBUG logging to LLSingleton operations. Specifically, log as LLSingleton captures inter-Singleton dependencies. Also log cleanupAll() calls to cleanupSingleton() and deleteAll() calls to deleteSingleton(), since they happen in an implicitly-determined order. But do not log anything during the implicit LLSingletonBase::deleteAll() call triggered by the runtime destroying the last LLSingleton's static data. That's too late in the run; even std::cerr might already have been destroyed! --- indra/llcommon/llsingleton.cpp | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index c3e8f6c7c7..76789e438e 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -36,6 +36,11 @@ #include #include +namespace { +void log(LLError::ELevel level, + const char* p1="", const char* p2="", const char* p3="", const char* p4=""); +} // anonymous namespace + // Our master list of all LLSingletons is itself an LLSingleton. We used to // store it in a function-local static, but that could get destroyed before // the last of the LLSingletons -- and ~LLSingletonBase() definitely wants to @@ -172,7 +177,13 @@ void LLSingletonBase::capture_dependency(EInitState initState) // Record the dependency. // initializing.back() is the LLSingletonBase* currently being // initialized. Store 'this' in its mDepends set. - initializing.back()->mDepends.insert(this); + LLSingletonBase* current(initializing.back()); + if (current->mDepends.insert(this).second) + { + // only log the FIRST time we hit this dependency! + log(LLError::LEVEL_DEBUG, demangle(typeid(*current).name()).c_str(), + " depends on ", demangle(typeid(*this).name()).c_str()); + } } } } @@ -229,6 +240,8 @@ void LLSingletonBase::cleanupAll() { sp->mCleaned = true; + log(LLError::LEVEL_DEBUG, "calling ", + demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton()"); try { sp->cleanupSingleton(); @@ -267,6 +280,7 @@ void LLSingletonBase::deleteAll() else { // properly initialized: call it. + log(LLError::LEVEL_DEBUG, "calling ", name.c_str(), "::deleteSingleton()"); // From this point on, DO NOT DEREFERENCE sp! sp->mDeleteSingleton(); } @@ -320,6 +334,19 @@ namespace { void log(LLError::ELevel level, const char* p1, const char* p2, const char* p3, const char* p4) { + // Check whether we're in the implicit final LLSingletonBase::deleteAll() + // call. We've carefully arranged for deleteAll() to be called when the + // last SingletonLifetimeManager instance is destroyed -- in other words, + // when the last translation unit containing an LLSingleton instance + // cleans up static data. That could happen after std::cerr is destroyed! + // The is_available() test below ensures that we'll stop logging once + // LLError has been cleaned up. If we had a similar portable test for + // std::cerr, this would be a good place to use it. As we do not, just + // don't log anything during implicit final deleteAll(). Detect that by + // the master refcount having gone to zero. + if (sMasterRefcount.refcount == 0) + return; + // Check LLError::is_available() because some of LLError's infrastructure // is itself an LLSingleton. If that LLSingleton has not yet been // initialized, trying to log will engage LLSingleton machinery... and -- cgit v1.2.3 From 3e1589709ff005822ddf2c17558b4195fa5e5c81 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 31 Aug 2016 14:46:03 -0400 Subject: MAINT-5232: LLMetricPerformanceTesterBasic::cleanClass->cleanupClass for consistency with everything else, so we can use SUBSYSTEM_CLEANUP() macro to call it. --- indra/llcommon/llmetricperformancetester.cpp | 2 +- indra/llcommon/llmetricperformancetester.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llmetricperformancetester.cpp b/indra/llcommon/llmetricperformancetester.cpp index 1fc821d9a9..16fc365da1 100644 --- a/indra/llcommon/llmetricperformancetester.cpp +++ b/indra/llcommon/llmetricperformancetester.cpp @@ -40,7 +40,7 @@ LLMetricPerformanceTesterBasic::name_tester_map_t LLMetricPerformanceTesterBasic::sTesterMap ; /*static*/ -void LLMetricPerformanceTesterBasic::cleanClass() +void LLMetricPerformanceTesterBasic::cleanupClass() { for (name_tester_map_t::iterator iter = sTesterMap.begin() ; iter != sTesterMap.end() ; ++iter) { diff --git a/indra/llcommon/llmetricperformancetester.h b/indra/llcommon/llmetricperformancetester.h index 1a18cdf36f..e6b46be1cf 100644 --- a/indra/llcommon/llmetricperformancetester.h +++ b/indra/llcommon/llmetricperformancetester.h @@ -156,7 +156,7 @@ public: /** * @brief Delete all testers and reset the tester map */ - static void cleanClass() ; + static void cleanupClass() ; private: // Add a tester to the map. Returns false if adding fails. -- cgit v1.2.3 From 334eb89d8e993c05511f858b6a36732e7dd659df Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 31 Aug 2016 16:17:50 -0400 Subject: MAINT-5232: Add a tag to LLSingleton log messages. --- indra/llcommon/llsingleton.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 76789e438e..4212014e1b 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -353,7 +353,7 @@ void log(LLError::ELevel level, // around and around we go. if (LLError::is_available()) { - lllog(level, false) << p1 << p2 << p3 << p4 << LL_ENDL; + lllog(level, false, "LLSingleton") << p1 << p2 << p3 << p4 << LL_ENDL; } else { -- cgit v1.2.3 From 56a83d2115f3dc55902a3a9aa43afbfeeb4a462a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 1 Sep 2016 19:53:56 -0400 Subject: MAINT-5011: Use LL_VLOGS() rather than raw lllog() macro. Raw lllog() doesn't work for varying log level, which is why LL_VLOGS() exists. --- indra/llcommon/llsingleton.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 4212014e1b..cd5c2a7f0e 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -353,7 +353,7 @@ void log(LLError::ELevel level, // around and around we go. if (LLError::is_available()) { - lllog(level, false, "LLSingleton") << p1 << p2 << p3 << p4 << LL_ENDL; + LL_VLOGS(level, "LLSingleton") << p1 << p2 << p3 << p4 << LL_ENDL; } else { -- cgit v1.2.3 From a05ee7324df475231d5b46a1ba6b20f23ce71d0f Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 2 Sep 2016 14:03:28 -0400 Subject: MAINT-5232: Abbreviate __FILE__ path in log_subsystem_cleanup(). LLError::abbreviateFile() is specifically to avoid cluttering log output with the prefix of an absolute file path on the original build system, pointless for anyone trying to read the log. --- indra/llcommon/llcleanup.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcleanup.cpp b/indra/llcommon/llcleanup.cpp index f45f4925ce..c5283507bf 100644 --- a/indra/llcommon/llcleanup.cpp +++ b/indra/llcommon/llcleanup.cpp @@ -18,11 +18,12 @@ // external library headers // other Linden headers #include "llerror.h" +#include "llerrorcontrol.h" void log_subsystem_cleanup(const char* file, int line, const char* function, const char* classname) { - LL_INFOS("Cleanup") << file << "(" << line << "): calling " - << classname << "::cleanupClass() in " + LL_INFOS("Cleanup") << LLError::abbreviateFile(file) << "(" << line << "): " + << "calling " << classname << "::cleanupClass() in " << function << LL_ENDL; } -- cgit v1.2.3 From 4af7e496b489aaddea60453f40fd422ef5381a2d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 3 Sep 2016 11:22:54 -0400 Subject: MAINT-5232: Make LLError::is_available() depend on both LLSingletons. LLError machinery depends on two different LLSingletons. Its is_available() function is primarily for LLSingleton itself to determine whether it is, or is not, safe to log. Until both of LLError's LLSingletons have been constructed, attempting to log LLSingleton operations could produce infinite recursion. --- indra/llcommon/llerror.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 638cecb054..a34b50f816 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -414,11 +414,6 @@ namespace namespace LLError { - bool is_available() - { - return Globals::instanceExists(); - } - class SettingsConfig : public LLRefCount { friend class Settings; @@ -458,7 +453,7 @@ namespace LLError Settings(); SettingsConfigPtr getSettingsConfig(); - + void reset(); SettingsStoragePtr saveAndReset(); void restore(SettingsStoragePtr pSettingsStorage); @@ -466,7 +461,7 @@ namespace LLError private: SettingsConfigPtr mSettingsConfig; }; - + SettingsConfig::SettingsConfig() : LLRefCount(), mPrintLocation(false), @@ -501,26 +496,31 @@ namespace LLError { return mSettingsConfig; } - + void Settings::reset() { Globals::getInstance()->invalidateCallSites(); mSettingsConfig = new SettingsConfig(); } - + SettingsStoragePtr Settings::saveAndReset() { SettingsStoragePtr oldSettingsConfig(mSettingsConfig.get()); reset(); return oldSettingsConfig; } - + void Settings::restore(SettingsStoragePtr pSettingsStorage) { Globals::getInstance()->invalidateCallSites(); SettingsConfigPtr newSettingsConfig(dynamic_cast(pSettingsStorage.get())); mSettingsConfig = newSettingsConfig; } + + bool is_available() + { + return Settings::instanceExists() && Globals::instanceExists(); + } } namespace LLError -- cgit v1.2.3 From c71e6222295a1fb183436b5d67c21bffddfaefa6 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 3 Sep 2016 11:30:53 -0400 Subject: MAINT-5232: Add DEBUG logging to LLSingleton dependency tracking. Specifically, add DEBUG logging to the code that maintains the stack of LLSingletons currently being initialized. This involves passing LLSingletonBase's constructor the name of LLSingleton's template parameter subclass, since during that constructor typeid(*this).name() will only produce "LLSingletonBase". Also add logdebugs() and oktolog() helper functions. --- indra/llcommon/llsingleton.cpp | 67 ++++++++++++++++++++++++++++++++++-------- indra/llcommon/llsingleton.h | 16 +++++++--- 2 files changed, 66 insertions(+), 17 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index cd5c2a7f0e..479244400d 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -38,7 +38,11 @@ namespace { void log(LLError::ELevel level, - const char* p1="", const char* p2="", const char* p3="", const char* p4=""); + 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 // Our master list of all LLSingletons is itself an LLSingleton. We used to @@ -95,38 +99,64 @@ LLSingletonBase::list_t& LLSingletonBase::get_initializing() return sList; } -LLSingletonBase::LLSingletonBase(): +LLSingletonBase::LLSingletonBase(const char* name): mCleaned(false), mDeleteSingleton(NULL) { // Make this the currently-initializing LLSingleton. - push_initializing(); + push_initializing(name); } LLSingletonBase::~LLSingletonBase() {} -void LLSingletonBase::push_initializing() +void LLSingletonBase::push_initializing(const char* name) { + // log BEFORE pushing so logging singletons don't cry circularity + log_initializing("Pushing", name); get_initializing().push_back(this); } void LLSingletonBase::pop_initializing() { list_t& list(get_initializing()); + if (list.empty()) { logerrs("Underflow in stack of currently-initializing LLSingletons at ", demangle(typeid(*this).name()).c_str(), "::getInstance()"); } - if (list.back() != this) + + // Now we know list.back() exists: capture it + LLSingletonBase* back(list.back()); + // and pop it + list.pop_back(); + + if (back != this) { - LLSingletonBase* back(list.back()); logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", demangle(typeid(*this).name()).c_str(), "::getInstance() trying to pop ", demangle(typeid(*back).name()).c_str()); } - // Here we're sure that list.back() == this. Whew, pop it. - list.pop_back(); + + // log AFTER popping so logging singletons don't cry circularity + log_initializing("Popping", typeid(*back).name()); +} + +//static +void LLSingletonBase::log_initializing(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) + { + LLSingletonBase* sb(*ri); + LL_CONT << ' ' << demangle(typeid(*sb).name()); + } + LL_ENDL; + } } void LLSingletonBase::capture_dependency(EInitState initState) @@ -181,8 +211,8 @@ void LLSingletonBase::capture_dependency(EInitState initState) if (current->mDepends.insert(this).second) { // only log the FIRST time we hit this dependency! - log(LLError::LEVEL_DEBUG, demangle(typeid(*current).name()).c_str(), - " depends on ", demangle(typeid(*this).name()).c_str()); + logdebugs(demangle(typeid(*current).name()).c_str(), + " depends on ", demangle(typeid(*this).name()).c_str()); } } } @@ -240,8 +270,8 @@ void LLSingletonBase::cleanupAll() { sp->mCleaned = true; - log(LLError::LEVEL_DEBUG, "calling ", - demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton()"); + logdebugs("calling ", + demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton()"); try { sp->cleanupSingleton(); @@ -280,7 +310,7 @@ void LLSingletonBase::deleteAll() else { // properly initialized: call it. - log(LLError::LEVEL_DEBUG, "calling ", name.c_str(), "::deleteSingleton()"); + logdebugs("calling ", name.c_str(), "::deleteSingleton()"); // From this point on, DO NOT DEREFERENCE sp! sp->mDeleteSingleton(); } @@ -331,6 +361,12 @@ void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) /*---------------------------- Logging helpers -----------------------------*/ namespace { +bool oktolog() +{ + // See comments in log() below. + return sMasterRefcount.refcount && LLError::is_available(); +} + void log(LLError::ELevel level, const char* p1, const char* p2, const char* p3, const char* p4) { @@ -362,6 +398,11 @@ void log(LLError::ELevel level, std::cerr << p1 << p2 << p3 << p4 << std::endl; } } + +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 diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 6a7f27bed4..78092fdc11 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -66,8 +66,10 @@ protected: } EInitState; // Base-class constructor should only be invoked by the DERIVED_TYPE - // constructor. - LLSingletonBase(); + // constructor, which passes the DERIVED_TYPE class name for logging + // purposes. Within LLSingletonBase::LLSingletonBase, of course the + // formula typeid(*this).name() produces "LLSingletonBase". + LLSingletonBase(const char* name); virtual ~LLSingletonBase(); // Every new LLSingleton should be added to/removed from the master list @@ -87,11 +89,15 @@ protected: // single C++ scope, else we'd use RAII to track it. But we do know that // LLSingletonBase's constructor definitely runs just before // LLSingleton's, which runs just before the specific subclass's. - void push_initializing(); + void push_initializing(const char*); // LLSingleton is, and must remain, the only caller to initSingleton(). // That being the case, we control exactly when it happens -- and we can // pop the stack immediately thereafter. void pop_initializing(); +private: + // logging + static void log_initializing(const char* verb, const char* name); +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); @@ -281,7 +287,9 @@ private: }; protected: - LLSingleton() + // Use typeid(DERIVED_TYPE) rather than typeid(*this) because, until our + // constructor completes, *this isn't yet a full-fledged DERIVED_TYPE. + LLSingleton(): LLSingletonBase(typeid(DERIVED_TYPE).name()) { // populate base-class function pointer with the static // deleteSingleton() function for this particular specialization -- cgit v1.2.3 From f931f6ef521527a929ceddf1354c7e2d85b35b63 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 3 Sep 2016 11:39:17 -0400 Subject: MAINT-5232: Add LLCoros::get_id() to identify the running coroutine. Change the module-static thread_specific_ptr to a function-static thread_specific_ptr so it will be initialized on demand -- since LLSingleton will need to rely on get_id(). Note that since LLCoros isa LLSingleton, we must take great care to avoid circularity. Introduce a private helper class LLCoros::Current to obtain and bind that thread_specific_ptr. Change all existing internal references from the static thread_specific_ptr to the new Current helper class. --- indra/llcommon/llcoros.cpp | 58 ++++++++++++++++++++++++++++------------------ indra/llcommon/llcoros.h | 24 ++++++++++++++++--- 2 files changed, 56 insertions(+), 26 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index d16bf0160b..e84fa6aea0 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -44,16 +44,25 @@ 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. -// 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. -boost::thread_specific_ptr -LLCoros::sCurrentCoro(LLCoros::no_cleanup); +LLCoros::Current::Current() +{ + // 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); + mCurrent = &sCurrent; +} //static LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) { - CoroData* current = sCurrentCoro.get(); + CoroData* current = Current(); if (! current) { LL_ERRS("LLCoros") << "Calling " << caller << " from non-coroutine context!" << LL_ENDL; @@ -79,20 +88,23 @@ bool LLCoros::get_consuming() return get_CoroData("get_consuming()").mConsuming; } -llcoro::Suspending::Suspending(): - mSuspended(LLCoros::sCurrentCoro.get()) +llcoro::Suspending::Suspending() { - // Revert mCurrentCoro to the value it had at the moment we last switched + 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. - LLCoros::sCurrentCoro.reset(mSuspended->mPrev); + current.reset(mSuspended->mPrev); } llcoro::Suspending::~Suspending() { + LLCoros::Current current; // Okay, we're back, update our mPrev - mSuspended->mPrev = LLCoros::sCurrentCoro.get(); - // and reinstate our sCurrentCoro. - LLCoros::sCurrentCoro.reset(mSuspended); + mSuspended->mPrev = current; + // and reinstate our Current. + current.reset(mSuspended); } LLCoros::LLCoros(): @@ -212,7 +224,7 @@ bool LLCoros::kill(const std::string& name) std::string LLCoros::getName() const { - CoroData* current = sCurrentCoro.get(); + CoroData* current = Current(); if (! current) { // not in a coroutine @@ -228,8 +240,8 @@ void LLCoros::setStackSize(S32 stacksize) } // Top-level wrapper around caller's coroutine callable. This function accepts -// the coroutine library's implicit coro::self& parameter and sets sCurrentSelf -// but does not pass it down to the caller's callable. +// 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) { // capture the 'self' param in CoroData @@ -237,8 +249,8 @@ void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& calla // run the code the caller actually wants in the coroutine callable(); // This cleanup isn't perfectly symmetrical with the way we initially set - // data->mPrev, but this is our last chance to reset mCurrentCoro. - sCurrentCoro.reset(data->mPrev); + // data->mPrev, but this is our last chance to reset Current. + Current().reset(data->mPrev); } /***************************************************************************** @@ -261,7 +273,7 @@ LLCoros::CoroData::CoroData(CoroData* prev, const std::string& name, mPrev(prev), mName(name), // Wrap the caller's callable in our toplevel() function so we can manage - // sCurrentCoro appropriately at startup and shutdown of each coroutine. + // 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), @@ -272,13 +284,13 @@ LLCoros::CoroData::CoroData(CoroData* prev, const std::string& name, std::string LLCoros::launch(const std::string& prefix, const callable_t& callable) { std::string name(generateDistinctName(prefix)); - // pass the current value of sCurrentCoro as previous context - CoroData* newCoro = new CoroData(sCurrentCoro.get(), name, - callable, mStackSize); + Current current; + // pass the current value of Current as previous context + CoroData* newCoro = new CoroData(current, name, callable, mStackSize); // Store it in our pointer map mCoros.insert(name, newCoro); // also set it as current - sCurrentCoro.reset(newCoro); + current.reset(newCoro); /* Run the coroutine until its first wait, then return here */ (newCoro->mCoro)(std::nothrow); return name; diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index 39316ed0e6..93e5ea8b9f 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -145,6 +146,10 @@ public: */ std::string getName() const; + /// Get an opaque, distinct token for the running coroutine (or main). + typedef void* id; + static id get_id() { return Current(); } + /// for delayed initialization void setStackSize(S32 stacksize); @@ -222,8 +227,21 @@ private: typedef boost::ptr_map CoroMap; CoroMap mCoros; - // identify the current coroutine's CoroData - static boost::thread_specific_ptr sCurrentCoro; + // 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* get() { return mCurrent->get(); } + void reset(LLCoros::CoroData* ptr) { mCurrent->reset(ptr); } + + private: + boost::thread_specific_ptr* mCurrent; + }; }; namespace llcoro @@ -231,7 +249,7 @@ namespace llcoro /// Instantiate one of these in a block surrounding any leaf point when /// control literally switches away from this coroutine. -class Suspending +class Suspending: boost::noncopyable { public: Suspending(); -- cgit v1.2.3 From 976f4b6252f30f7e64cba83ec43d98eb260a5261 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 3 Sep 2016 12:04:36 -0400 Subject: MAINT-5232: Break out LLCoros::get_id() into its own header file. We need LLSingleton machinery to be able to reference get_id() without also depending on all the rest of LLCoros -- since LLCoros isa LLSingleton. --- indra/llcommon/CMakeLists.txt | 2 ++ indra/llcommon/llcoro_get_id.cpp | 32 ++++++++++++++++++++++++++++++++ indra/llcommon/llcoro_get_id.h | 30 ++++++++++++++++++++++++++++++ indra/llcommon/llcoros.h | 6 ++---- 4 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 indra/llcommon/llcoro_get_id.cpp create mode 100644 indra/llcommon/llcoro_get_id.h (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index e1261ee17d..ff20de4f08 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -43,6 +43,7 @@ set(llcommon_SOURCE_FILES llcleanup.cpp llcommon.cpp llcommonutils.cpp + llcoro_get_id.cpp llcoros.cpp llcrc.cpp llcriticaldamp.cpp @@ -139,6 +140,7 @@ set(llcommon_HEADER_FILES llcleanup.h llcommon.h llcommonutils.h + llcoro_get_id.h llcoros.h llcrc.h llcriticaldamp.h diff --git a/indra/llcommon/llcoro_get_id.cpp b/indra/llcommon/llcoro_get_id.cpp new file mode 100644 index 0000000000..24ed1fe0c9 --- /dev/null +++ b/indra/llcommon/llcoro_get_id.cpp @@ -0,0 +1,32 @@ +/** + * @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 new file mode 100644 index 0000000000..4c1dca6f19 --- /dev/null +++ b/indra/llcommon/llcoro_get_id.h @@ -0,0 +1,30 @@ +/** + * @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.h b/indra/llcommon/llcoros.h index 93e5ea8b9f..5b6c6e5198 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -38,6 +38,7 @@ #include #include #include +#include "llcoro_get_id.h" // for friend declaration // forward-declare helper class namespace llcoro @@ -146,10 +147,6 @@ public: */ std::string getName() const; - /// Get an opaque, distinct token for the running coroutine (or main). - typedef void* id; - static id get_id() { return Current(); } - /// for delayed initialization void setStackSize(S32 stacksize); @@ -181,6 +178,7 @@ private: LLCoros(); friend class LLSingleton; friend class llcoro::Suspending; + friend llcoro::id llcoro::get_id(); std::string generateDistinctName(const std::string& prefix) const; bool cleanup(const LLSD&); struct CoroData; -- cgit v1.2.3 From a601c559e87d4f2d8b7a2b419b7e7b22cff37121 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Sep 2016 12:08:38 -0400 Subject: MAINT-5232: Ensure that llcoro::get_id() returns distinct values. Until now, the "main coroutine" (the initial context) of each thread left LLCoros::Current() NULL. The trouble with that is that llcoro::get_id() returns that CoroData* as an opaque token, and we want distinct values for every stack in the process. That would not be true if the "main coroutine" on thread A returned the same value (NULL) as the "main coroutine" on thread B, and so forth. Give each thread's "main coroutine" a dummy heap CoroData instance of its own. --- indra/llcommon/llcoros.cpp | 58 +++++++++++++++++++++++++++++++++++----------- indra/llcommon/llcoros.h | 1 + 2 files changed, 46 insertions(+), 13 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index e84fa6aea0..7ff5feac68 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -39,7 +39,12 @@ #include "llerror.h" #include "stringize.h" -// do nothing, when we need nothing done +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 @@ -56,6 +61,35 @@ LLCoros::Current::Current() // 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()) + { + // 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 + } + mCurrent = &sCurrent; } @@ -63,17 +97,21 @@ LLCoros::Current::Current() LLCoros::CoroData& LLCoros::get_CoroData(const std::string& caller) { CoroData* current = Current(); - if (! current) - { - LL_ERRS("LLCoros") << "Calling " << caller << " from non-coroutine context!" << LL_ENDL; - } + // With the dummy CoroData set in LLCoros::Current::Current(), this + // pointer should never be NULL. + llassert_always(current); return *current; } //static LLCoros::coro::self& LLCoros::get_self() { - return *get_CoroData("get_self()").mSelf; + CoroData& current = get_CoroData("get_self()"); + if (! current.mSelf) + { + LL_ERRS("LLCoros") << "Calling get_self() from non-coroutine context!" << LL_ENDL; + } + return *current.mSelf; } //static @@ -224,13 +262,7 @@ bool LLCoros::kill(const std::string& name) std::string LLCoros::getName() const { - CoroData* current = Current(); - if (! current) - { - // not in a coroutine - return ""; - } - return current->mName; + return Current()->mName; } void LLCoros::setStackSize(S32 stacksize) diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index 5b6c6e5198..0da7a3a6e4 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -234,6 +234,7 @@ private: 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); } -- cgit v1.2.3 From 90f424980a3aba06ac85cc23373795bc53a3fd87 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Sep 2016 21:07:38 -0400 Subject: MAINT-5232: Make LLSingleton's 'initializing' stack coro-specific. The stack we maintain of which LLSingletons are currently initializing only makes sense when associated with a particular C++ call stack. But each coroutine introduces another C++ call stack! Move the initializing stack from function-static storage to LLSingletonBase::MasterList. Make it a map keyed by llcoro::id. Each coro then has a stack of its own. This introduces more dependencies on the MasterList singleton, requiring additional LLSingleton_manage_master workarounds. --- indra/llcommon/llsingleton.cpp | 54 +++++++++++++++++++++++++++++------------ indra/llcommon/llsingleton.h | 55 +++++++++++++++++++++++++++++++++--------- 2 files changed, 82 insertions(+), 27 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 479244400d..3f65820f2e 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -30,7 +30,9 @@ #include "llerror.h" #include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" +#include "llcoro_get_id.h" #include +#include #include #include // std::cerr in dire emergency #include @@ -61,13 +63,43 @@ private: public: // No need to make this private with accessors; nobody outside this source // file can see it. - LLSingletonBase::list_t mList; + + // 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; + + // 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' + // 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; + + // non-static method, cf. LLSingletonBase::get_initializing() + 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()]; + } }; //static LLSingletonBase::list_t& LLSingletonBase::get_master() { - return LLSingletonBase::MasterList::instance().mList; + return LLSingletonBase::MasterList::instance().mMaster; } void LLSingletonBase::add_master() @@ -88,23 +120,16 @@ void LLSingletonBase::remove_master() get_master().remove(this); } -// Wrapping our initializing list in a static method ensures that it will be -// constructed on demand. This list doesn't also need to be in an LLSingleton -// because (a) it should be empty by program shutdown and (b) none of our -// destructors reference it. //static LLSingletonBase::list_t& LLSingletonBase::get_initializing() { - static list_t sList; - return sList; + return LLSingletonBase::MasterList::instance().get_initializing_(); } -LLSingletonBase::LLSingletonBase(const char* name): - mCleaned(false), - mDeleteSingleton(NULL) +//static +LLSingletonBase::list_t& LLSingletonBase::get_initializing_from(MasterList* master) { - // Make this the currently-initializing LLSingleton. - push_initializing(name); + return master->get_initializing_();; } LLSingletonBase::~LLSingletonBase() {} @@ -159,13 +184,12 @@ void LLSingletonBase::log_initializing(const char* verb, const char* name) } } -void LLSingletonBase::capture_dependency(EInitState initState) +void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initState) { // 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 // empty(). - list_t& initializing(get_initializing()); if (! initializing.empty()) { // getInstance() is being called by some other LLSingleton. But -- is diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 78092fdc11..92fba4c1a8 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -46,6 +46,7 @@ private: // This, on the other hand, is a stack whose top indicates the LLSingleton // currently being initialized. static list_t& get_initializing(); + static list_t& get_initializing_from(MasterList*); // Produce a vector of master list, in dependency order. typedef std::vector vec_t; static vec_t dep_sort(); @@ -65,11 +66,19 @@ protected: DELETED } EInitState; + // Define tag to pass to our template constructor. You can't explicitly + // invoke a template constructor with ordinary template syntax: + // http://stackoverflow.com/a/3960925/5533635 + template + struct tag + { + typedef T type; + }; + // Base-class constructor should only be invoked by the DERIVED_TYPE - // constructor, which passes the DERIVED_TYPE class name for logging - // purposes. Within LLSingletonBase::LLSingletonBase, of course the - // formula typeid(*this).name() produces "LLSingletonBase". - LLSingletonBase(const char* name); + // constructor, which passes tag for various purposes. + template + LLSingletonBase(tag); virtual ~LLSingletonBase(); // Every new LLSingleton should be added to/removed from the master list @@ -100,7 +109,7 @@ private: 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(list_t& initializing, EInitState); // delegate LL_ERRS() logging to llsingleton.cpp static void logerrs(const char* p1, const char* p2="", @@ -171,12 +180,15 @@ void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount*); void intrusive_ptr_release(LLSingletonBase::MasterRefcount*); // Most of the time, we want LLSingleton_manage_master() to forward its -// methods to LLSingletonBase::add_master() and remove_master(). +// methods to real LLSingletonBase methods. template struct LLSingleton_manage_master { void add(LLSingletonBase* sb) { sb->add_master(); } void remove(LLSingletonBase* sb) { sb->remove_master(); } + void push_initializing(LLSingletonBase* sb) { sb->push_initializing(typeid(T).name()); } + void pop_initializing (LLSingletonBase* sb) { sb->pop_initializing(); } + LLSingletonBase::list_t& get_initializing(T*) { return LLSingletonBase::get_initializing(); } }; // But for the specific case of LLSingletonBase::MasterList, don't. @@ -185,8 +197,24 @@ struct LLSingleton_manage_master { void add(LLSingletonBase*) {} void remove(LLSingletonBase*) {} + void push_initializing(LLSingletonBase*) {} + void pop_initializing (LLSingletonBase*) {} + LLSingletonBase::list_t& get_initializing(LLSingletonBase::MasterList* instance) + { + return LLSingletonBase::get_initializing_from(instance); + } }; +// Now we can implement LLSingletonBase's template constructor. +template +LLSingletonBase::LLSingletonBase(tag): + mCleaned(false), + mDeleteSingleton(NULL) +{ + // Make this the currently-initializing LLSingleton. + LLSingleton_manage_master().push_initializing(this); +} + /** * LLSingleton implements the getInstance() method part of the Singleton * pattern. It can't make the derived class constructors protected, though, so @@ -287,9 +315,10 @@ private: }; protected: - // Use typeid(DERIVED_TYPE) rather than typeid(*this) because, until our - // constructor completes, *this isn't yet a full-fledged DERIVED_TYPE. - LLSingleton(): LLSingletonBase(typeid(DERIVED_TYPE).name()) + // Pass DERIVED_TYPE explicitly to LLSingletonBase's constructor because, + // until our subclass constructor completes, *this isn't yet a + // full-fledged DERIVED_TYPE. + LLSingleton(): LLSingletonBase(LLSingletonBase::tag()) { // populate base-class function pointer with the static // deleteSingleton() function for this particular specialization @@ -363,7 +392,7 @@ public: // breaking cyclic dependencies sData.mInstance->initSingleton(); // pop this off stack of initializing singletons - sData.mInstance->pop_initializing(); + LLSingleton_manage_master().pop_initializing(sData.mInstance); break; case INITIALIZED: @@ -378,7 +407,7 @@ public: sData.mInitState = INITIALIZED; sData.mInstance->initSingleton(); // pop this off stack of initializing singletons - sData.mInstance->pop_initializing(); + LLSingleton_manage_master().pop_initializing(sData.mInstance); break; } @@ -387,7 +416,9 @@ public: // an LLSingleton that directly depends on DERIVED_TYPE. If this call // came from another LLSingleton, rather than from vanilla application // code, record the dependency. - sData.mInstance->capture_dependency(sData.mInitState); + sData.mInstance->capture_dependency( + LLSingleton_manage_master().get_initializing(sData.mInstance), + sData.mInitState); return sData.mInstance; } -- cgit v1.2.3 From 1cadeb40df15c1eaef3410064f9a2b8e4489082d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 6 Sep 2016 21:25:57 -0400 Subject: MAINT-5232: Prevent runaway LLSingletonBase::MasterList growth. Until we reimplement LLCoros on Boost.Fiber, we must hand-implement coroutine-local data. That presently takes the form of a map keyed on llcoro::id, whose values are the stacks of currently-initializing LLSingleton instances. But since the viewer launches an open-ended number of coroutines, we could end up with an open-ended number of map entries unless we intentionally prune the map. So every time we pop the stack to empty, remove that map entry. This could result in thrashing, a given coroutine's 'initializing' stack being created and deleted for almost every LLSingleton instantiated by that coroutine -- but the number of different LLSingletons is necessarily static, and the lifespan of each is the entire rest of the process. Even a couple dozen LLSingletons won't thrash that badly. --- indra/llcommon/llsingleton.cpp | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 3f65820f2e..24ccc8ddb4 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -94,6 +94,15 @@ public: // here. It returns a reference to the selected mapped_type instance. return mInitializing[llcoro::get_id()]; } + + void cleanup_initializing_() + { + InitializingMap::iterator found = mInitializing.find(llcoro::get_id()); + if (found != mInitializing.end()) + { + mInitializing.erase(found); + } + } }; //static @@ -129,7 +138,7 @@ LLSingletonBase::list_t& LLSingletonBase::get_initializing() //static LLSingletonBase::list_t& LLSingletonBase::get_initializing_from(MasterList* master) { - return master->get_initializing_();; + return master->get_initializing_(); } LLSingletonBase::~LLSingletonBase() {} @@ -156,6 +165,17 @@ void LLSingletonBase::pop_initializing() // and pop it list.pop_back(); + // The viewer launches an open-ended number of coroutines. While we don't + // expect most of them to initialize LLSingleton instances, our present + // get_initializing() logic could lead to an open-ended number of map + // entries. So every time we pop the stack back to empty, delete the entry + // entirely. + if (list.empty()) + { + MasterList::instance().cleanup_initializing_(); + } + + // Now validate the newly-popped LLSingleton. if (back != this) { logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", -- cgit v1.2.3 From d2c3c2f9fe197b1856e9a8ed37aeb56b77e2ff07 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 15 Sep 2016 20:18:12 -0400 Subject: MAINT-5232: Normalize LLSingleton subclasses. A shocking number of LLSingleton subclasses had public constructors -- and in several instances, were being explicitly instantiated independently of the LLSingleton machinery. This breaks the new LLSingleton dependency-tracking machinery. It seems only fair that if you say you want an LLSingleton, there should only be ONE INSTANCE! Introduce LLSINGLETON() and LLSINGLETON_EMPTY_CTOR() macros. These handle the friend class LLSingleton; and explicitly declare a private nullary constructor. To try to enforce the LLSINGLETON() convention, introduce a new pure virtual LLSingleton method you_must_use_LLSINGLETON_macro() which is, as you might suspect, defined by the macro. If you declare an LLSingleton subclass without using LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() in the class body, you can't instantiate the subclass for lack of a you_must_use_LLSINGLETON_macro() implementation -- which will hopefully remind the coder. Trawl through ALL LLSingleton subclass definitions, sprinkling in LLSINGLETON() or LLSINGLETON_EMPTY_CTOR() as appropriate. Remove all explicit constructor declarations, public or private, along with relevant 'friend class LLSingleton' declarations. Where destructors are declared, move them into private section as well. Where the constructor was inline but nontrivial, move out of class body. Fix several LLSingleton abuses revealed by making ctors/dtors private: LLGlobalEconomy was both an LLSingleton and the base class for LLRegionEconomy, a non-LLSingleton. (Therefore every LLRegionEconomy instance contained another instance of the LLGlobalEconomy "singleton.") Extract LLBaseEconomy; LLGlobalEconomy is now a trivial subclass of that. LLRegionEconomy, as you might suspect, now derives from LLBaseEconomy. LLToolGrab, an LLSingleton, was also explicitly instantiated by LLToolCompGun's constructor. Extract LLToolGrabBase, explicitly instantiated, with trivial subclass LLToolGrab, the LLSingleton instance. (WARNING: LLToolGrabBase methods have an unnerving tendency to go after LLToolGrab::getInstance(). I DO NOT KNOW what should be the relationship between the instance in LLToolCompGun and the LLToolGrab singleton instance.) LLGridManager declared a variant constructor accepting (const std::string&), with the comment: // initialize with an explicity grid file for testing. As there is no evidence of this being called from anywhere, delete it. LLChicletBar's constructor accepted an optional (const LLSD&). As the LLSD parameter wasn't used, and as there is no evidence of it being passed from anywhere, delete the parameter. LLViewerWindow::shutdownViews() was checking LLNavigationBar:: instanceExists(), then deleting its getInstance() pointer -- leaving a dangling LLSingleton instance pointer, a land mine if any subsequent code should attempt to reference it. Use deleteSingleton() instead. ~LLAppViewer() was calling LLViewerEventRecorder::instance() and then explicitly calling ~LLViewerEventRecorder() on that instance -- leaving the LLSingleton instance pointer pointing to an allocated-but-destroyed instance. Use deleteSingleton() instead. --- indra/llcommon/llassettype.cpp | 3 +- indra/llcommon/llcoros.h | 3 +- indra/llcommon/llerror.cpp | 9 ++-- indra/llcommon/llevents.h | 3 +- indra/llcommon/llinitdestroyclass.h | 8 +--- indra/llcommon/llpounceable.h | 3 +- indra/llcommon/llregistry.h | 5 ++- indra/llcommon/llsingleton.cpp | 3 +- indra/llcommon/llsingleton.h | 68 ++++++++++++++++++++++++++++++- indra/llcommon/tests/llsingleton_test.cpp | 31 +++++++------- 10 files changed, 98 insertions(+), 38 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llassettype.cpp b/indra/llcommon/llassettype.cpp index 5ae2df3994..4304db36be 100644 --- a/indra/llcommon/llassettype.cpp +++ b/indra/llcommon/llassettype.cpp @@ -63,8 +63,7 @@ struct AssetEntry : public LLDictionaryEntry class LLAssetDictionary : public LLSingleton, public LLDictionary { -public: - LLAssetDictionary(); + LLSINGLETON(LLAssetDictionary); }; LLAssetDictionary::LLAssetDictionary() diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index 0da7a3a6e4..bbe2d22af4 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -85,6 +85,7 @@ class Suspending; */ class LL_COMMON_API LLCoros: public LLSingleton { + LLSINGLETON(LLCoros); public: /// Canonical boost::dcoroutines::coroutine signature we use typedef boost::dcoroutines::coroutine coro; @@ -175,8 +176,6 @@ public: class Future; private: - LLCoros(); - friend class LLSingleton; friend class llcoro::Suspending; friend llcoro::id llcoro::get_id(); std::string generateDistinctName(const std::string& prefix) const; diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index a34b50f816..2ef748e3e4 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -374,9 +374,8 @@ namespace class Globals : public LLSingleton { + LLSINGLETON(Globals); public: - Globals(); - std::ostringstream messageStream; bool messageStreamInUse; @@ -449,9 +448,8 @@ namespace LLError class Settings : public LLSingleton { + LLSINGLETON(Settings); public: - Settings(); - SettingsConfigPtr getSettingsConfig(); void reset(); @@ -486,8 +484,7 @@ namespace LLError mRecorders.clear(); } - Settings::Settings() - : LLSingleton(), + Settings::Settings(): mSettingsConfig(new SettingsConfig()) { } diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index ba4fcd766e..6c8b66f596 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -209,7 +209,7 @@ class LLEventPump; */ class LL_COMMON_API LLEventPumps: public LLSingleton { - friend class LLSingleton; + LLSINGLETON(LLEventPumps); public: /** * Find or create an LLEventPump instance with a specific name. We return @@ -252,7 +252,6 @@ private: void unregister(const LLEventPump&); private: - LLEventPumps(); ~LLEventPumps(); testable: diff --git a/indra/llcommon/llinitdestroyclass.h b/indra/llcommon/llinitdestroyclass.h index 9c66211475..5f979614fe 100644 --- a/indra/llcommon/llinitdestroyclass.h +++ b/indra/llcommon/llinitdestroyclass.h @@ -78,9 +78,7 @@ class LLInitClassList : public LLCallbackRegistry, public LLSingleton { - friend class LLSingleton; -private: - LLInitClassList() {} + LLSINGLETON_EMPTY_CTOR(LLInitClassList); }; /** @@ -94,9 +92,7 @@ class LLDestroyClassList : public LLCallbackRegistry, public LLSingleton { - friend class LLSingleton; -private: - LLDestroyClassList() {} + LLSINGLETON_EMPTY_CTOR(LLDestroyClassList); }; /** diff --git a/indra/llcommon/llpounceable.h b/indra/llcommon/llpounceable.h index 77b711bdc6..0421ce966a 100644 --- a/indra/llcommon/llpounceable.h +++ b/indra/llcommon/llpounceable.h @@ -76,7 +76,8 @@ template class LLPounceableQueueSingleton: public LLSingleton > { -private: + LLSINGLETON_EMPTY_CTOR(LLPounceableQueueSingleton); + typedef LLPounceableTraits traits; typedef typename traits::owner_ptr owner_ptr; typedef typename traits::signal_t signal_t; diff --git a/indra/llcommon/llregistry.h b/indra/llcommon/llregistry.h index fde729f8f9..750fe9fdc8 100644 --- a/indra/llcommon/llregistry.h +++ b/indra/llcommon/llregistry.h @@ -247,7 +247,10 @@ class LLRegistrySingleton : public LLRegistry, public LLSingleton { - friend class LLSingleton; + // This LLRegistrySingleton doesn't use LLSINGLETON(LLRegistrySingleton) + // because the concrete class is actually DERIVED_TYPE, not + // LLRegistrySingleton. So each concrete subclass needs + // LLSINGLETON(whatever) -- not this intermediate base class. public: typedef LLRegistry registry_t; typedef const KEY& ref_const_key_t; diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 24ccc8ddb4..9025e53bb2 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -57,8 +57,7 @@ bool oktolog(); class LLSingletonBase::MasterList: public LLSingleton { -private: - friend class LLSingleton; + LLSINGLETON_EMPTY_CTOR(MasterList); public: // No need to make this private with accessors; nobody outside this source diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 92fba4c1a8..1b915dfd6e 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -223,7 +223,13 @@ LLSingletonBase::LLSingletonBase(tag): * Derive your class from LLSingleton, passing your subclass name as * LLSingleton's template parameter, like so: * - * class Foo: public LLSingleton{}; + * class Foo: public LLSingleton + * { + * // use this macro at start of every LLSingleton subclass + * LLSINGLETON(Foo); + * public: + * // ... + * }; * * Foo& instance = Foo::instance(); * @@ -279,6 +285,16 @@ private: return new DERIVED_TYPE(); } + // We know of no way to instruct the compiler that every subclass + // constructor MUST be private. However, we can make the LLSINGLETON() + // macro both declare a private constructor and provide the required + // friend declaration. How can we ensure that every subclass uses + // LLSINGLETON()? By making that macro provide a definition for this pure + // virtual method. If you get "can't instantiate class due to missing pure + // virtual method" for this method, then add LLSINGLETON(yourclass) in the + // subclass body. + virtual void you_must_use_LLSINGLETON_macro() = 0; + // stores pointer to singleton instance struct SingletonLifetimeManager { @@ -450,4 +466,54 @@ private: template typename LLSingleton::SingletonData LLSingleton::sData; +/** + * Use LLSINGLETON(Foo); at the start of an LLSingleton subclass body + * when you want to declare an out-of-line constructor: + * + * @code + * class Foo: public LLSingleton + * { + * // use this macro at start of every LLSingleton subclass + * LLSINGLETON(Foo); + * public: + * // ... + * }; + * // ... + * [inline] + * Foo::Foo() { ... } + * @endcode + * + * Unfortunately, this mechanism does not permit you to define even a simple + * (but nontrivial) constructor within the class body. If it's literally + * trivial, use LLSINGLETON_EMPTY_CTOR(); if not, use LLSINGLETON() and define + * the constructor outside the class body. If you must define it in a header + * file, use 'inline' (unless it's a template class) to avoid duplicate-symbol + * errors at link time. + */ +#define LLSINGLETON(DERIVED_CLASS) \ +private: \ + /* implement LLSingleton pure virtual method whose sole purpose */ \ + /* is to remind people to use this macro */ \ + virtual void you_must_use_LLSINGLETON_macro() {} \ + friend class LLSingleton; \ + DERIVED_CLASS() + +/** + * Use LLSINGLETON_EMPTY_CTOR(Foo); at the start of an LLSingleton + * subclass body when the constructor is trivial: + * + * @code + * class Foo: public LLSingleton + * { + * // use this macro at start of every LLSingleton subclass + * LLSINGLETON_EMPTY_CTOR(Foo); + * public: + * // ... + * }; + * @endcode + */ +#define LLSINGLETON_EMPTY_CTOR(DERIVED_CLASS) \ + /* LLSINGLETON() is carefully implemented to permit exactly this */ \ + LLSINGLETON(DERIVED_CLASS) {} + #endif diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp index a05f650f25..56886bc73f 100644 --- a/indra/llcommon/tests/llsingleton_test.cpp +++ b/indra/llcommon/tests/llsingleton_test.cpp @@ -34,21 +34,22 @@ // Capture execution sequence by appending to log string. std::string sLog; -#define DECLARE_CLASS(CLS) \ -struct CLS: public LLSingleton \ -{ \ - static enum dep_flag { \ - DEP_NONE, /* no dependency */ \ - DEP_CTOR, /* dependency in ctor */ \ +#define DECLARE_CLASS(CLS) \ +struct CLS: public LLSingleton \ +{ \ + LLSINGLETON(CLS); \ + ~CLS(); \ +public: \ + static enum dep_flag { \ + DEP_NONE, /* no dependency */ \ + DEP_CTOR, /* dependency in ctor */ \ DEP_INIT /* dependency in initSingleton */ \ - } sDepFlag; \ - \ - CLS(); \ - void initSingleton(); \ - void cleanupSingleton(); \ - ~CLS(); \ -}; \ - \ + } sDepFlag; \ + \ + void initSingleton(); \ + void cleanupSingleton(); \ +}; \ + \ CLS::dep_flag CLS::sDepFlag = DEP_NONE DECLARE_CLASS(A); @@ -93,7 +94,7 @@ namespace tut // We need a class created with the LLSingleton template to test with. class LLSingletonTest: public LLSingleton { - + LLSINGLETON_EMPTY_CTOR(LLSingletonTest); }; }; -- cgit v1.2.3 From d0249fb7c099d30015f22e49cc5421f3c80e05b7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 17 Sep 2016 20:54:50 -0400 Subject: MAINT-5232: Eliminate pointless string search for "class " prefix. The Visual C++ runtime produces typeid(MyClass).name() as "class MyClass". It's prudent to check for the presence of that prefix before stripping off the first six characters, but if the first comparison should ever fail, find() would continue searching the rest of the string for "class " -- a search guaranteed to fail. Use compare() instead. --- indra/llcommon/llerror.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2ef748e3e4..245118b00f 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -270,15 +270,15 @@ namespace LLError // DevStudio: type_info::name() includes the text "class " at the start static const std::string class_prefix = "class "; - std::string name = mangled; - std::string::size_type p = name.find(class_prefix); - if (p == std::string::npos) + if (0 != name.compare(0, class_prefix.length(), class_prefix)) { + LL_DEBUGS() << "Did not see '" << class_prefix << "' prefix on '" + << name << "'" << LL_ENDL; return name; } - return name.substr(p + class_prefix.size()); + return name.substr(class_prefix.length()); #else return mangled; -- cgit v1.2.3 From b941de80dbe15a20a0eaea795d1282cf36306c30 Mon Sep 17 00:00:00 2001 From: Mnikolenko Productengine Date: Fri, 30 Sep 2016 11:21:22 +0300 Subject: MAINT-6783 Xml parser warnings should show the file name --- indra/llcommon/llinitparam.cpp | 7 ++++++- indra/llcommon/llinitparam.h | 1 + indra/llcommon/llsdparam.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llinitparam.cpp b/indra/llcommon/llinitparam.cpp index aa2f4eb289..1d104cf55d 100644 --- a/indra/llcommon/llinitparam.cpp +++ b/indra/llcommon/llinitparam.cpp @@ -193,7 +193,12 @@ namespace LLInitParam { if (!silent) { - p.parserWarning(llformat("Failed to parse parameter \"%s\"", p.getCurrentElementName().c_str())); + std::string file_name = p.getCurrentFileName(); + if(!file_name.empty()) + { + file_name = "in file: " + file_name; + } + p.parserWarning(llformat("Failed to parse parameter \"%s\" %s", p.getCurrentElementName().c_str(), file_name.c_str())); } return false; } diff --git a/indra/llcommon/llinitparam.h b/indra/llcommon/llinitparam.h index c65b05f610..f1f4226c40 100644 --- a/indra/llcommon/llinitparam.h +++ b/indra/llcommon/llinitparam.h @@ -551,6 +551,7 @@ namespace LLInitParam } virtual std::string getCurrentElementName() = 0; + virtual std::string getCurrentFileName() = 0; virtual void parserWarning(const std::string& message); virtual void parserError(const std::string& message); void setParseSilently(bool silent) { mParseSilently = silent; } diff --git a/indra/llcommon/llsdparam.h b/indra/llcommon/llsdparam.h index 09f1bdf1e3..93910b70ae 100644 --- a/indra/llcommon/llsdparam.h +++ b/indra/llcommon/llsdparam.h @@ -66,6 +66,7 @@ public: } /*virtual*/ std::string getCurrentElementName(); + /*virtual*/ std::string getCurrentFileName(){ return LLStringUtil::null; } private: void writeSDImpl(LLSD& sd, -- cgit v1.2.3 From 763509589dc9933d04a26e8109f90591eef1d012 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 Oct 2016 23:01:48 -0400 Subject: MAINT-5232: Add LLHeteroMap to contain objects of unrelated classes. --- indra/llcommon/CMakeLists.txt | 3 + indra/llcommon/llheteromap.cpp | 32 ++++++ indra/llcommon/llheteromap.h | 95 ++++++++++++++++++ indra/llcommon/tests/llheteromap_test.cpp | 158 ++++++++++++++++++++++++++++++ 4 files changed, 288 insertions(+) create mode 100644 indra/llcommon/llheteromap.cpp create mode 100644 indra/llcommon/llheteromap.h create mode 100644 indra/llcommon/tests/llheteromap_test.cpp (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 99514fdbf7..d9ab7c1b38 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -68,6 +68,7 @@ set(llcommon_SOURCE_FILES llformat.cpp llframetimer.cpp llheartbeat.cpp + llheteromap.cpp llinitparam.cpp llinitdestroyclass.cpp llinstancetracker.cpp @@ -173,6 +174,7 @@ set(llcommon_HEADER_FILES llhandle.h llhash.h llheartbeat.h + llheteromap.h llindexedvector.h llinitdestroyclass.h llinitparam.h @@ -337,6 +339,7 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llstreamqueue "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llheteromap "" "${test_libs}") ## llexception_test.cpp isn't a regression test, and doesn't need to be run ## every build. It's to help a developer make implementation choices about diff --git a/indra/llcommon/llheteromap.cpp b/indra/llcommon/llheteromap.cpp new file mode 100644 index 0000000000..7c19196e0c --- /dev/null +++ b/indra/llcommon/llheteromap.cpp @@ -0,0 +1,32 @@ +/** + * @file llheteromap.cpp + * @author Nat Goodspeed + * @date 2016-10-12 + * @brief Implementation for llheteromap. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llheteromap.h" +// STL headers +// std headers +// external library headers +// other Linden headers + +LLHeteroMap::~LLHeteroMap() +{ + // For each entry in our map, we must call its deleter, which is the only + // record we have of its original type. + for (TypeMap::iterator mi(mMap.begin()), me(mMap.end()); mi != me; ++mi) + { + // mi->second is the std::pair; mi->second.first is the void*; + // mi->second.second points to the deleter function + (mi->second.second)(mi->second.first); + mi->second.first = NULL; + } +} diff --git a/indra/llcommon/llheteromap.h b/indra/llcommon/llheteromap.h new file mode 100644 index 0000000000..9d6f303d08 --- /dev/null +++ b/indra/llcommon/llheteromap.h @@ -0,0 +1,95 @@ +/** + * @file llheteromap.h + * @author Nat Goodspeed + * @date 2016-10-12 + * @brief Map capable of storing objects of diverse types, looked up by type. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LLHETEROMAP_H) +#define LL_LLHETEROMAP_H + +#include +#include // std::pair +#include + +/** + * LLHeteroMap addresses an odd requirement. Usually when you want to put + * objects of different classes into a runtime collection of any kind, you + * derive them all from a common base class and store pointers to that common + * base class. + * + * LLInitParam::BaseBlock uses disturbing raw-pointer arithmetic to find data + * members in its subclasses. It seems that no BaseBlock subclass can be + * stored in a polymorphic class of any kind: the presence of a vtbl pointer + * in the layout silently throws off the reinterpret_cast arithmetic. Bad + * Things result. (Many thanks to Nicky D for this analysis!) + * + * LLHeteroMap collects objects WITHOUT a common base class, retrieves them by + * object type and destroys them when the LLHeteroMap is destroyed. + */ +class LLHeteroMap +{ +public: + ~LLHeteroMap(); + + /// find or create + template + T& obtain() + { + // Look up map entry by typeid(T). We don't simply use mMap[typeid(T)] + // because that requires default-constructing T on every lookup. For + // some kinds of T, that could be expensive. + TypeMap::iterator found = mMap.find(&typeid(T)); + if (found == mMap.end()) + { + // Didn't find typeid(T). Create an entry. Because we're storing + // only a void* in the map, discarding type information, make sure + // we capture that type information in our deleter. + void* ptr = new T(); + void (*dlfn)(void*) = &deleter; + std::pair inserted = + mMap.insert(TypeMap::value_type(&typeid(T), + TypeMap::mapped_type(ptr, dlfn))); + // Okay, now that we have an entry, claim we found it. + found = inserted.first; + } + // found->second is the std::pair; second.first is the void* + // pointer to the object in question. Cast it to correct type and + // dereference it. + return *(static_cast(found->second.first)); + } + +private: + template + static + void deleter(void* p) + { + delete static_cast(p); + } + + // Comparing two std::type_info* values is tricky, because the standard + // does not guarantee that there will be only one type_info instance for a + // given type. In other words, &typeid(A) in one part of the program may + // not always equal &typeid(A) in some other part. Use special comparator. + struct type_info_ptr_comp + { + bool operator()(const std::type_info* lhs, const std::type_info* rhs) + { + return lhs->before(*rhs); + } + }; + + // What we actually store is a map from std::type_info (permitting lookup + // by object type) to a void* pointer to the object PLUS its deleter. + typedef std::map< + const std::type_info*, std::pair, + type_info_ptr_comp> + TypeMap; + TypeMap mMap; +}; + +#endif /* ! defined(LL_LLHETEROMAP_H) */ diff --git a/indra/llcommon/tests/llheteromap_test.cpp b/indra/llcommon/tests/llheteromap_test.cpp new file mode 100644 index 0000000000..4f38933e50 --- /dev/null +++ b/indra/llcommon/tests/llheteromap_test.cpp @@ -0,0 +1,158 @@ +/** + * @file llheteromap_test.cpp + * @author Nat Goodspeed + * @date 2016-10-12 + * @brief Test for llheteromap. + * + * $LicenseInfo:firstyear=2016&license=viewerlgpl$ + * Copyright (c) 2016, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "llheteromap.h" +// STL headers +#include +// std headers +// external library headers +// other Linden headers +#include "../test/lltut.h" + +static std::string clog; +static std::set dlog; + +std::ostream& operator<<(std::ostream& out, const std::set& strset) +{ + out << '{'; + const char* delim = ""; + for (std::set::const_iterator si(strset.begin()), se(strset.end()); + si != se; ++si) + { + out << delim << '"' << *si << '"'; + delim = ", "; + } + out << '}'; + return out; +} + +struct Chalk +{ + int dummy; + std::string name; + + Chalk(): + dummy(0) + { + clog.append("a"); + } + + ~Chalk() + { + dlog.insert("a"); + } + +private: + Chalk(const Chalk&); // no implementation +}; + +struct Cheese +{ + std::string name; + + Cheese() + { + clog.append("e"); + } + + ~Cheese() + { + dlog.insert("e"); + } + +private: + Cheese(const Cheese&); // no implementation +}; + +struct Chowdah +{ + char displace[17]; + std::string name; + + Chowdah() + { + displace[0] = '\0'; + clog.append("o"); + } + + ~Chowdah() + { + dlog.insert("o"); + } + +private: + Chowdah(const Chowdah&); // no implementation +}; + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct llheteromap_data + { + llheteromap_data() + { + clog.erase(); + dlog.clear(); + } + }; + typedef test_group llheteromap_group; + typedef llheteromap_group::object object; + llheteromap_group llheteromapgrp("llheteromap"); + + template<> template<> + void object::test<1>() + { + set_test_name("create, get, delete"); + + { + LLHeteroMap map; + + { + // create each instance + Chalk& chalk = map.obtain(); + chalk.name = "Chalk"; + + Cheese& cheese = map.obtain(); + cheese.name = "Cheese"; + + Chowdah& chowdah = map.obtain(); + chowdah.name = "Chowdah"; + } // refs go out of scope + + { + // verify each instance + Chalk& chalk = map.obtain(); + ensure_equals(chalk.name, "Chalk"); + + Cheese& cheese = map.obtain(); + ensure_equals(cheese.name, "Cheese"); + + Chowdah& chowdah = map.obtain(); + ensure_equals(chowdah.name, "Chowdah"); + } + } // destroy map + + // Chalk, Cheese and Chowdah should have been created in specific order + ensure_equals(clog, "aeo"); + + // We don't care what order they're destroyed in, as long as each is + // appropriately destroyed. + std::set dtorset; + for (const char* cp = "aeo"; *cp; ++cp) + dtorset.insert(std::string(1, *cp)); + ensure_equals(dlog, dtorset); + } +} // namespace tut -- cgit v1.2.3 From 23d50f53cb7beed12a1ff15f32b5c85e2b9e6b4d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 13 Oct 2016 07:11:18 -0400 Subject: MAINT-5232: Ensure custom operator<<() overload is visible to TUT. --- indra/llcommon/tests/llheteromap_test.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/tests/llheteromap_test.cpp b/indra/llcommon/tests/llheteromap_test.cpp index 4f38933e50..686bffb878 100644 --- a/indra/llcommon/tests/llheteromap_test.cpp +++ b/indra/llcommon/tests/llheteromap_test.cpp @@ -17,12 +17,16 @@ #include // std headers // external library headers + +// (pacify clang) +std::ostream& operator<<(std::ostream& out, const std::set& strset); // other Linden headers #include "../test/lltut.h" static std::string clog; static std::set dlog; +// want to be able to use ensure_equals() on a set std::ostream& operator<<(std::ostream& out, const std::set& strset) { out << '{'; @@ -37,6 +41,7 @@ std::ostream& operator<<(std::ostream& out, const std::set& strset) return out; } +// unrelated test classes struct Chalk { int dummy; -- cgit v1.2.3 From 759b4f14d8c9160da928c79dde4bdc95f8fa0021 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Thu, 13 Oct 2016 22:49:06 +0300 Subject: MAINT-6828 Removed unnessesary spam in logs --- indra/llcommon/llfile.cpp | 4 ++-- indra/llcommon/llfile.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llfile.cpp b/indra/llcommon/llfile.cpp index 873a7bce25..7b559861bb 100644 --- a/indra/llcommon/llfile.cpp +++ b/indra/llcommon/llfile.cpp @@ -239,7 +239,7 @@ int LLFile::close(LLFILE * file) } -int LLFile::remove(const std::string& filename) +int LLFile::remove(const std::string& filename, int supress_error) { #if LL_WINDOWS std::string utf8filename = filename; @@ -248,7 +248,7 @@ int LLFile::remove(const std::string& filename) #else int rc = ::remove(filename.c_str()); #endif - return warnif("remove", filename, rc); + return warnif("remove", filename, rc, supress_error); } int LLFile::rename(const std::string& filename, const std::string& newname) diff --git a/indra/llcommon/llfile.h b/indra/llcommon/llfile.h index 3e25228aeb..d8f84daf2b 100644 --- a/indra/llcommon/llfile.h +++ b/indra/llcommon/llfile.h @@ -72,7 +72,7 @@ public: static int mkdir(const std::string& filename, int perms = 0700); static int rmdir(const std::string& filename); - static int remove(const std::string& filename); + static int remove(const std::string& filename, int supress_error = 0); static int rename(const std::string& filename,const std::string& newname); static bool copy(const std::string from, const std::string to); -- cgit v1.2.3 From 26e73e2f5822bde8ece0c01ba6445bbe0f042180 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Fri, 28 Oct 2016 17:29:30 +0300 Subject: MAINT-6825 Fixing bad_alloc crash --- indra/llcommon/llsdserialize.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index d49ff0feb5..81ba8631c6 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -1200,6 +1200,7 @@ bool LLSDBinaryParser::parseString( read(istr, (char*)&value_nbo, sizeof(U32)); /*Flawfinder: ignore*/ S32 size = (S32)ntohl(value_nbo); if(mCheckLimits && (size > mMaxBytesLeft)) return false; + if(size < 0) return false; std::vector buf; if(size) { -- cgit v1.2.3 From 86449a0ac4cb1432a55c17bfabe83c4c42c096a8 Mon Sep 17 00:00:00 2001 From: "Brad Payne (Vir Linden)" Date: Tue, 14 Feb 2017 09:31:08 -0500 Subject: SL-409 - debug setting to enable/disable use of ViewerAsset cap by asset type. Temporary construction until UDP path goes away. --- indra/llcommon/llassettype.cpp | 93 +++++++++++++++++++++++++++++++++++++++++- indra/llcommon/llassettype.h | 5 +++ 2 files changed, 97 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llassettype.cpp b/indra/llcommon/llassettype.cpp index 4304db36be..6ecc2ec740 100644 --- a/indra/llcommon/llassettype.cpp +++ b/indra/llcommon/llassettype.cpp @@ -31,6 +31,9 @@ #include "llmemory.h" #include "llsingleton.h" +#include +#include + ///---------------------------------------------------------------------------- /// Class LLAssetType ///---------------------------------------------------------------------------- @@ -48,7 +51,8 @@ struct AssetEntry : public LLDictionaryEntry mHumanName(human_name), mCanLink(can_link), mCanFetch(can_fetch), - mCanKnow(can_know) + mCanKnow(can_know), + mFetchWithVACap(false) { llassert(strlen(mTypeName) <= 8); } @@ -58,6 +62,7 @@ struct AssetEntry : public LLDictionaryEntry bool mCanLink; bool mCanFetch; bool mCanKnow; + bool mFetchWithVACap; }; class LLAssetDictionary : public LLSingleton, @@ -251,3 +256,89 @@ bool LLAssetType::lookupIsAssetIDKnowable(EType asset_type) } return false; } + +// static +bool LLAssetType::lookupFetchWithVACap(EType asset_type) +{ + LLAssetDictionary *dict = LLAssetDictionary::getInstance(); + const AssetEntry *entry = dict->lookup(asset_type); + if (entry) + { + return entry->mFetchWithVACap; + } + return false; +} + +// FIXME asset-http yank all this after asset-http becomes universal +void LLAssetType::setFetchWithVACapTypeString(const std::string& type_string) +{ + LLAssetDictionary *dict = LLAssetDictionary::getInstance(); + if (type_string=="none") + { + for (LLAssetDictionary::iterator iter = dict->begin(); + iter != dict->end(); + iter++) + { + AssetEntry *entry = iter->second; + entry->mFetchWithVACap = false; + } + } + else if (type_string=="all") + { + for (LLAssetDictionary::iterator iter = dict->begin(); + iter != dict->end(); + iter++) + { + AssetEntry *entry = iter->second; + entry->mFetchWithVACap = true; + } + } + else + { + for (LLAssetDictionary::iterator iter = dict->begin(); + iter != dict->end(); + iter++) + { + AssetEntry *entry = iter->second; + if (entry->mTypeName==type_string) + { + entry->mFetchWithVACap = true; + } + } + } +} + +// FIXME asset-http yank all this after asset-http becomes universal +void LLAssetType::setFetchWithVACapConfigString(const std::string& config_string) +{ + // Clear any currently enabled types + LLAssetType::setFetchWithVACapTypeString("none"); + + // Enable all types specified in the config string. + std::set type_names_for_va_cap; + boost::split(type_names_for_va_cap, config_string, boost::is_any_of(" :,")); + for (std::set::const_iterator it = type_names_for_va_cap.begin(); + it != type_names_for_va_cap.end(); ++it) + { + const std::string& type_string = *it; + LLAssetType::setFetchWithVACapTypeString(type_string); + } + + LLAssetDictionary *dict = LLAssetDictionary::getInstance(); + bool any_found = false; + for (LLAssetDictionary::iterator iter = dict->begin(); + iter != dict->end(); + iter++) + { + AssetEntry *entry = iter->second; + if (entry->mFetchWithVACap) + { + any_found = true; + LL_WARNS() << "Fetch with ViewerAsset cap enabled for " << entry->mTypeName << LL_ENDL; + } + } + if (!any_found) + { + LL_WARNS() << "Fetch with ViewerAsset cap disabled for all types" << LL_ENDL; + } +} diff --git a/indra/llcommon/llassettype.h b/indra/llcommon/llassettype.h index 3a4b5dad18..e06ebc2a35 100644 --- a/indra/llcommon/llassettype.h +++ b/indra/llcommon/llassettype.h @@ -152,6 +152,11 @@ public: static bool lookupIsAssetFetchByIDAllowed(EType asset_type); // the asset allows direct download static bool lookupIsAssetIDKnowable(EType asset_type); // asset data can be known by the viewer + + static bool lookupFetchWithVACap(EType asset_type); // asset data is fetched via http using ViewerAsset cap. + + static void setFetchWithVACapTypeString(const std::string& type_string); + static void setFetchWithVACapConfigString(const std::string& config_string); static const std::string& badLookup(); // error string when a lookup fails -- cgit v1.2.3 From f70abb4ad628b19c993a22c7e86d350395555fcf Mon Sep 17 00:00:00 2001 From: "Brad Payne (Vir Linden)" Date: Fri, 3 Mar 2017 15:14:09 -0500 Subject: SL-409 - added tracking for bytes fetched to viewer assets metrics (does not currently work for textures) --- indra/llcommon/llerror.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index e6407ecf22..9c49f7eff4 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -572,7 +572,7 @@ namespace LLError mFunctionString += std::string(mFunction) + ":"; for (size_t i = 0; i < mTagCount; i++) { - mTagString += std::string("#") + mTags[i] + ((i == mTagCount - 1) ? "" : ","); + mTagString += std::string("#") + mTags[i] + ((i == mTagCount - 1) ? " " : ","); } } -- cgit v1.2.3 From 17384ce6c43a9e07031d7deeea4ec9bbee76199c Mon Sep 17 00:00:00 2001 From: "Brad Payne (Vir Linden)" Date: Mon, 20 Mar 2017 16:04:02 -0400 Subject: MAINT-7195 work (can't repro), removed UDP fetching path, handle more possible timing issues while connecting to region --- indra/llcommon/llassettype.cpp | 90 +----------------------------------------- indra/llcommon/llassettype.h | 5 --- 2 files changed, 1 insertion(+), 94 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llassettype.cpp b/indra/llcommon/llassettype.cpp index 6ecc2ec740..5e8129b9b2 100644 --- a/indra/llcommon/llassettype.cpp +++ b/indra/llcommon/llassettype.cpp @@ -51,8 +51,7 @@ struct AssetEntry : public LLDictionaryEntry mHumanName(human_name), mCanLink(can_link), mCanFetch(can_fetch), - mCanKnow(can_know), - mFetchWithVACap(false) + mCanKnow(can_know) { llassert(strlen(mTypeName) <= 8); } @@ -62,7 +61,6 @@ struct AssetEntry : public LLDictionaryEntry bool mCanLink; bool mCanFetch; bool mCanKnow; - bool mFetchWithVACap; }; class LLAssetDictionary : public LLSingleton, @@ -256,89 +254,3 @@ bool LLAssetType::lookupIsAssetIDKnowable(EType asset_type) } return false; } - -// static -bool LLAssetType::lookupFetchWithVACap(EType asset_type) -{ - LLAssetDictionary *dict = LLAssetDictionary::getInstance(); - const AssetEntry *entry = dict->lookup(asset_type); - if (entry) - { - return entry->mFetchWithVACap; - } - return false; -} - -// FIXME asset-http yank all this after asset-http becomes universal -void LLAssetType::setFetchWithVACapTypeString(const std::string& type_string) -{ - LLAssetDictionary *dict = LLAssetDictionary::getInstance(); - if (type_string=="none") - { - for (LLAssetDictionary::iterator iter = dict->begin(); - iter != dict->end(); - iter++) - { - AssetEntry *entry = iter->second; - entry->mFetchWithVACap = false; - } - } - else if (type_string=="all") - { - for (LLAssetDictionary::iterator iter = dict->begin(); - iter != dict->end(); - iter++) - { - AssetEntry *entry = iter->second; - entry->mFetchWithVACap = true; - } - } - else - { - for (LLAssetDictionary::iterator iter = dict->begin(); - iter != dict->end(); - iter++) - { - AssetEntry *entry = iter->second; - if (entry->mTypeName==type_string) - { - entry->mFetchWithVACap = true; - } - } - } -} - -// FIXME asset-http yank all this after asset-http becomes universal -void LLAssetType::setFetchWithVACapConfigString(const std::string& config_string) -{ - // Clear any currently enabled types - LLAssetType::setFetchWithVACapTypeString("none"); - - // Enable all types specified in the config string. - std::set type_names_for_va_cap; - boost::split(type_names_for_va_cap, config_string, boost::is_any_of(" :,")); - for (std::set::const_iterator it = type_names_for_va_cap.begin(); - it != type_names_for_va_cap.end(); ++it) - { - const std::string& type_string = *it; - LLAssetType::setFetchWithVACapTypeString(type_string); - } - - LLAssetDictionary *dict = LLAssetDictionary::getInstance(); - bool any_found = false; - for (LLAssetDictionary::iterator iter = dict->begin(); - iter != dict->end(); - iter++) - { - AssetEntry *entry = iter->second; - if (entry->mFetchWithVACap) - { - any_found = true; - LL_WARNS() << "Fetch with ViewerAsset cap enabled for " << entry->mTypeName << LL_ENDL; - } - } - if (!any_found) - { - LL_WARNS() << "Fetch with ViewerAsset cap disabled for all types" << LL_ENDL; - } -} diff --git a/indra/llcommon/llassettype.h b/indra/llcommon/llassettype.h index e06ebc2a35..b849be9f16 100644 --- a/indra/llcommon/llassettype.h +++ b/indra/llcommon/llassettype.h @@ -153,11 +153,6 @@ public: static bool lookupIsAssetFetchByIDAllowed(EType asset_type); // the asset allows direct download static bool lookupIsAssetIDKnowable(EType asset_type); // asset data can be known by the viewer - static bool lookupFetchWithVACap(EType asset_type); // asset data is fetched via http using ViewerAsset cap. - - static void setFetchWithVACapTypeString(const std::string& type_string); - static void setFetchWithVACapConfigString(const std::string& config_string); - static const std::string& badLookup(); // error string when a lookup fails protected: -- cgit v1.2.3 From 7af4a49835880fc92461882d5354c0ff12a9672a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 23 Mar 2017 22:39:31 -0400 Subject: MAINT-6789: Add LLEventBatch, LLEventThrottle, LLEventBatchThrottle. These classes are as yet untested: they are straw people for API review, based on email conversations with Caladbolg and Rider. --- indra/llcommon/lleventfilter.cpp | 115 +++++++++++++++++++++++++++++++++++++++ indra/llcommon/lleventfilter.h | 104 +++++++++++++++++++++++++++++++++++ 2 files changed, 219 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 64ab58adcd..1fd29ea9e5 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -148,6 +148,11 @@ bool LLEventTimeoutBase::tick(const LLSD&) return false; // show event to other listeners } +bool LLEventTimeoutBase::running() const +{ + return mMainloop.connected(); +} + LLEventTimeout::LLEventTimeout() {} LLEventTimeout::LLEventTimeout(LLEventPump& source): @@ -164,3 +169,113 @@ bool LLEventTimeout::countdownElapsed() const { return mTimer.hasExpired(); } + +LLEventBatch::LLEventBatch(std::size_t size): + LLEventFilter("batch"), + mBatchSize(size) +{} + +LLEventBatch::LLEventBatch(LLEventPump& source, std::size_t size): + LLEventFilter(source, "batch"), + mBatchSize(size) +{} + +void LLEventBatch::flush() +{ + // copy and clear mBatch BEFORE posting to avoid weird circularity effects + LLSD batch(mBatch); + mBatch.clear(); + LLEventStream::post(batch); +} + +bool LLEventBatch::post(const LLSD& event) +{ + mBatch.append(event); + if (mBatch.size() >= mBatchSize) + { + flush(); + } + return false; +} + +LLEventThrottle::LLEventThrottle(F32 interval): + LLEventFilter("throttle"), + mInterval(interval), + mPosts(0) +{} + +LLEventThrottle::LLEventThrottle(LLEventPump& source, F32 interval): + LLEventFilter(source, "throttle"), + mInterval(interval), + mPosts(0) +{} + +void LLEventThrottle::flush() +{ + // flush() is a no-op unless there's something pending. + // Don't test mPending because there's no requirement that the consumer + // post() anything but an isUndefined(). This is what mPosts is for. + if (mPosts) + { + mPosts = 0; + mAlarm.cancel(); + // This is not to set our alarm; we are not yet requesting + // any notification. This is just to track whether subsequent post() + // calls fall within this mInterval or not. + mTimer.setTimerExpirySec(mInterval); + // copy and clear mPending BEFORE posting to avoid weird circularity + // effects + LLSD pending = mPending; + mPending.clear(); + LLEventStream::post(pending); + } +} + +LLSD LLEventThrottle::pending() const +{ + return mPending; +} + +bool LLEventThrottle::post(const LLSD& event) +{ + // Always capture most recent post() event data. If caller wants to + // aggregate multiple events, let them retrieve pending() and modify + // before calling post(). + mPending = event; + // Always increment mPosts. Unless we count this call, flush() does + // nothing. + ++mPosts; + // We reset mTimer on every flush() call to let us know if we're still + // within the same mInterval. So -- are we? + F32 timeRemaining = mTimer.getRemainingTimeF32(); + if (! timeRemaining) + { + // more than enough time has elapsed, immediately flush() + flush(); + } + else + { + // still within mInterval of the last flush() call: have to defer + if (! mAlarm.running()) + { + // timeRemaining tells us how much longer it will be until + // mInterval seconds since the last flush() call. At that time, + // flush() deferred events. + mAlarm.actionAfter(timeRemaining, boost::bind(&LLEventThrottle::flush, this)); + } + } +} + +LLEventBatchThrottle::LLEventBatchThrottle(F32 interval): + LLEventThrottle(interval) +{} + +LLEventBatchThrottle::LLEventBatchThrottle(LLEventPump& source, F32 interval): + LLEventThrottle(source, interval) +{} + +bool LLEventBatchThrottle::post(const LLSD& event) +{ + // simply retrieve pending value and append the new event to it + return LLEventThrottle::post(pending().append(event)); +} diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 66f3c14869..1445b8a3b7 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -177,6 +177,9 @@ public: /// Cancel timer without event void cancel(); + /// Is this timer currently running? + bool running() const; + protected: virtual void setCountdown(F32 seconds) = 0; virtual bool countdownElapsed() const = 0; @@ -215,4 +218,105 @@ private: LLTimer mTimer; }; +/** + * LLEventBatch: accumulate post() events (LLSD blobs) into an LLSD Array + * until the array reaches a certain size, then call listeners with the Array + * and clear it back to empty. + */ +class LL_COMMON_API LLEventBatch: public LLEventFilter +{ +public: + // pass batch size + LLEventBatch(std::size_t size); + // construct and connect + LLEventBatch(LLEventPump& source, std::size_t size); + + // force out the pending batch + void flush(); + + // accumulate an event and flush() when big enough + virtual bool post(const LLSD& event); + +private: + LLSD mBatch; + std::size_t mBatchSize; +}; + +/** + * LLEventThrottle: construct with a time interval. Regardless of how + * frequently you call post(), LLEventThrottle will pass on an event to + * its listeners no more often than once per specified interval. + * + * A new event after more than the specified interval will immediately be + * passed along to listeners. But subsequent events will be delayed until at + * least one time interval since listeners were last called. Consider the + * sequence below. Suppose we have an LLEventThrottle constructed with an + * interval of 3 seconds. The numbers on the left are timestamps in seconds + * relative to an arbitrary reference point. + * + * 1: post(): event immediately passed to listeners, next no sooner than 4 + * 2: post(): deferred: waiting for 3 seconds to elapse + * 3: post(): deferred + * 4: no post() call, but event delivered to listeners; next no sooner than 7 + * 6: post(): deferred + * 7: no post() call, but event delivered; next no sooner than 10 + * 12: post(): immediately passed to listeners, next no sooner than 15 + * 17: post(): immediately passed to listeners, next no sooner than 20 + * + * For a deferred event, the LLSD blob delivered to listeners is from the most + * recent deferred post() call. However, you may obtain the previous event + * blob by calling pending(), modify it however you want and post() the new + * value. Each time an event is delivered to listeners, the pending() value is + * reset to isUndefined(). + * + * You may also call flush() to immediately pass along any deferred events to + * all listeners. + */ +class LL_COMMON_API LLEventThrottle: public LLEventFilter +{ +public: + // pass time interval + LLEventThrottle(F32 interval); + // construct and connect + LLEventThrottle(LLEventPump& source, F32 interval); + + // force out any deferred events + void flush(); + + // retrieve (aggregate) deferred event since last event sent to listeners + LLSD pending() const; + + // register an event, may be either passed through or deferred + virtual bool post(const LLSD& event); + +private: + // remember throttle interval + F32 mInterval; + // count post() calls since last flush() + std::size_t mPosts; + // pending event data from most recent deferred event + LLSD mPending; + // use this to arrange a deferred flush() call + LLEventTimeout mAlarm; + // use this to track whether we're within mInterval of last flush() + LLTimer mTimer; +}; + +/** + * LLEventBatchThrottle: like LLEventThrottle, it refuses to pass events to + * listeners more often than once per specified time interval. + * Like LLEventBatch, it accumulates pending events into an LLSD Array. + */ +class LLEventBatchThrottle: public LLEventThrottle +{ +public: + // pass time interval + LLEventBatchThrottle(F32 interval); + // construct and connect + LLEventBatchThrottle(LLEventPump& source, F32 interval); + + // append a new event to current batch + virtual bool post(const LLSD& event); +}; + #endif /* ! defined(LL_LLEVENTFILTER_H) */ -- cgit v1.2.3 From 2fe4f6b9187d9153e335bf54ea43fc89a7987459 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 24 Mar 2017 18:25:16 -0400 Subject: MAINT-6789: Add LLEventThrottle::getInterval(), setInterval() plus LLEventBatch::getSize(), setSize() plus LLEventThrottle::getPostCount() and getDelay(). The interesting thing about LLEventThrottle::setInterval() and LLEventBatch::setSize() is that either might cause an immediate flush(). --- indra/llcommon/lleventfilter.cpp | 49 ++++++++++++++++++++++++++++++++++++++++ indra/llcommon/lleventfilter.h | 14 ++++++++++++ 2 files changed, 63 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 1fd29ea9e5..87eb583cb6 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -198,6 +198,16 @@ bool LLEventBatch::post(const LLSD& event) return false; } +void LLEventBatch::setSize(std::size_t size) +{ + mBatchSize = size; + // changing the size might mean that we have to flush NOW + if (mBatch.size() >= mBatchSize) + { + flush(); + } +} + LLEventThrottle::LLEventThrottle(F32 interval): LLEventFilter("throttle"), mInterval(interval), @@ -264,6 +274,45 @@ bool LLEventThrottle::post(const LLSD& event) mAlarm.actionAfter(timeRemaining, boost::bind(&LLEventThrottle::flush, this)); } } + return false; +} + +void LLEventThrottle::setInterval(F32 interval) +{ + F32 oldInterval = mInterval; + mInterval = interval; + // If we are not now within oldInterval of the last flush(), we're done: + // this will only affect behavior starting with the next flush(). + F32 timeRemaining = mTimer.getRemainingTimeF32(); + if (timeRemaining) + { + // We are currently within oldInterval of the last flush(). Figure out + // how much time remains until (the new) mInterval of the last + // flush(). Bt we don't actually store a timestamp for the last + // flush(); it's implicit. There are timeRemaining seconds until what + // used to be the end of the interval. Move that endpoint by the + // difference between the new interval and the old. + timeRemaining += (mInterval - oldInterval); + // If we're called with a larger interval, the difference is positive + // and timeRemaining increases. + // If we're called with a smaller interval, the difference is negative + // and timeRemaining decreases. The interesting case is when it goes + // nonpositive: when the new interval means we can flush immediately. + if (timeRemaining <= 0.0f) + { + flush(); + } + else + { + // immediately reset mTimer + mTimer.setTimerExpirySec(timeRemaining); + // and if mAlarm is running, reset that too + if (mAlarm.running()) + { + mAlarm.actionAfter(timeRemaining, boost::bind(&LLEventThrottle::flush, this)); + } + } + } } LLEventBatchThrottle::LLEventBatchThrottle(F32 interval): diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 1445b8a3b7..68890846a7 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -237,6 +237,10 @@ public: // accumulate an event and flush() when big enough virtual bool post(const LLSD& event); + // query or reset batch size + std::size_t getSize() const { return mBatchSize; } + void setSize(std::size_t size); + private: LLSD mBatch; std::size_t mBatchSize; @@ -289,6 +293,16 @@ public: // register an event, may be either passed through or deferred virtual bool post(const LLSD& event); + // query or reset interval + F32 getInterval() const { return mInterval; } + void setInterval(F32 interval); + + // deferred posts + std::size_t getPostCount() const { return mPosts; } + + // time until next event would be passed through, 0.0 if now + F32 getDelay() const { return mTimer.getRemainingTimeF32(); } + private: // remember throttle interval F32 mInterval; -- cgit v1.2.3 From c27dbc62148f51ff5848f63573f5816235f0cc35 Mon Sep 17 00:00:00 2001 From: mnikolenko Date: Mon, 3 Apr 2017 02:21:18 +0300 Subject: MAINT-6404 FIXED When pasting text with mac linebreak into a notecard, it shouldn't be removed --- indra/llcommon/llstring.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llstring.h b/indra/llcommon/llstring.h index a40db0f8cc..2255e638c2 100644 --- a/indra/llcommon/llstring.h +++ b/indra/llcommon/llstring.h @@ -336,6 +336,7 @@ public: static void addCRLF(string_type& string); static void removeCRLF(string_type& string); + static void removeWindowsCR(string_type& string); static void replaceTabsWithSpaces( string_type& string, size_type spaces_per_tab ); static void replaceNonstandardASCII( string_type& string, T replacement ); @@ -1322,6 +1323,28 @@ void LLStringUtilBase::removeCRLF(string_type& string) //static template +void LLStringUtilBase::removeWindowsCR(string_type& string) +{ + const T LF = 10; + const T CR = 13; + + size_type cr_count = 0; + size_type len = string.size(); + size_type i; + for( i = 0; i < len - cr_count - 1; i++ ) + { + if( string[i+cr_count] == CR && string[i+cr_count+1] == LF) + { + cr_count++; + } + + string[i] = string[i+cr_count]; + } + string.erase(i, cr_count); +} + +//static +template void LLStringUtilBase::replaceChar( string_type& string, T target, T replacement ) { size_type found_pos = 0; -- cgit v1.2.3 From 10213f994f64de1f50e1ff6b9b5f43be549d19cb Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Fri, 7 Apr 2017 20:29:24 +0300 Subject: MAINT-6283 Names that contain 'http:' or 'https:' were causing new line in chat history --- indra/llcommon/lluri.cpp | 7 ++++--- indra/llcommon/lluri.h | 8 ++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lluri.cpp b/indra/llcommon/lluri.cpp index 9f12d49244..758b98e143 100644 --- a/indra/llcommon/lluri.cpp +++ b/indra/llcommon/lluri.cpp @@ -40,7 +40,8 @@ #include #include -void encode_character(std::ostream& ostr, std::string::value_type val) +// static +void LLURI::encodeCharacter(std::ostream& ostr, std::string::value_type val) { ostr << "%" @@ -95,7 +96,7 @@ std::string LLURI::escape( } else { - encode_character(ostr, c); + encodeCharacter(ostr, c); } } } @@ -106,7 +107,7 @@ std::string LLURI::escape( c = *it; if(allowed.find(c) == std::string::npos) { - encode_character(ostr, c); + encodeCharacter(ostr, c); } else { diff --git a/indra/llcommon/lluri.h b/indra/llcommon/lluri.h index c82a666e48..9e44cc7da2 100644 --- a/indra/llcommon/lluri.h +++ b/indra/llcommon/lluri.h @@ -120,6 +120,14 @@ public: /** @name Escaping Utilities */ //@{ + /** + * @brief 'Escape' symbol into stream + * + * @param ostr Output stream. + * @param val Symbol to encode. + */ + static void encodeCharacter(std::ostream& ostr, std::string::value_type val); + /** * @brief Escape the string passed except for unreserved * -- cgit v1.2.3 From 0dcb423cf3dc42e11621eece972801b036657e91 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Tue, 25 Apr 2017 17:48:34 +0300 Subject: MAINT-7145 Eliminate LLSingleton circular references --- indra/llcommon/llsingleton.cpp | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 9025e53bb2..a3a87edd88 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -220,6 +220,9 @@ void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initSt std::find(initializing.begin(), initializing.end(), this); if (found != initializing.end()) { + list_t::const_iterator it_next = found; + it_next++; + // Report the circularity. Requiring the coder to dig through the // logic to diagnose exactly how we got here is less than helpful. std::ostringstream out; @@ -238,11 +241,30 @@ void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initSt // 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 based on initState. They have - // identical signatures. - ((initState == CONSTRUCTING)? logerrs : logwarns) - ("LLSingleton circularity: ", out.str().c_str(), - demangle(typeid(*this).name()).c_str(), ""); + // Decide which log helper to call. + if (initState == CONSTRUCTING) + { + logerrs("LLSingleton circularity in Constructor: ", out.str().c_str(), + demangle(typeid(*this).name()).c_str(), ""); + } + else if (it_next == initializing.end()) + { + // Points to self after construction, but during initialization. + // Singletons can initialize other classes that depend onto them, + // so this is expected. + // + // Example: LLNotifications singleton initializes default channels. + // Channels register themselves with singleton once done. + logdebugs("LLSingleton circularity: ", out.str().c_str(), + demangle(typeid(*this).name()).c_str(), ""); + } + else + { + // Actual circularity with other singleton (or single singleton is used extensively). + // Dependency can be unclear. + logwarns("LLSingleton circularity: ", out.str().c_str(), + demangle(typeid(*this).name()).c_str(), ""); + } } else { -- cgit v1.2.3 From b7b8d6e1aedeac1dfdfcc9200024bbcc8e2dacae Mon Sep 17 00:00:00 2001 From: "Brad Payne (Vir Linden)" Date: Wed, 26 Apr 2017 12:39:14 -0400 Subject: MAINT-7343 - added periodic logging of state of the asset store. --- indra/llcommon/llassettype.cpp | 3 --- 1 file changed, 3 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llassettype.cpp b/indra/llcommon/llassettype.cpp index 5e8129b9b2..4304db36be 100644 --- a/indra/llcommon/llassettype.cpp +++ b/indra/llcommon/llassettype.cpp @@ -31,9 +31,6 @@ #include "llmemory.h" #include "llsingleton.h" -#include -#include - ///---------------------------------------------------------------------------- /// Class LLAssetType ///---------------------------------------------------------------------------- -- cgit v1.2.3 From 9c66072cacae0b3b86a277780e3a19e94accd6bc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 10 May 2017 15:04:18 -0400 Subject: Add LLEventThrottle tests; actually *all* lleventfilter.cpp tests. For some reason there wasn't an entry in indra/llcommon/CMakeLists.txt to run the tests in indra/llcommon/tests/lleventfilter_test.cpp. It seems likely that at some point it existed, since all previous tests built and ran successfully. In any case, (re-)add lleventfilter_test.cpp to the set of llcommon tests. Also alphabetize them to make it easier to find a particular test invocation. Also add new tests for LLEventThrottle. To support this, refactor the concrete LLEventThrottle class into LLEventThrottleBase containing all the tricky logic, with pure virtual methods for access to LLTimer and LLEventTimeout, and an LLEventThrottle subclass containing the LLTimer and LLEventTimeout instances and corresponding implementations of the new pure virtual methods. That permits us to introduce TestEventThrottle, an alternate subclass with dummy implementations of the methods related to LLTimer and LLEventTimeout. In particular, we can explicitly advance simulated realtime to simulate particular LLTimer and LLEventTimeout behaviors. Finally, introduce Concat, a test LLEventPump listener class whose function is to concatenate received string event data into a composite string so we can readily test for particular sequences of events. --- indra/llcommon/CMakeLists.txt | 15 ++-- indra/llcommon/lleventfilter.cpp | 96 +++++++++++++++++---- indra/llcommon/lleventfilter.h | 49 +++++++++-- indra/llcommon/tests/listener.h | 11 +++ indra/llcommon/tests/lleventfilter_test.cpp | 124 +++++++++++++++++++++++++++- 5 files changed, 262 insertions(+), 33 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index 3493f80556..aa76a57f1d 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -324,26 +324,27 @@ if (LL_TESTS) LL_ADD_INTEGRATION_TEST(lldeadmantimer "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lldependencies "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llerror "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(lleventdispatcher "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(lleventcoro "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(lleventfilter "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llframetimer "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llheteromap "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llinstancetracker "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocessor "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llprocinfo "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llsdserialize "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llsingleton "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(llstreamqueue "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llstring "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lltrace "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lltreeiterators "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lluri "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llunits "" "${test_libs}") LL_ADD_INTEGRATION_TEST(stringize "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(lleventdispatcher "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(lleventcoro "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llprocess "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llleap "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llstreamqueue "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llpounceable "" "${test_libs}") - LL_ADD_INTEGRATION_TEST(llheteromap "" "${test_libs}") ## llexception_test.cpp isn't a regression test, and doesn't need to be run ## every build. It's to help a developer make implementation choices about diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index 87eb583cb6..ddd7d5547a 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -38,12 +38,18 @@ #include "llerror.h" // LL_ERRS #include "llsdutil.h" // llsd_matches() +/***************************************************************************** +* LLEventFilter +*****************************************************************************/ LLEventFilter::LLEventFilter(LLEventPump& source, const std::string& name, bool tweak): LLEventStream(name, tweak), mSource(source.listen(getName(), boost::bind(&LLEventFilter::post, this, _1))) { } +/***************************************************************************** +* LLEventMatching +*****************************************************************************/ LLEventMatching::LLEventMatching(const LLSD& pattern): LLEventFilter("matching"), mPattern(pattern) @@ -64,6 +70,9 @@ bool LLEventMatching::post(const LLSD& event) return LLEventStream::post(event); } +/***************************************************************************** +* LLEventTimeoutBase +*****************************************************************************/ LLEventTimeoutBase::LLEventTimeoutBase(): LLEventFilter("timeout") { @@ -153,6 +162,9 @@ bool LLEventTimeoutBase::running() const return mMainloop.connected(); } +/***************************************************************************** +* LLEventTimeout +*****************************************************************************/ LLEventTimeout::LLEventTimeout() {} LLEventTimeout::LLEventTimeout(LLEventPump& source): @@ -170,6 +182,9 @@ bool LLEventTimeout::countdownElapsed() const return mTimer.hasExpired(); } +/***************************************************************************** +* LLEventBatch +*****************************************************************************/ LLEventBatch::LLEventBatch(std::size_t size): LLEventFilter("batch"), mBatchSize(size) @@ -208,19 +223,22 @@ void LLEventBatch::setSize(std::size_t size) } } -LLEventThrottle::LLEventThrottle(F32 interval): +/***************************************************************************** +* LLEventThrottleBase +*****************************************************************************/ +LLEventThrottleBase::LLEventThrottleBase(F32 interval): LLEventFilter("throttle"), mInterval(interval), mPosts(0) {} -LLEventThrottle::LLEventThrottle(LLEventPump& source, F32 interval): +LLEventThrottleBase::LLEventThrottleBase(LLEventPump& source, F32 interval): LLEventFilter(source, "throttle"), mInterval(interval), mPosts(0) {} -void LLEventThrottle::flush() +void LLEventThrottleBase::flush() { // flush() is a no-op unless there's something pending. // Don't test mPending because there's no requirement that the consumer @@ -228,11 +246,11 @@ void LLEventThrottle::flush() if (mPosts) { mPosts = 0; - mAlarm.cancel(); + alarmCancel(); // This is not to set our alarm; we are not yet requesting // any notification. This is just to track whether subsequent post() // calls fall within this mInterval or not. - mTimer.setTimerExpirySec(mInterval); + timerSet(mInterval); // copy and clear mPending BEFORE posting to avoid weird circularity // effects LLSD pending = mPending; @@ -241,12 +259,12 @@ void LLEventThrottle::flush() } } -LLSD LLEventThrottle::pending() const +LLSD LLEventThrottleBase::pending() const { return mPending; } -bool LLEventThrottle::post(const LLSD& event) +bool LLEventThrottleBase::post(const LLSD& event) { // Always capture most recent post() event data. If caller wants to // aggregate multiple events, let them retrieve pending() and modify @@ -257,7 +275,7 @@ bool LLEventThrottle::post(const LLSD& event) ++mPosts; // We reset mTimer on every flush() call to let us know if we're still // within the same mInterval. So -- are we? - F32 timeRemaining = mTimer.getRemainingTimeF32(); + F32 timeRemaining = timerGetRemaining(); if (! timeRemaining) { // more than enough time has elapsed, immediately flush() @@ -266,24 +284,24 @@ bool LLEventThrottle::post(const LLSD& event) else { // still within mInterval of the last flush() call: have to defer - if (! mAlarm.running()) + if (! alarmRunning()) { // timeRemaining tells us how much longer it will be until // mInterval seconds since the last flush() call. At that time, // flush() deferred events. - mAlarm.actionAfter(timeRemaining, boost::bind(&LLEventThrottle::flush, this)); + alarmActionAfter(timeRemaining, boost::bind(&LLEventThrottleBase::flush, this)); } } return false; } -void LLEventThrottle::setInterval(F32 interval) +void LLEventThrottleBase::setInterval(F32 interval) { F32 oldInterval = mInterval; mInterval = interval; // If we are not now within oldInterval of the last flush(), we're done: // this will only affect behavior starting with the next flush(). - F32 timeRemaining = mTimer.getRemainingTimeF32(); + F32 timeRemaining = timerGetRemaining(); if (timeRemaining) { // We are currently within oldInterval of the last flush(). Figure out @@ -305,16 +323,60 @@ void LLEventThrottle::setInterval(F32 interval) else { // immediately reset mTimer - mTimer.setTimerExpirySec(timeRemaining); + timerSet(timeRemaining); // and if mAlarm is running, reset that too - if (mAlarm.running()) + if (alarmRunning()) { - mAlarm.actionAfter(timeRemaining, boost::bind(&LLEventThrottle::flush, this)); + alarmActionAfter(timeRemaining, boost::bind(&LLEventThrottleBase::flush, this)); } } } } +F32 LLEventThrottleBase::getDelay() const +{ + return timerGetRemaining(); +} + +/***************************************************************************** +* LLEventThrottle implementation +*****************************************************************************/ +LLEventThrottle::LLEventThrottle(F32 interval): + LLEventThrottleBase(interval) +{} + +LLEventThrottle::LLEventThrottle(LLEventPump& source, F32 interval): + LLEventThrottleBase(source, interval) +{} + +void LLEventThrottle::alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) +{ + mAlarm.actionAfter(interval, action); +} + +bool LLEventThrottle::alarmRunning() const +{ + return mAlarm.running(); +} + +void LLEventThrottle::alarmCancel() +{ + return mAlarm.cancel(); +} + +void LLEventThrottle::timerSet(F32 interval) +{ + mTimer.setTimerExpirySec(interval); +} + +F32 LLEventThrottle::timerGetRemaining() const +{ + return mTimer.getRemainingTimeF32(); +} + +/***************************************************************************** +* LLEventBatchThrottle +*****************************************************************************/ LLEventBatchThrottle::LLEventBatchThrottle(F32 interval): LLEventThrottle(interval) {} @@ -326,5 +388,7 @@ LLEventBatchThrottle::LLEventBatchThrottle(LLEventPump& source, F32 interval): bool LLEventBatchThrottle::post(const LLSD& event) { // simply retrieve pending value and append the new event to it - return LLEventThrottle::post(pending().append(event)); + LLSD partial = pending(); + partial.append(event); + return LLEventThrottle::post(partial); } diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index 68890846a7..cae18bfd86 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -247,7 +247,7 @@ private: }; /** - * LLEventThrottle: construct with a time interval. Regardless of how + * LLEventThrottleBase: construct with a time interval. Regardless of how * frequently you call post(), LLEventThrottle will pass on an event to * its listeners no more often than once per specified interval. * @@ -268,21 +268,24 @@ private: * 17: post(): immediately passed to listeners, next no sooner than 20 * * For a deferred event, the LLSD blob delivered to listeners is from the most - * recent deferred post() call. However, you may obtain the previous event - * blob by calling pending(), modify it however you want and post() the new - * value. Each time an event is delivered to listeners, the pending() value is - * reset to isUndefined(). + * recent deferred post() call. However, a sender may obtain the previous + * event blob by calling pending(), modifying it as desired and post()ing the + * new value. Each time an event is delivered to listeners, the pending() + * value is reset to isUndefined(). * * You may also call flush() to immediately pass along any deferred events to * all listeners. + * + * @NOTE This is an abstract base class so that, for testing, we can use an + * alternate "timer" that doesn't actually consume real time. */ -class LL_COMMON_API LLEventThrottle: public LLEventFilter +class LL_COMMON_API LLEventThrottleBase: public LLEventFilter { public: // pass time interval - LLEventThrottle(F32 interval); + LLEventThrottleBase(F32 interval); // construct and connect - LLEventThrottle(LLEventPump& source, F32 interval); + LLEventThrottleBase(LLEventPump& source, F32 interval); // force out any deferred events void flush(); @@ -301,7 +304,17 @@ public: std::size_t getPostCount() const { return mPosts; } // time until next event would be passed through, 0.0 if now - F32 getDelay() const { return mTimer.getRemainingTimeF32(); } + F32 getDelay() const; + +protected: + // Implement these time-related methods for a valid LLEventThrottleBase + // subclass (see LLEventThrottle). For testing, we use a subclass that + // doesn't involve actual elapsed time. + virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) = 0; + virtual bool alarmRunning() const = 0; + virtual void alarmCancel() = 0; + virtual void timerSet(F32 interval) = 0; + virtual F32 timerGetRemaining() const = 0; private: // remember throttle interval @@ -310,6 +323,24 @@ private: std::size_t mPosts; // pending event data from most recent deferred event LLSD mPending; +}; + +/** + * Production implementation of LLEventThrottle. + */ +class LLEventThrottle: public LLEventThrottleBase +{ +public: + LLEventThrottle(F32 interval); + LLEventThrottle(LLEventPump& source, F32 interval); + +private: + virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) override; + virtual bool alarmRunning() const override; + virtual void alarmCancel() override; + virtual void timerSet(F32 interval) override; + virtual F32 timerGetRemaining() const override; + // use this to arrange a deferred flush() call LLEventTimeout mAlarm; // use this to track whether we're within mInterval of last flush() diff --git a/indra/llcommon/tests/listener.h b/indra/llcommon/tests/listener.h index 9c5c18a150..6072060bb6 100644 --- a/indra/llcommon/tests/listener.h +++ b/indra/llcommon/tests/listener.h @@ -138,4 +138,15 @@ struct Collect StringVec result; }; +struct Concat +{ + bool operator()(const LLSD& event) + { + result += event.asString(); + return false; + } + void clear() { result.clear(); } + std::string result; +}; + #endif /* ! defined(LL_LISTENER_H) */ diff --git a/indra/llcommon/tests/lleventfilter_test.cpp b/indra/llcommon/tests/lleventfilter_test.cpp index 2cdfb52f2f..712864bf63 100644 --- a/indra/llcommon/tests/lleventfilter_test.cpp +++ b/indra/llcommon/tests/lleventfilter_test.cpp @@ -70,6 +70,85 @@ private: bool mElapsed; }; +// Similar remarks about LLEventThrottle: we're actually testing the logic in +// LLEventThrottleBase, dummying out the LLTimer and LLEventTimeout used by +// the production LLEventThrottle class. +class TestEventThrottle: public LLEventThrottleBase +{ +public: + TestEventThrottle(F32 interval): + LLEventThrottleBase(interval), + mAlarmRemaining(-1), + mTimerRemaining(-1) + {} + TestEventThrottle(LLEventPump& source, F32 interval): + LLEventThrottleBase(source, interval), + mAlarmRemaining(-1), + mTimerRemaining(-1) + {} + + /*----- implementation of LLEventThrottleBase timing functionality -----*/ + virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) override + { + mAlarmRemaining = interval; + mAlarmAction = action; + } + + virtual bool alarmRunning() const override + { + // decrementing to exactly 0 should mean the alarm fires + return mAlarmRemaining > 0; + } + + virtual void alarmCancel() override + { + mAlarmRemaining = -1; + } + + virtual void timerSet(F32 interval) override + { + mTimerRemaining = interval; + } + + virtual F32 timerGetRemaining() const override + { + // LLTimer.getRemainingTimeF32() never returns negative; 0.0 means expired + return (mTimerRemaining > 0.0)? mTimerRemaining : 0.0; + } + + /*------------------- methods for manipulating time --------------------*/ + void alarmAdvance(F32 delta) + { + bool wasRunning = alarmRunning(); + mAlarmRemaining -= delta; + if (wasRunning && ! alarmRunning()) + { + mAlarmAction(); + } + } + + void timerAdvance(F32 delta) + { + // This simple implementation, like alarmAdvance(), completely ignores + // HOW negative mTimerRemaining might go. All that matters is whether + // it's negative. We trust that no test method in this source will + // drive it beyond the capacity of an F32. Seems like a safe assumption. + mTimerRemaining -= delta; + } + + void advance(F32 delta) + { + // Advance the timer first because it has no side effects. + // alarmAdvance() might call flush(), which will need to see the + // change in the timer. + timerAdvance(delta); + alarmAdvance(delta); + } + + F32 mAlarmRemaining, mTimerRemaining; + LLEventTimeoutBase::Action mAlarmAction; +}; + /***************************************************************************** * TUT *****************************************************************************/ @@ -116,7 +195,9 @@ namespace tut listener0.listenTo(driver)); // Construct a pattern LLSD: desired Event must have a key "foo" // containing string "bar" - LLEventMatching filter(driver, LLSD().insert("foo", "bar")); + LLSD pattern; + pattern.insert("foo", "bar"); + LLEventMatching filter(driver, pattern); listener1.reset(0); LLTempBoundListener temp2( listener1.listenTo(filter)); @@ -285,6 +366,47 @@ namespace tut mainloop.post(17); check_listener("no timeout 3", listener0, LLSD(0)); } + + template<> template<> + void filter_object::test<5>() + { + set_test_name("LLEventThrottle"); + TestEventThrottle throttle(3); + Concat cat; + throttle.listen("concat", boost::ref(cat)); + + // (sequence taken from LLEventThrottleBase Doxygen comments) + // 1: post(): event immediately passed to listeners, next no sooner than 4 + throttle.advance(1); + throttle.post("1"); + ensure_equals("1", cat.result, "1"); // delivered immediately + // 2: post(): deferred: waiting for 3 seconds to elapse + throttle.advance(1); + throttle.post("2"); + ensure_equals("2", cat.result, "1"); // "2" not yet delivered + // 3: post(): deferred + throttle.advance(1); + throttle.post("3"); + ensure_equals("3", cat.result, "1"); // "3" not yet delivered + // 4: no post() call, but event delivered to listeners; next no sooner than 7 + throttle.advance(1); + ensure_equals("4", cat.result, "13"); // "3" delivered + // 6: post(): deferred + throttle.advance(2); + throttle.post("6"); + ensure_equals("6", cat.result, "13"); // "6" not yet delivered + // 7: no post() call, but event delivered; next no sooner than 10 + throttle.advance(1); + ensure_equals("7", cat.result, "136"); // "6" delivered + // 12: post(): immediately passed to listeners, next no sooner than 15 + throttle.advance(5); + throttle.post(";12"); + ensure_equals("12", cat.result, "136;12"); // "12" delivered + // 17: post(): immediately passed to listeners, next no sooner than 20 + throttle.advance(5); + throttle.post(";17"); + ensure_equals("17", cat.result, "136;12;17"); // "17" delivered + } } // namespace tut /***************************************************************************** -- cgit v1.2.3 From 4d87ded8862f032682a66704b9c68ef5626779ea Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 10 May 2017 17:37:06 -0400 Subject: Add size limit to LLEventBatchThrottle like LLEventBatch. The new behavior is that it will flush when either the pending batch has grown to the specified size, or the time interval has expired. --- indra/llcommon/lleventfilter.cpp | 35 ++++++++++++++++++++++++++--------- indra/llcommon/lleventfilter.h | 30 +++++++++++++++++++++--------- 2 files changed, 47 insertions(+), 18 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventfilter.cpp b/indra/llcommon/lleventfilter.cpp index ddd7d5547a..9fb18dc67d 100644 --- a/indra/llcommon/lleventfilter.cpp +++ b/indra/llcommon/lleventfilter.cpp @@ -206,10 +206,8 @@ void LLEventBatch::flush() bool LLEventBatch::post(const LLSD& event) { mBatch.append(event); - if (mBatch.size() >= mBatchSize) - { - flush(); - } + // calling setSize(same) performs the very check we want + setSize(mBatchSize); return false; } @@ -377,12 +375,14 @@ F32 LLEventThrottle::timerGetRemaining() const /***************************************************************************** * LLEventBatchThrottle *****************************************************************************/ -LLEventBatchThrottle::LLEventBatchThrottle(F32 interval): - LLEventThrottle(interval) +LLEventBatchThrottle::LLEventBatchThrottle(F32 interval, std::size_t size): + LLEventThrottle(interval), + mBatchSize(size) {} -LLEventBatchThrottle::LLEventBatchThrottle(LLEventPump& source, F32 interval): - LLEventThrottle(source, interval) +LLEventBatchThrottle::LLEventBatchThrottle(LLEventPump& source, F32 interval, std::size_t size): + LLEventThrottle(source, interval), + mBatchSize(size) {} bool LLEventBatchThrottle::post(const LLSD& event) @@ -390,5 +390,22 @@ bool LLEventBatchThrottle::post(const LLSD& event) // simply retrieve pending value and append the new event to it LLSD partial = pending(); partial.append(event); - return LLEventThrottle::post(partial); + bool ret = LLEventThrottle::post(partial); + // The post() call above MIGHT have called flush() already. If it did, + // then pending() was reset to empty. If it did not, though, but the batch + // size has grown to the limit, flush() anyway. If there's a limit at all, + // of course. Calling setSize(same) performs the very check we want. + setSize(mBatchSize); + return ret; +} + +void LLEventBatchThrottle::setSize(std::size_t size) +{ + mBatchSize = size; + // Changing the size might mean that we have to flush NOW. Don't forget + // that 0 means unlimited. + if (mBatchSize && pending().size() >= mBatchSize) + { + flush(); + } } diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index cae18bfd86..ccbbc8bd25 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -270,14 +270,15 @@ private: * For a deferred event, the LLSD blob delivered to listeners is from the most * recent deferred post() call. However, a sender may obtain the previous * event blob by calling pending(), modifying it as desired and post()ing the - * new value. Each time an event is delivered to listeners, the pending() - * value is reset to isUndefined(). + * new value. (See LLEventBatchThrottle.) Each time an event is delivered to + * listeners, the pending() value is reset to isUndefined(). * * You may also call flush() to immediately pass along any deferred events to * all listeners. * * @NOTE This is an abstract base class so that, for testing, we can use an - * alternate "timer" that doesn't actually consume real time. + * alternate "timer" that doesn't actually consume real time. See + * LLEventThrottle. */ class LL_COMMON_API LLEventThrottleBase: public LLEventFilter { @@ -348,20 +349,31 @@ private: }; /** - * LLEventBatchThrottle: like LLEventThrottle, it refuses to pass events to - * listeners more often than once per specified time interval. - * Like LLEventBatch, it accumulates pending events into an LLSD Array. + * LLEventBatchThrottle: like LLEventThrottle, it's reluctant to pass events + * to listeners more often than once per specified time interval -- but only + * reluctant, since exceeding the specified batch size limit can cause it to + * deliver accumulated events sooner. Like LLEventBatch, it accumulates + * pending events into an LLSD Array, optionally flushing when the batch grows + * to a certain size. */ class LLEventBatchThrottle: public LLEventThrottle { public: - // pass time interval - LLEventBatchThrottle(F32 interval); + // pass time interval and (optionally) max batch size; 0 means batch can + // grow arbitrarily large + LLEventBatchThrottle(F32 interval, std::size_t size = 0); // construct and connect - LLEventBatchThrottle(LLEventPump& source, F32 interval); + LLEventBatchThrottle(LLEventPump& source, F32 interval, std::size_t size = 0); // append a new event to current batch virtual bool post(const LLSD& event); + + // query or reset batch size + std::size_t getSize() const { return mBatchSize; } + void setSize(std::size_t size); + +private: + std::size_t mBatchSize; }; #endif /* ! defined(LL_LLEVENTFILTER_H) */ -- cgit v1.2.3 From 4a90678cc7406b0c2c8dad5691cdc2555186931b Mon Sep 17 00:00:00 2001 From: AndreyL ProductEngine Date: Thu, 18 May 2017 03:13:57 +0300 Subject: Linux buildfix; this should be reverted after gcc update to 4.7+ --- indra/llcommon/lleventfilter.h | 10 +++++----- indra/llcommon/tests/lleventfilter_test.cpp | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/lleventfilter.h b/indra/llcommon/lleventfilter.h index ccbbc8bd25..ff8fc9bc7f 100644 --- a/indra/llcommon/lleventfilter.h +++ b/indra/llcommon/lleventfilter.h @@ -336,11 +336,11 @@ public: LLEventThrottle(LLEventPump& source, F32 interval); private: - virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) override; - virtual bool alarmRunning() const override; - virtual void alarmCancel() override; - virtual void timerSet(F32 interval) override; - virtual F32 timerGetRemaining() const override; + virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) /*override*/; + virtual bool alarmRunning() const /*override*/; + virtual void alarmCancel() /*override*/; + virtual void timerSet(F32 interval) /*override*/; + virtual F32 timerGetRemaining() const /*override*/; // use this to arrange a deferred flush() call LLEventTimeout mAlarm; diff --git a/indra/llcommon/tests/lleventfilter_test.cpp b/indra/llcommon/tests/lleventfilter_test.cpp index 712864bf63..eb98b12ef5 100644 --- a/indra/llcommon/tests/lleventfilter_test.cpp +++ b/indra/llcommon/tests/lleventfilter_test.cpp @@ -88,29 +88,29 @@ public: {} /*----- implementation of LLEventThrottleBase timing functionality -----*/ - virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) override + virtual void alarmActionAfter(F32 interval, const LLEventTimeoutBase::Action& action) /*override*/ { mAlarmRemaining = interval; mAlarmAction = action; } - virtual bool alarmRunning() const override + virtual bool alarmRunning() const /*override*/ { // decrementing to exactly 0 should mean the alarm fires return mAlarmRemaining > 0; } - virtual void alarmCancel() override + virtual void alarmCancel() /*override*/ { mAlarmRemaining = -1; } - virtual void timerSet(F32 interval) override + virtual void timerSet(F32 interval) /*override*/ { mTimerRemaining = interval; } - virtual F32 timerGetRemaining() const override + virtual F32 timerGetRemaining() const /*override*/ { // LLTimer.getRemainingTimeF32() never returns negative; 0.0 means expired return (mTimerRemaining > 0.0)? mTimerRemaining : 0.0; -- cgit v1.2.3 From f2780a93c082e9c9a9557b7b0f6378980632b634 Mon Sep 17 00:00:00 2001 From: AndreyL ProductEngine Date: Wed, 7 Jun 2017 03:16:56 +0300 Subject: MAINT-6697 Added a nullcheck to unzip_llsd() --- indra/llcommon/llsdserialize.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 81ba8631c6..41cdb14886 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2175,6 +2175,14 @@ bool unzip_llsd(LLSD& data, std::istream& is, S32 size) U32 have = CHUNK-strm.avail_out; result = (U8*) realloc(result, cur_size + have); + if (result == NULL) + { + LL_WARNS() << "Failed to unzip LLSD block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; + inflateEnd(&strm); + free(result); + delete[] in; + return false; + } memcpy(result+cur_size, out, have); cur_size += have; -- cgit v1.2.3 From d9fe21f17f8c392a602773fa36b0814a0c672761 Mon Sep 17 00:00:00 2001 From: AndreyL ProductEngine Date: Wed, 7 Jun 2017 19:30:32 +0300 Subject: MAINT-6697 More nullchecks for zip/unzip functions --- indra/llcommon/llsdserialize.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 41cdb14886..0568a639a0 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2092,6 +2092,12 @@ std::string zip_llsd(LLSD& data) have = CHUNK-strm.avail_out; output = (U8*) realloc(output, cur_size+have); + if (output == NULL) + { + LL_WARNS() << "Failed to compress LLSD block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; + deflateEnd(&strm); + return std::string(); + } memcpy(output+cur_size, out, have); cur_size += have; } @@ -2179,7 +2185,6 @@ bool unzip_llsd(LLSD& data, std::istream& is, S32 size) { LL_WARNS() << "Failed to unzip LLSD block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; inflateEnd(&strm); - free(result); delete[] in; return false; } @@ -2275,6 +2280,14 @@ U8* unzip_llsdNavMesh( bool& valid, unsigned int& outsize, std::istream& is, S32 U32 have = CHUNK-strm.avail_out; result = (U8*) realloc(result, cur_size + have); + if (result == NULL) + { + LL_WARNS() << "Failed to unzip LLSD NavMesh block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; + inflateEnd(&strm); + delete[] in; + valid = false; + return NULL; + } memcpy(result+cur_size, out, have); cur_size += have; -- cgit v1.2.3 From 7cc9455fe1b8a6194f52062c90761fffa6ae6fdc Mon Sep 17 00:00:00 2001 From: AndreyL ProductEngine Date: Wed, 7 Jun 2017 23:05:37 +0300 Subject: MAINT-6697 Correct pointer freeing --- indra/llcommon/llsdserialize.cpp | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsdserialize.cpp b/indra/llcommon/llsdserialize.cpp index 0568a639a0..3a219eb998 100644 --- a/indra/llcommon/llsdserialize.cpp +++ b/indra/llcommon/llsdserialize.cpp @@ -2091,13 +2091,18 @@ std::string zip_llsd(LLSD& data) } have = CHUNK-strm.avail_out; - output = (U8*) realloc(output, cur_size+have); - if (output == NULL) + U8* new_output = (U8*) realloc(output, cur_size+have); + if (new_output == NULL) { LL_WARNS() << "Failed to compress LLSD block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; deflateEnd(&strm); + if (output) + { + free(output); + } return std::string(); } + output = new_output; memcpy(output+cur_size, out, have); cur_size += have; } @@ -2180,14 +2185,19 @@ bool unzip_llsd(LLSD& data, std::istream& is, S32 size) U32 have = CHUNK-strm.avail_out; - result = (U8*) realloc(result, cur_size + have); - if (result == NULL) + U8* new_result = (U8*)realloc(result, cur_size + have); + if (new_result == NULL) { LL_WARNS() << "Failed to unzip LLSD block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; inflateEnd(&strm); + if (result) + { + free(result); + } delete[] in; return false; } + result = new_result; memcpy(result+cur_size, out, have); cur_size += have; @@ -2279,15 +2289,20 @@ U8* unzip_llsdNavMesh( bool& valid, unsigned int& outsize, std::istream& is, S32 U32 have = CHUNK-strm.avail_out; - result = (U8*) realloc(result, cur_size + have); - if (result == NULL) + U8* new_result = (U8*) realloc(result, cur_size + have); + if (new_result == NULL) { LL_WARNS() << "Failed to unzip LLSD NavMesh block: can't reallocate memory, current size: " << cur_size << " bytes; requested " << cur_size + have << " bytes." << LL_ENDL; inflateEnd(&strm); + if (result) + { + free(result); + } delete[] in; valid = false; return NULL; } + result = new_result; memcpy(result+cur_size, out, have); cur_size += have; -- cgit v1.2.3 From 65208b7741c1d842b5a5c6822a56cb9d34158dea Mon Sep 17 00:00:00 2001 From: Mnikolenko Productengine Date: Fri, 16 Jun 2017 17:46:12 +0300 Subject: MAINT-7488 FIXED [Windows] Viewer crashes when pasting empty string from clipboard --- indra/llcommon/llstring.h | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llstring.h b/indra/llcommon/llstring.h index 2255e638c2..abe5fda603 100644 --- a/indra/llcommon/llstring.h +++ b/indra/llcommon/llstring.h @@ -1325,6 +1325,10 @@ void LLStringUtilBase::removeCRLF(string_type& string) template void LLStringUtilBase::removeWindowsCR(string_type& string) { + if (string.empty()) + { + return; + } const T LF = 10; const T CR = 13; -- cgit v1.2.3 From ab26a700474aa5480d678da67543c9d0f31bb52a Mon Sep 17 00:00:00 2001 From: Mnikolenko Productengine Date: Thu, 13 Jul 2017 11:01:20 +0300 Subject: MAINT-7593 FIXED "Failed to parse parameter" spam --- indra/llcommon/llinitparam.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llinitparam.cpp b/indra/llcommon/llinitparam.cpp index 1d104cf55d..aa2f4eb289 100644 --- a/indra/llcommon/llinitparam.cpp +++ b/indra/llcommon/llinitparam.cpp @@ -193,12 +193,7 @@ namespace LLInitParam { if (!silent) { - std::string file_name = p.getCurrentFileName(); - if(!file_name.empty()) - { - file_name = "in file: " + file_name; - } - p.parserWarning(llformat("Failed to parse parameter \"%s\" %s", p.getCurrentElementName().c_str(), file_name.c_str())); + p.parserWarning(llformat("Failed to parse parameter \"%s\"", p.getCurrentElementName().c_str())); } return false; } -- cgit v1.2.3 From c21b3bbaccdad847611c5af78f612a3db2f47cc1 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Fri, 25 Aug 2017 20:26:25 +0300 Subject: MAINT-7739 Make LLOSInfo a Singleton --- indra/llcommon/llsys.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llsys.h b/indra/llcommon/llsys.h index 962367f69f..294d0066ca 100644 --- a/indra/llcommon/llsys.h +++ b/indra/llcommon/llsys.h @@ -37,13 +37,14 @@ // #include "llsd.h" +#include "llsingleton.h" #include #include -class LL_COMMON_API LLOSInfo +class LL_COMMON_API LLOSInfo : public LLSingleton { + LLSINGLETON(LLOSInfo); public: - LLOSInfo(); void stream(std::ostream& s) const; const std::string& getOSString() const; -- cgit v1.2.3 From f8254a9d787ab6235c8fb076bd65dc7cd978dce9 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Wed, 30 Aug 2017 19:57:02 +0300 Subject: MAINT-7758 Fixed freeze on loading lsl scripts from unicode named windows folder. --- indra/llcommon/llmetricperformancetester.cpp | 14 +++++++------- indra/llcommon/llmetricperformancetester.h | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llmetricperformancetester.cpp b/indra/llcommon/llmetricperformancetester.cpp index 16fc365da1..f8a93baf45 100644 --- a/indra/llcommon/llmetricperformancetester.cpp +++ b/indra/llcommon/llmetricperformancetester.cpp @@ -132,8 +132,8 @@ void LLMetricPerformanceTesterBasic::doAnalysisMetrics(std::string baseline, std } // Open baseline and current target, exit if one is inexistent - std::ifstream base_is(baseline.c_str()); - std::ifstream target_is(target.c_str()); + llifstream base_is(baseline.c_str()); + llifstream target_is(target.c_str()); if (!base_is.is_open() || !target_is.is_open()) { LL_WARNS() << "'-analyzeperformance' error : baseline or current target file inexistent" << LL_ENDL; @@ -151,7 +151,7 @@ void LLMetricPerformanceTesterBasic::doAnalysisMetrics(std::string baseline, std target_is.close(); //output comparision - std::ofstream os(output.c_str()); + llofstream os(output.c_str()); os << "Label, Metric, Base(B), Target(T), Diff(T-B), Percentage(100*T/B)\n"; for(LLMetricPerformanceTesterBasic::name_tester_map_t::iterator iter = LLMetricPerformanceTesterBasic::sTesterMap.begin() ; @@ -212,7 +212,7 @@ void LLMetricPerformanceTesterBasic::addMetric(std::string str) } /*virtual*/ -void LLMetricPerformanceTesterBasic::analyzePerformance(std::ofstream* os, LLSD* base, LLSD* current) +void LLMetricPerformanceTesterBasic::analyzePerformance(llofstream* os, LLSD* base, LLSD* current) { resetCurrentCount() ; @@ -254,14 +254,14 @@ void LLMetricPerformanceTesterBasic::analyzePerformance(std::ofstream* os, LLSD* } /*virtual*/ -void LLMetricPerformanceTesterBasic::compareTestResults(std::ofstream* os, std::string metric_string, S32 v_base, S32 v_current) +void LLMetricPerformanceTesterBasic::compareTestResults(llofstream* os, std::string metric_string, S32 v_base, S32 v_current) { *os << llformat(" ,%s, %d, %d, %d, %.4f\n", metric_string.c_str(), v_base, v_current, v_current - v_base, (v_base != 0) ? 100.f * v_current / v_base : 0) ; } /*virtual*/ -void LLMetricPerformanceTesterBasic::compareTestResults(std::ofstream* os, std::string metric_string, F32 v_base, F32 v_current) +void LLMetricPerformanceTesterBasic::compareTestResults(llofstream* os, std::string metric_string, F32 v_base, F32 v_current) { *os << llformat(" ,%s, %.4f, %.4f, %.4f, %.4f\n", metric_string.c_str(), v_base, v_current, v_current - v_base, (fabs(v_base) > 0.0001f) ? 100.f * v_current / v_base : 0.f ) ; @@ -293,7 +293,7 @@ LLMetricPerformanceTesterWithSession::~LLMetricPerformanceTesterWithSession() } /*virtual*/ -void LLMetricPerformanceTesterWithSession::analyzePerformance(std::ofstream* os, LLSD* base, LLSD* current) +void LLMetricPerformanceTesterWithSession::analyzePerformance(llofstream* os, LLSD* base, LLSD* current) { // Load the base session resetCurrentCount() ; diff --git a/indra/llcommon/llmetricperformancetester.h b/indra/llcommon/llmetricperformancetester.h index e6b46be1cf..2e99ed979d 100644 --- a/indra/llcommon/llmetricperformancetester.h +++ b/indra/llcommon/llmetricperformancetester.h @@ -60,7 +60,7 @@ public: * By default, compares the test results against the baseline one by one, item by item, * in the increasing order of the LLSD record counter, starting from the first one. */ - virtual void analyzePerformance(std::ofstream* os, LLSD* base, LLSD* current) ; + virtual void analyzePerformance(llofstream* os, LLSD* base, LLSD* current) ; static void doAnalysisMetrics(std::string baseline, std::string target, std::string output) ; @@ -93,8 +93,8 @@ protected: * @param[in] v_base - Base value of the metric. * @param[in] v_current - Current value of the metric. */ - virtual void compareTestResults(std::ofstream* os, std::string metric_string, S32 v_base, S32 v_current) ; - virtual void compareTestResults(std::ofstream* os, std::string metric_string, F32 v_base, F32 v_current) ; + virtual void compareTestResults(llofstream* os, std::string metric_string, S32 v_base, S32 v_current) ; + virtual void compareTestResults(llofstream* os, std::string metric_string, F32 v_base, F32 v_current) ; /** * @brief Reset internal record count. Count starts with 1. @@ -181,7 +181,7 @@ public: * This will be loading the base and current sessions and compare them using the virtual * abstract methods loadTestSession() and compareTestSessions() */ - virtual void analyzePerformance(std::ofstream* os, LLSD* base, LLSD* current) ; + virtual void analyzePerformance(llofstream* os, LLSD* base, LLSD* current) ; protected: /** @@ -205,7 +205,7 @@ protected: * @brief Compare the base session and the target session. Assumes base and current sessions have been loaded. * @param[out] os - The comparison result as a standard stream */ - virtual void compareTestSessions(std::ofstream* os) = 0; + virtual void compareTestSessions(llofstream* os) = 0; LLTestSession* mBaseSessionp; LLTestSession* mCurrentSessionp; -- cgit v1.2.3 From 4a4d93d8c0c45248f76d6f27fb523b8ad361a473 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Wed, 6 Sep 2017 16:04:59 +0300 Subject: MAINT-7691 Fixed crash report not generating files in unicode named folders --- indra/llcommon/llapp.cpp | 62 ++++++++++++++++++++++++++++++++++++++++++++---- indra/llcommon/llapp.h | 16 ++++++++++++- 2 files changed, 73 insertions(+), 5 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 2c76f29020..55138e48ee 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -256,6 +256,60 @@ bool LLApp::parseCommandOptions(int argc, char** argv) return true; } +bool LLApp::parseCommandOptions(int argc, wchar_t** wargv) +{ + LLSD commands; + std::string name; + std::string value; + for(int ii = 1; ii < argc; ++ii) + { + if(wargv[ii][0] != '-') + { + LL_INFOS() << "Did not find option identifier while parsing token: " + << wargv[ii] << LL_ENDL; + return false; + } + int offset = 1; + if(wargv[ii][1] == '-') ++offset; + name.assign(utf16str_to_utf8str(&wargv[ii][offset])); + if(((ii+1) >= argc) || (wargv[ii+1][0] == '-')) + { + // we found another option after this one or we have + // reached the end. simply record that this option was + // found and continue. + int flag = name.compare("logfile"); + if (0 == flag) + { + commands[name] = "log"; + } + else + { + commands[name] = true; + } + + continue; + } + ++ii; + value.assign(utf16str_to_utf8str(wargv[ii])); + +#if LL_WINDOWS + //Windows changed command line parsing. Deal with it. + S32 slen = value.length() - 1; + S32 start = 0; + S32 end = slen; + if (wargv[ii][start]=='"')start++; + if (wargv[ii][end]=='"')end--; + if (start!=0 || end!=slen) + { + value = value.substr (start,end); + } +#endif + + commands[name] = value; + } + setOptionData(PRIORITY_COMMAND_LINE, commands); + return true; +} void LLApp::manageLiveFile(LLLiveFile* livefile) { @@ -354,7 +408,7 @@ void LLApp::setupErrorHandling(bool second_instance) std::wstring wpipe_name; wpipe_name = mCrashReportPipeStr + wstringize(getPid()); - const std::wstring wdump_path(wstringize(mDumpPath)); + const std::wstring wdump_path(utf8str_to_utf16str(mDumpPath)); int retries = 30; for (; retries > 0; --retries) @@ -515,9 +569,9 @@ void LLApp::setMiniDumpDir(const std::string &path) if(mExceptionHandler == 0) return; #ifdef LL_WINDOWS - wchar_t buffer[MAX_MINDUMP_PATH_LENGTH]; - mbstowcs(buffer, mDumpPath.c_str(), MAX_MINDUMP_PATH_LENGTH); - mExceptionHandler->set_dump_path(std::wstring(buffer)); + std::wstring buffer(utf8str_to_utf16str(mDumpPath)); + if (buffer.size() > MAX_MINDUMP_PATH_LENGTH) buffer.resize(MAX_MINDUMP_PATH_LENGTH); + mExceptionHandler->set_dump_path(buffer); #elif LL_LINUX //google_breakpad::MinidumpDescriptor desc("/tmp"); //path works in debug fails in production inside breakpad lib so linux gets a little less stack reporting until it is patched. google_breakpad::MinidumpDescriptor desc(mDumpPath); //path works in debug fails in production inside breakpad lib so linux gets a little less stack reporting until it is patched. diff --git a/indra/llcommon/llapp.h b/indra/llcommon/llapp.h index ff9a92b45f..acd829d864 100644 --- a/indra/llcommon/llapp.h +++ b/indra/llcommon/llapp.h @@ -106,7 +106,7 @@ public: LLSD getOption(const std::string& name) const; /** - * @brief Parse command line options and insert them into + * @brief Parse ASCII command line options and insert them into * application command line options. * * The name inserted into the option will have leading option @@ -119,6 +119,20 @@ public: */ bool parseCommandOptions(int argc, char** argv); + /** + * @brief Parse Unicode command line options and insert them into + * application command line options. + * + * The name inserted into the option will have leading option + * identifiers (a minus or double minus) stripped. All options + * with values will be stored as a string, while all options + * without values will be stored as true. + * @param argc The argc passed into main(). + * @param wargv The wargv passed into main(). + * @return Returns true if the parse succeeded. + */ + bool parseCommandOptions(int argc, wchar_t** wargv); + /** * @brief Keep track of live files automatically. * -- cgit v1.2.3 From e6f3cfe247912b27e9c8087da631a2f9d20b56b4 Mon Sep 17 00:00:00 2001 From: Mnikolenko Productengine Date: Thu, 7 Sep 2017 13:08:26 +0300 Subject: mac and linux build fix --- indra/llcommon/llapp.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 55138e48ee..6cc9e804d4 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -271,7 +271,12 @@ bool LLApp::parseCommandOptions(int argc, wchar_t** wargv) } int offset = 1; if(wargv[ii][1] == '-') ++offset; - name.assign(utf16str_to_utf8str(&wargv[ii][offset])); + +#if LL_WINDOWS + name.assign(utf16str_to_utf8str(&wargv[ii][offset])); +#else + name.assign(wstring_to_utf8str(&wargv[ii][offset])); +#endif if(((ii+1) >= argc) || (wargv[ii+1][0] == '-')) { // we found another option after this one or we have @@ -290,7 +295,12 @@ bool LLApp::parseCommandOptions(int argc, wchar_t** wargv) continue; } ++ii; - value.assign(utf16str_to_utf8str(wargv[ii])); + +#if LL_WINDOWS + value.assign(utf16str_to_utf8str((wargv[ii]))); +#else + value.assign(wstring_to_utf8str((wargv[ii]))); +#endif #if LL_WINDOWS //Windows changed command line parsing. Deal with it. -- cgit v1.2.3 From a2f71a242a868cf961c0aba52ec9b644742cd917 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Tue, 19 Sep 2017 16:36:00 +0300 Subject: MAINT-7820 Fixed crash in LLEventPump --- indra/llcommon/llevents.cpp | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 97270e4931..93db6c0d17 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -316,6 +316,13 @@ LLBoundListener LLEventPump::listen_impl(const std::string& name, const LLEventL const NameList& after, const NameList& before) { + if (!mSignal) + { + LL_WARNS() << "Can't connect listener" << LL_ENDL; + // connect will fail, return dummy + return LLBoundListener(); + } + float nodePosition = 1.0; // if the supplied name is empty we are not interested in the ordering mechanism -- cgit v1.2.3 From 7d393759bae8028be8e2ca2d9e09b7b90a0d9ed5 Mon Sep 17 00:00:00 2001 From: andreykproductengine Date: Thu, 5 Oct 2017 16:03:49 +0300 Subject: MAINT-7820 Additional crash logging --- indra/llcommon/llcoros.cpp | 45 +++++++++++++++++++++++++++++++++++++++++++++ indra/llcommon/llcoros.h | 3 +++ 2 files changed, 48 insertions(+) (limited to 'indra/llcommon') diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp index 3ffce4810a..934f04287d 100644 --- a/indra/llcommon/llcoros.cpp +++ b/indra/llcommon/llcoros.cpp @@ -40,6 +40,10 @@ #include "stringize.h" #include "llexception.h" +#if LL_WINDOWS +#include +#endif + namespace { void no_op() {} } // anonymous namespace @@ -272,6 +276,43 @@ void LLCoros::setStackSize(S32 stacksize) mStackSize = stacksize; } +#if LL_WINDOWS + +static const U32 STATUS_MSC_EXCEPTION = 0xE06D7363; // compiler specific + +U32 exception_filter(U32 code, struct _EXCEPTION_POINTERS *exception_infop) +{ + if (code == STATUS_MSC_EXCEPTION) + { + // C++ exception, go on + return EXCEPTION_CONTINUE_SEARCH; + } + else + { + // handle it + return EXCEPTION_EXECUTE_HANDLER; + } +} + +void LLCoros::winlevel(const callable_t& callable) +{ + __try + { + callable(); + } + __except (exception_filter(GetExceptionCode(), GetExceptionInformation())) + { + // convert to C++ styled exception + // Note: it might be better to use _se_set_translator + // if you want exception to inherit full callstack + char integer_string[32]; + sprintf(integer_string, "SEH, code: %lu\n", GetExceptionCode()); + throw std::exception(integer_string); + } +} + +#endif + // 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. @@ -282,7 +323,11 @@ void LLCoros::toplevel(coro::self& self, CoroData* data, const callable_t& calla // run the code the caller actually wants in the coroutine try { +#if LL_WINDOWS + winlevel(callable); +#else callable(); +#endif } catch (const LLContinueError&) { diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h index bbe2d22af4..884d6b159c 100644 --- a/indra/llcommon/llcoros.h +++ b/indra/llcommon/llcoros.h @@ -182,6 +182,9 @@ private: bool cleanup(const LLSD&); 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); -- cgit v1.2.3