diff options
Diffstat (limited to 'indra/llcommon')
| -rw-r--r-- | indra/llcommon/lleventtimer.cpp | 3 | ||||
| -rw-r--r-- | indra/llcommon/llfasttimer.cpp | 31 | ||||
| -rw-r--r-- | indra/llcommon/llinstancetracker.cpp | 17 | ||||
| -rw-r--r-- | indra/llcommon/llinstancetracker.h | 706 | ||||
| -rw-r--r-- | indra/llcommon/llleaplistener.cpp | 8 | ||||
| -rw-r--r-- | indra/llcommon/llthreadlocalstorage.cpp | 12 | ||||
| -rw-r--r-- | indra/llcommon/lltrace.h | 2 | ||||
| -rw-r--r-- | indra/llcommon/lltracethreadrecorder.cpp | 12 | ||||
| -rw-r--r-- | indra/llcommon/tests/llinstancetracker_test.cpp | 107 | ||||
| -rw-r--r-- | indra/llcommon/tests/llleap_test.cpp | 28 |
10 files changed, 499 insertions, 427 deletions
diff --git a/indra/llcommon/lleventtimer.cpp b/indra/llcommon/lleventtimer.cpp index 0d96e03da4..3986dee3ac 100644 --- a/indra/llcommon/lleventtimer.cpp +++ b/indra/llcommon/lleventtimer.cpp @@ -58,9 +58,8 @@ LLEventTimer::~LLEventTimer() void LLEventTimer::updateClass() { std::list<LLEventTimer*> completed_timers; - for (instance_iter iter = beginInstances(); iter != endInstances(); ) + for (auto& timer : instance_snapshot()) { - LLEventTimer& timer = *iter++; F32 et = timer.mEventTimer.getElapsedTimeF32(); if (timer.mEventTimer.getStarted() && et > timer.mPeriod) { timer.mEventTimer.reset(); diff --git a/indra/llcommon/llfasttimer.cpp b/indra/llcommon/llfasttimer.cpp index 3d28cd15b0..08ea668964 100644 --- a/indra/llcommon/llfasttimer.cpp +++ b/indra/llcommon/llfasttimer.cpp @@ -193,27 +193,26 @@ TimeBlockTreeNode& BlockTimerStatHandle::getTreeNode() const void BlockTimer::bootstrapTimerTree() { - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& timer = static_cast<BlockTimerStatHandle&>(*it); + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& timer = static_cast<BlockTimerStatHandle&>(base); if (&timer == &BlockTimer::getRootTimeBlock()) continue; // bootstrap tree construction by attaching to last timer to be on stack // when this timer was called if (timer.getParent() == &BlockTimer::getRootTimeBlock()) -{ + { TimeBlockAccumulator& accumulator = timer.getCurrentAccumulator(); if (accumulator.mLastCaller) - { + { timer.setParent(accumulator.mLastCaller); accumulator.mParent = accumulator.mLastCaller; - } + } // no need to push up tree on first use, flag can be set spuriously accumulator.mMoveUpTree = false; - } + } } } @@ -306,12 +305,10 @@ void BlockTimer::processTimes() updateTimes(); // reset for next frame - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), - end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& timer = static_cast<BlockTimerStatHandle&>(*it); + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& timer = static_cast<BlockTimerStatHandle&>(base); TimeBlockAccumulator& accumulator = timer.getCurrentAccumulator(); accumulator.mLastCaller = NULL; @@ -362,12 +359,10 @@ void BlockTimer::logStats() LLSD sd; { - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), - end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& timer = static_cast<BlockTimerStatHandle&>(*it); + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& timer = static_cast<BlockTimerStatHandle&>(base); LLTrace::PeriodicRecording& frame_recording = LLTrace::get_frame_recording(); sd[timer.getName()]["Time"] = (LLSD::Real) (frame_recording.getLastRecording().getSum(timer).value()); sd[timer.getName()]["Calls"] = (LLSD::Integer) (frame_recording.getLastRecording().getSum(timer.callCount())); diff --git a/indra/llcommon/llinstancetracker.cpp b/indra/llcommon/llinstancetracker.cpp index 3f990f4869..accb4286e8 100644 --- a/indra/llcommon/llinstancetracker.cpp +++ b/indra/llcommon/llinstancetracker.cpp @@ -34,18 +34,5 @@ // external library headers // other Linden headers -void LLInstanceTrackerBase::StaticBase::incrementDepth() -{ - ++sIterationNestDepth; -} - -void LLInstanceTrackerBase::StaticBase::decrementDepth() -{ - llassert(sIterationNestDepth); - --sIterationNestDepth; -} - -U32 LLInstanceTrackerBase::StaticBase::getDepth() -{ - return sIterationNestDepth; -} +// This .cpp file is required by our CMake test macro. It contributes no code +// to the viewer. diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h index 363d0bcbd5..76b201ad8c 100644 --- a/indra/llcommon/llinstancetracker.h +++ b/indra/llcommon/llinstancetracker.h @@ -28,354 +28,456 @@ #ifndef LL_LLINSTANCETRACKER_H #define LL_LLINSTANCETRACKER_H -#include <atomic> #include <map> +#include <set> +#include <vector> #include <typeinfo> +#include <mutex> +#include <memory> +#include <type_traits> -#include "llstringtable.h" #include <boost/iterator/transform_iterator.hpp> #include <boost/iterator/indirect_iterator.hpp> +#include <boost/iterator/filter_iterator.hpp> -// As of 2017-05-06, as far as nat knows, only clang supports __has_feature(). -// Unfortunately VS2013's preprocessor shortcut logic doesn't prevent it from -// producing (fatal) warnings for defined(__clang__) && __has_feature(...). -// Have to work around that. -#if ! defined(__clang__) -#define __has_feature(x) 0 -#endif // __clang__ - -#if defined(LL_TEST_llinstancetracker) && __has_feature(cxx_noexcept) -// ~LLInstanceTracker() performs llassert_always() validation. That's fine in -// production code, since the llassert_always() is implemented as an LL_ERRS -// message, which will crash-with-message. In our integration test executable, -// though, this llassert_always() throws an exception instead so we can test -// error conditions and continue running the test. However -- as of C++11, -// destructors are implicitly noexcept(true). Unless we mark -// ~LLInstanceTracker() noexcept(false), the test executable crashes even on -// the ATTEMPT to throw. -#define LLINSTANCETRACKER_DTOR_NOEXCEPT noexcept(false) -#else -// If we're building for production, or in fact building *any other* test, or -// we're using a compiler that doesn't support __has_feature(), or we're not -// compiling with a C++ version that supports noexcept -- don't specify it. -#define LLINSTANCETRACKER_DTOR_NOEXCEPT -#endif - +/***************************************************************************** +* LLInstanceTrackerBase +*****************************************************************************/ /** * Base class manages "class-static" data that must actually have singleton * semantics: one instance per process, rather than one instance per module as * sometimes happens with data simply declared static. */ +namespace LLInstanceTrackerStuff +{ + struct StaticBase + { + // We need to be able to lock static data while manipulating it. + typedef std::mutex mutex_t; + mutex_t mMutex; + }; +} // namespace LLInstanceTrackerStuff + +template <class Static> class LL_COMMON_API LLInstanceTrackerBase { protected: - /// It's not essential to derive your STATICDATA (for use with - /// getStatic()) from StaticBase; it's just that both known - /// implementations do. - struct StaticBase + typedef Static StaticData; + + // Instantiate this class to obtain a pointer to the canonical static + // instance of class Static while holding a lock on that instance. Use of + // Static::mMutex presumes either that Static is derived from StaticBase, + // or that Static declares some other suitable mMutex. + class LockStatic { - StaticBase(): - sIterationNestDepth(0) + typedef std::unique_lock<decltype(Static::mMutex)> lock_t; + public: + LockStatic(): + mData(getStatic()), + mLock(mData->mMutex) {} - - void incrementDepth(); - void decrementDepth(); - U32 getDepth(); - private: -#ifdef LL_WINDOWS - std::atomic_uint32_t sIterationNestDepth; -#else - std::atomic_uint sIterationNestDepth; -#endif - }; + Static* get() const { return mData; } + operator Static*() const { return get(); } + Static* operator->() const { return get(); } + // sometimes we must explicitly unlock... + void unlock() + { + // but once we do, access is no longer permitted + mData = nullptr; + mLock.unlock(); + } + protected: + Static* mData; + lock_t mLock; + private: + Static* getStatic() + { + static Static sData; + return &sData; + } + }; }; -LL_COMMON_API void assert_main_thread(); - +/***************************************************************************** +* LLInstanceTracker with key +*****************************************************************************/ enum EInstanceTrackerAllowKeyCollisions { - LLInstanceTrackerErrorOnCollision, - LLInstanceTrackerReplaceOnCollision + LLInstanceTrackerErrorOnCollision, + LLInstanceTrackerReplaceOnCollision }; +namespace LLInstanceTrackerStuff +{ + template <typename KEY, typename VALUE> + struct StaticMap: public StaticBase + { + typedef std::map<KEY, VALUE> InstanceMap; + InstanceMap mMap; + }; +} // LLInstanceTrackerStuff + /// This mix-in class adds support for tracking all instances of the specified class parameter T /// The (optional) key associates a value of type KEY with a given instance of T, for quick lookup /// If KEY is not provided, then instances are stored in a simple set /// @NOTE: see explicit specialization below for default KEY==void case -/// @NOTE: this class is not thread-safe unless used as read-only -template<typename T, typename KEY = void, EInstanceTrackerAllowKeyCollisions KEY_COLLISION_BEHAVIOR = LLInstanceTrackerErrorOnCollision> -class LLInstanceTracker : public LLInstanceTrackerBase +template<typename T, typename KEY = void, + EInstanceTrackerAllowKeyCollisions KEY_COLLISION_BEHAVIOR = LLInstanceTrackerErrorOnCollision> +class LLInstanceTracker : + public LLInstanceTrackerBase<LLInstanceTrackerStuff::StaticMap<KEY, std::shared_ptr<T>>> { - typedef LLInstanceTracker<T, KEY> self_t; - typedef typename std::multimap<KEY, T*> InstanceMap; - struct StaticData: public StaticBase - { - InstanceMap sMap; - }; - static StaticData& getStatic() { static StaticData sData; return sData;} - static InstanceMap& getMap_() { return getStatic().sMap; } + typedef LLInstanceTrackerBase<LLInstanceTrackerStuff::StaticMap<KEY, std::shared_ptr<T>>> super; + using typename super::StaticData; + using typename super::LockStatic; + typedef typename StaticData::InstanceMap InstanceMap; public: - class instance_iter : public boost::iterator_facade<instance_iter, T, boost::forward_traversal_tag> - { - public: - typedef boost::iterator_facade<instance_iter, T, boost::forward_traversal_tag> super_t; - - instance_iter(const typename InstanceMap::iterator& it) - : mIterator(it) - { - getStatic().incrementDepth(); - } - - ~instance_iter() - { - getStatic().decrementDepth(); - } - - - private: - friend class boost::iterator_core_access; - - void increment() { mIterator++; } - bool equal(instance_iter const& other) const - { - return mIterator == other.mIterator; - } - - T& dereference() const - { - return *(mIterator->second); - } - - typename InstanceMap::iterator mIterator; - }; - - class key_iter : public boost::iterator_facade<key_iter, KEY, boost::forward_traversal_tag> - { - public: - typedef boost::iterator_facade<key_iter, KEY, boost::forward_traversal_tag> super_t; - - key_iter(typename InstanceMap::iterator it) - : mIterator(it) - { - getStatic().incrementDepth(); - } - - key_iter(const key_iter& other) - : mIterator(other.mIterator) - { - getStatic().incrementDepth(); - } - - ~key_iter() - { - getStatic().decrementDepth(); - } - - - private: - friend class boost::iterator_core_access; - - void increment() { mIterator++; } - bool equal(key_iter const& other) const - { - return mIterator == other.mIterator; - } - - KEY& dereference() const - { - return const_cast<KEY&>(mIterator->first); - } - - typename InstanceMap::iterator mIterator; - }; - - static T* getInstance(const KEY& k) - { - const InstanceMap& map(getMap_()); - typename InstanceMap::const_iterator found = map.find(k); - return (found == map.end()) ? NULL : found->second; - } - - static instance_iter beginInstances() - { - return instance_iter(getMap_().begin()); - } - - static instance_iter endInstances() - { - return instance_iter(getMap_().end()); - } - - static S32 instanceCount() - { - return getMap_().size(); - } - - static key_iter beginKeys() - { - return key_iter(getMap_().begin()); - } - static key_iter endKeys() - { - return key_iter(getMap_().end()); - } + // snapshot of std::pair<const KEY, std::shared_ptr<T>> pairs + class snapshot + { + // It's very important that what we store in this snapshot are + // weak_ptrs, NOT shared_ptrs. That's how we discover whether any + // instance has been deleted during the lifespan of a snapshot. + typedef std::vector<std::pair<const KEY, std::weak_ptr<T>>> VectorType; + // Dereferencing our iterator produces a std::shared_ptr for each + // instance that still exists. Since we store weak_ptrs, that involves + // two chained transformations: + // - a transform_iterator to lock the weak_ptr and return a shared_ptr + // - a filter_iterator to skip any shared_ptr that has become invalid. + // It is very important that we filter lazily, that is, during + // traversal. Any one of our stored weak_ptrs might expire during + // traversal. + typedef std::pair<const KEY, std::shared_ptr<T>> strong_pair; + // Note for future reference: nat has not yet had any luck (up to + // Boost 1.67) trying to use boost::transform_iterator with a hand- + // coded functor, only with actual functions. In my experience, an + // internal boost::result_of() operation fails, even with an explicit + // result_type typedef. But this works. + static strong_pair strengthen(typename VectorType::value_type& pair) + { + return { pair.first, pair.second.lock() }; + } + static bool dead_skipper(const strong_pair& pair) + { + return bool(pair.second); + } + + public: + snapshot(): + // populate our vector with a snapshot of (locked!) InstanceMap + // note, this assigns pair<KEY, shared_ptr> to pair<KEY, weak_ptr> + mData(mLock->mMap.begin(), mLock->mMap.end()) + { + // release the lock once we've populated mData + mLock.unlock(); + } + + // You can't make a transform_iterator (or anything else) that + // literally stores a C++ function (decltype(strengthen)) -- but you + // can make a transform_iterator based on a _function pointer._ + typedef boost::transform_iterator<decltype(strengthen)*, + typename VectorType::iterator> strong_iterator; + typedef boost::filter_iterator<decltype(dead_skipper)*, strong_iterator> iterator; + + iterator begin() { return make_iterator(mData.begin()); } + iterator end() { return make_iterator(mData.end()); } + + private: + iterator make_iterator(typename VectorType::iterator iter) + { + // transform_iterator only needs the base iterator and the transform. + // filter_iterator wants the predicate and both ends of the range. + return iterator(dead_skipper, + strong_iterator(iter, strengthen), + strong_iterator(mData.end(), strengthen)); + } + + LockStatic mLock; // lock static data during construction + VectorType mData; + }; + + // iterate over this for references to each instance + class instance_snapshot: public snapshot + { + private: + static T& instance_getter(typename snapshot::iterator::reference pair) + { + return *pair.second; + } + public: + typedef boost::transform_iterator<decltype(instance_getter)*, + typename snapshot::iterator> iterator; + iterator begin() { return iterator(snapshot::begin(), instance_getter); } + iterator end() { return iterator(snapshot::end(), instance_getter); } + + void deleteAll() + { + for (auto it(snapshot::begin()), end(snapshot::end()); it != end; ++it) + { + delete it->second.get(); + } + } + }; + + // iterate over this for each key + class key_snapshot: public snapshot + { + private: + static KEY key_getter(typename snapshot::iterator::reference pair) + { + return pair.first; + } + public: + typedef boost::transform_iterator<decltype(key_getter)*, + typename snapshot::iterator> iterator; + iterator begin() { return iterator(snapshot::begin(), key_getter); } + iterator end() { return iterator(snapshot::end(), key_getter); } + }; + + static T* getInstance(const KEY& k) + { + LockStatic lock; + const InstanceMap& map(lock->mMap); + typename InstanceMap::const_iterator found = map.find(k); + return (found == map.end()) ? NULL : found->second.get(); + } + + static S32 instanceCount() + { + return LockStatic()->mMap.size(); + } protected: - LLInstanceTracker(const KEY& key) - { - // make sure static data outlives all instances - getStatic(); - add_(key); - } - virtual ~LLInstanceTracker() LLINSTANCETRACKER_DTOR_NOEXCEPT - { - // it's unsafe to delete instances of this type while all instances are being iterated over. - llassert_always(getStatic().getDepth() == 0); - remove_(); - } - virtual void setKey(KEY key) { remove_(); add_(key); } - virtual const KEY& getKey() const { return mInstanceKey; } + LLInstanceTracker(const KEY& key) + { + // We do not intend to manage the lifespan of this object with + // shared_ptr, so give it a no-op deleter. We store shared_ptrs in our + // InstanceMap specifically so snapshot can store weak_ptrs so we can + // detect deletions during traversals. + std::shared_ptr<T> ptr(static_cast<T*>(this), [](T*){}); + LockStatic lock; + add_(lock, key, ptr); + } +public: + virtual ~LLInstanceTracker() + { + LockStatic lock; + remove_(lock); + } +protected: + virtual void setKey(KEY key) + { + LockStatic lock; + // Even though the shared_ptr we store in our map has a no-op deleter + // for T itself, letting the use count decrement to 0 will still + // delete the use-count object. Capture the shared_ptr we just removed + // and re-add it to the map with the new key. + auto ptr = remove_(lock); + add_(lock, key, ptr); + } +public: + virtual const KEY& getKey() const { return mInstanceKey; } private: - LLInstanceTracker( const LLInstanceTracker& ); - const LLInstanceTracker& operator=( const LLInstanceTracker& ); - - void add_(const KEY& key) - { - mInstanceKey = key; - InstanceMap& map = getMap_(); - typename InstanceMap::iterator insertion_point_it = map.lower_bound(key); - if (insertion_point_it != map.end() - && insertion_point_it->first == key) - { // found existing entry with that key - switch(KEY_COLLISION_BEHAVIOR) - { - case LLInstanceTrackerErrorOnCollision: - { - // use assert here instead of LL_ERRS(), otherwise the error will be ignored since this call is made during global object initialization - llassert_always_msg(false, "Instance with this same key already exists!"); - break; - } - case LLInstanceTrackerReplaceOnCollision: - { - // replace pointer, but leave key (should have compared equal anyway) - insertion_point_it->second = static_cast<T*>(this); - break; - } - default: - break; - } - } - else - { // new key - map.insert(insertion_point_it, std::make_pair(key, static_cast<T*>(this))); - } - } - void remove_() - { - InstanceMap& map = getMap_(); - typename InstanceMap::iterator iter = map.find(mInstanceKey); - if (iter != map.end()) - { - map.erase(iter); - } - } + LLInstanceTracker( const LLInstanceTracker& ) = delete; + LLInstanceTracker& operator=( const LLInstanceTracker& ) = delete; + + // for logging + template <typename K> + static K report(K key) { return key; } + static std::string report(const std::string& key) { return "'" + key + "'"; } + static std::string report(const char* key) { return report(std::string(key)); } + + // caller must instantiate LockStatic + void add_(LockStatic& lock, const KEY& key, const std::shared_ptr<T>& ptr) + { + mInstanceKey = key; + InstanceMap& map = lock->mMap; + switch(KEY_COLLISION_BEHAVIOR) + { + case LLInstanceTrackerErrorOnCollision: + { + // map stores shared_ptr to self + auto pair = map.emplace(key, ptr); + if (! pair.second) + { + LL_ERRS("LLInstanceTracker") << "Instance with key " << report(key) + << " already exists!" << LL_ENDL; + } + break; + } + case LLInstanceTrackerReplaceOnCollision: + map[key] = ptr; + break; + default: + break; + } + } + std::shared_ptr<T> remove_(LockStatic& lock) + { + InstanceMap& map = lock->mMap; + typename InstanceMap::iterator iter = map.find(mInstanceKey); + if (iter != map.end()) + { + auto ret = iter->second; + map.erase(iter); + return ret; + } + return {}; + } private: - KEY mInstanceKey; + KEY mInstanceKey; }; +/***************************************************************************** +* LLInstanceTracker without key +*****************************************************************************/ +namespace LLInstanceTrackerStuff +{ + template <typename VALUE> + struct StaticSet: public StaticBase + { + typedef std::set<VALUE> InstanceSet; + InstanceSet mSet; + }; +} // LLInstanceTrackerStuff + +// TODO: +// - For the case of omitted KEY template parameter, consider storing +// std::map<T*, std::shared_ptr<T>> instead of std::set<std::shared_ptr<T>>. +// That might let us share more of the implementation between KEY and +// non-KEY LLInstanceTracker subclasses. +// - Even if not that, consider trying to unify the snapshot implementations. +// The trouble is that the 'iterator' published by each (and by their +// subclasses) must reflect the specific type of the callables that +// distinguish them. (Maybe make instance_snapshot() and key_snapshot() +// factory functions that pass lambdas to a factory function for the generic +// template class?) + /// explicit specialization for default case where KEY is void /// use a simple std::set<T*> template<typename T, EInstanceTrackerAllowKeyCollisions KEY_COLLISION_BEHAVIOR> -class LLInstanceTracker<T, void, KEY_COLLISION_BEHAVIOR> : public LLInstanceTrackerBase +class LLInstanceTracker<T, void, KEY_COLLISION_BEHAVIOR> : + public LLInstanceTrackerBase<LLInstanceTrackerStuff::StaticSet<std::shared_ptr<T>>> { - typedef LLInstanceTracker<T, void> self_t; - typedef typename std::set<T*> InstanceSet; - struct StaticData: public StaticBase - { - InstanceSet sSet; - }; - static StaticData& getStatic() { static StaticData sData; return sData; } - static InstanceSet& getSet_() { return getStatic().sSet; } + typedef LLInstanceTrackerBase<LLInstanceTrackerStuff::StaticSet<std::shared_ptr<T>>> super; + using typename super::StaticData; + using typename super::LockStatic; + typedef typename StaticData::InstanceSet InstanceSet; public: + /** + * Storing a dumb T* somewhere external is a bad idea, since + * LLInstanceTracker subclasses are explicitly destroyed rather than + * managed by smart pointers. It's legal to declare stack instances of an + * LLInstanceTracker subclass. But it's reasonable to store a + * std::weak_ptr<T>, which will become invalid when the T instance is + * destroyed. + */ + std::weak_ptr<T> getWeak() + { + return mSelf; + } + + static S32 instanceCount() { return LockStatic()->mSet.size(); } - /** - * Does a particular instance still exist? Of course, if you already have - * a T* in hand, you need not call getInstance() to @em locate the - * instance -- unlike the case where getInstance() accepts some kind of - * key. Nonetheless this method is still useful to @em validate a - * particular T*, since each instance's destructor removes itself from the - * underlying set. - */ - static T* getInstance(T* k) - { - const InstanceSet& set(getSet_()); - typename InstanceSet::const_iterator found = set.find(k); - return (found == set.end())? NULL : *found; - } - static S32 instanceCount() { return getSet_().size(); } - - class instance_iter : public boost::iterator_facade<instance_iter, T, boost::forward_traversal_tag> - { - public: - instance_iter(const typename InstanceSet::iterator& it) - : mIterator(it) - { - getStatic().incrementDepth(); - } - - instance_iter(const instance_iter& other) - : mIterator(other.mIterator) - { - getStatic().incrementDepth(); - } - - ~instance_iter() - { - getStatic().decrementDepth(); - } - - private: - friend class boost::iterator_core_access; - - void increment() { mIterator++; } - bool equal(instance_iter const& other) const - { - return mIterator == other.mIterator; - } - - T& dereference() const - { - return **mIterator; - } - - typename InstanceSet::iterator mIterator; - }; - - static instance_iter beginInstances() { return instance_iter(getSet_().begin()); } - static instance_iter endInstances() { return instance_iter(getSet_().end()); } + // snapshot of std::shared_ptr<T> pointers + class snapshot + { + // It's very important that what we store in this snapshot are + // weak_ptrs, NOT shared_ptrs. That's how we discover whether any + // instance has been deleted during the lifespan of a snapshot. + typedef std::vector<std::weak_ptr<T>> VectorType; + // Dereferencing our iterator produces a std::shared_ptr for each + // instance that still exists. Since we store weak_ptrs, that involves + // two chained transformations: + // - a transform_iterator to lock the weak_ptr and return a shared_ptr + // - a filter_iterator to skip any shared_ptr that has become invalid. + typedef std::shared_ptr<T> strong_ptr; + static strong_ptr strengthen(typename VectorType::value_type& ptr) + { + return ptr.lock(); + } + static bool dead_skipper(const strong_ptr& ptr) + { + return bool(ptr); + } + + public: + snapshot(): + // populate our vector with a snapshot of (locked!) InstanceSet + // note, this assigns stored shared_ptrs to weak_ptrs for snapshot + mData(mLock->mSet.begin(), mLock->mSet.end()) + { + // release the lock once we've populated mData + mLock.unlock(); + } + + typedef boost::transform_iterator<decltype(strengthen)*, + typename VectorType::iterator> strong_iterator; + typedef boost::filter_iterator<decltype(dead_skipper)*, strong_iterator> iterator; + + iterator begin() { return make_iterator(mData.begin()); } + iterator end() { return make_iterator(mData.end()); } + + private: + iterator make_iterator(typename VectorType::iterator iter) + { + // transform_iterator only needs the base iterator and the transform. + // filter_iterator wants the predicate and both ends of the range. + return iterator(dead_skipper, + strong_iterator(iter, strengthen), + strong_iterator(mData.end(), strengthen)); + } + + LockStatic mLock; // lock static data during construction + VectorType mData; + }; + + // iterate over this for references to each instance + struct instance_snapshot: public snapshot + { + typedef boost::indirect_iterator<typename snapshot::iterator> iterator; + iterator begin() { return iterator(snapshot::begin()); } + iterator end() { return iterator(snapshot::end()); } + + void deleteAll() + { + for (auto it(snapshot::begin()), end(snapshot::end()); it != end; ++it) + { + delete it->get(); + } + } + }; protected: - LLInstanceTracker() - { - // make sure static data outlives all instances - getStatic(); - getSet_().insert(static_cast<T*>(this)); - } - virtual ~LLInstanceTracker() LLINSTANCETRACKER_DTOR_NOEXCEPT - { - // it's unsafe to delete instances of this type while all instances are being iterated over. - llassert_always(getStatic().getDepth() == 0); - getSet_().erase(static_cast<T*>(this)); - } - - LLInstanceTracker(const LLInstanceTracker& other) - { - getSet_().insert(static_cast<T*>(this)); - } + LLInstanceTracker() + { + // Since we do not intend for this shared_ptr to manage lifespan, give + // it a no-op deleter. + std::shared_ptr<T> ptr(static_cast<T*>(this), [](T*){}); + // save corresponding weak_ptr for future reference + mSelf = ptr; + // Also store it in our class-static set to track this instance. + LockStatic()->mSet.emplace(ptr); + } +public: + virtual ~LLInstanceTracker() + { + // convert weak_ptr to shared_ptr because that's what we store in our + // InstanceSet + LockStatic()->mSet.erase(mSelf.lock()); + } +protected: + LLInstanceTracker(const LLInstanceTracker& other): + LLInstanceTracker() + {} + +private: + // Storing a weak_ptr to self is a bit like deriving from + // std::enable_shared_from_this(), except more explicit. + std::weak_ptr<T> mSelf; }; #endif diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index fa5730f112..f50bacb1e8 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -228,13 +228,11 @@ void LLLeapListener::getAPIs(const LLSD& request) const { Response reply(LLSD(), request); - for (LLEventAPI::instance_iter eai(LLEventAPI::beginInstances()), - eaend(LLEventAPI::endInstances()); - eai != eaend; ++eai) + for (auto& ea : LLEventAPI::instance_snapshot()) { LLSD info; - info["desc"] = eai->getDesc(); - reply[eai->getName()] = info; + info["desc"] = ea.getDesc(); + reply[ea.getName()] = info; } } diff --git a/indra/llcommon/llthreadlocalstorage.cpp b/indra/llcommon/llthreadlocalstorage.cpp index 8cef05caac..d8a063e8d5 100644 --- a/indra/llcommon/llthreadlocalstorage.cpp +++ b/indra/llcommon/llthreadlocalstorage.cpp @@ -93,11 +93,9 @@ void LLThreadLocalPointerBase::initAllThreadLocalStorage() { if (!sInitialized) { - for (LLInstanceTracker<LLThreadLocalPointerBase>::instance_iter it = beginInstances(), end_it = endInstances(); - it != end_it; - ++it) + for (auto& base : instance_snapshot()) { - (*it).initStorage(); + base.initStorage(); } sInitialized = true; } @@ -108,11 +106,9 @@ void LLThreadLocalPointerBase::destroyAllThreadLocalStorage() { if (sInitialized) { - //for (LLInstanceTracker<LLThreadLocalPointerBase>::instance_iter it = beginInstances(), end_it = endInstances(); - // it != end_it; - // ++it) + //for (auto& base : instance_snapshot()) //{ - // (*it).destroyStorage(); + // base.destroyStorage(); //} sInitialized = false; } diff --git a/indra/llcommon/lltrace.h b/indra/llcommon/lltrace.h index 79ff55b739..0d0cd6f581 100644 --- a/indra/llcommon/lltrace.h +++ b/indra/llcommon/lltrace.h @@ -57,7 +57,7 @@ class StatBase { public: StatBase(const char* name, const char* description); - virtual ~StatBase() LLINSTANCETRACKER_DTOR_NOEXCEPT {} + virtual ~StatBase() {} virtual const char* getUnitLabel() const; const std::string& getName() const { return mName; } diff --git a/indra/llcommon/lltracethreadrecorder.cpp b/indra/llcommon/lltracethreadrecorder.cpp index 181fc2f058..025dc57044 100644 --- a/indra/llcommon/lltracethreadrecorder.cpp +++ b/indra/llcommon/lltracethreadrecorder.cpp @@ -28,6 +28,7 @@ #include "lltracethreadrecorder.h" #include "llfasttimer.h" #include "lltrace.h" +#include "llstl.h" namespace LLTrace { @@ -64,16 +65,15 @@ void ThreadRecorder::init() activate(&mThreadRecordingBuffers); // initialize time block parent pointers - for (BlockTimerStatHandle::instance_tracker_t::instance_iter it = BlockTimerStatHandle::instance_tracker_t::beginInstances(), end_it = BlockTimerStatHandle::instance_tracker_t::endInstances(); - it != end_it; - ++it) + for (auto& base : BlockTimerStatHandle::instance_snapshot()) { - BlockTimerStatHandle& time_block = static_cast<BlockTimerStatHandle&>(*it); - TimeBlockTreeNode& tree_node = mTimeBlockTreeNodes[it->getIndex()]; + // because of indirect derivation from LLInstanceTracker, have to downcast + BlockTimerStatHandle& time_block = static_cast<BlockTimerStatHandle&>(base); + TimeBlockTreeNode& tree_node = mTimeBlockTreeNodes[time_block.getIndex()]; tree_node.mBlock = &time_block; tree_node.mParent = &root_time_block; - it->getCurrentAccumulator().mParent = &root_time_block; + time_block.getCurrentAccumulator().mParent = &root_time_block; } mRootTimer = new BlockTimer(root_time_block); diff --git a/indra/llcommon/tests/llinstancetracker_test.cpp b/indra/llcommon/tests/llinstancetracker_test.cpp index d94fc0c56d..9b89159625 100644 --- a/indra/llcommon/tests/llinstancetracker_test.cpp +++ b/indra/llcommon/tests/llinstancetracker_test.cpp @@ -41,7 +41,6 @@ #include <boost/scoped_ptr.hpp> // other Linden headers #include "../test/lltut.h" -#include "wrapllerrs.h" struct Badness: public std::runtime_error { @@ -112,24 +111,22 @@ namespace tut void object::test<2>() { ensure_equals(Unkeyed::instanceCount(), 0); - Unkeyed* dangling = NULL; + std::weak_ptr<Unkeyed> dangling; { Unkeyed one; ensure_equals(Unkeyed::instanceCount(), 1); - Unkeyed* found = Unkeyed::getInstance(&one); - ensure_equals(found, &one); + std::weak_ptr<Unkeyed> found = one.getWeak(); + ensure(! found.expired()); { boost::scoped_ptr<Unkeyed> two(new Unkeyed); ensure_equals(Unkeyed::instanceCount(), 2); - Unkeyed* found = Unkeyed::getInstance(two.get()); - ensure_equals(found, two.get()); } ensure_equals(Unkeyed::instanceCount(), 1); - // store an unwise pointer to a temp Unkeyed instance - dangling = &one; + // store a weak pointer to a temp Unkeyed instance + dangling = found; } // make that instance vanish // check the now-invalid pointer to the destroyed instance - ensure("getInstance(T*) failed to track destruction", ! Unkeyed::getInstance(dangling)); + ensure("weak_ptr<Unkeyed> failed to track destruction", dangling.expired()); ensure_equals(Unkeyed::instanceCount(), 0); } @@ -142,7 +139,8 @@ namespace tut // reimplement LLInstanceTracker using, say, a hash map instead of a // std::map. We DO insist that every key appear exactly once. typedef std::vector<std::string> StringVector; - StringVector keys(Keyed::beginKeys(), Keyed::endKeys()); + auto snap = Keyed::key_snapshot(); + StringVector keys(snap.begin(), snap.end()); std::sort(keys.begin(), keys.end()); StringVector::const_iterator ki(keys.begin()); ensure_equals(*ki++, "one"); @@ -153,17 +151,15 @@ namespace tut ensure("didn't reach end", ki == keys.end()); // Use a somewhat different approach to order independence with - // beginInstances(): explicitly capture the instances we know in a + // instance_snapshot(): explicitly capture the instances we know in a // set, and delete them as we iterate through. typedef std::set<Keyed*> InstanceSet; InstanceSet instances; instances.insert(&one); instances.insert(&two); instances.insert(&three); - for (Keyed::instance_iter ii(Keyed::beginInstances()), iend(Keyed::endInstances()); - ii != iend; ++ii) + for (auto& ref : Keyed::instance_snapshot()) { - Keyed& ref = *ii; ensure_equals("spurious instance", instances.erase(&ref), 1); } ensure_equals("unreported instance", instances.size(), 0); @@ -180,11 +176,10 @@ namespace tut instances.insert(&two); instances.insert(&three); - for (Unkeyed::instance_iter ii(Unkeyed::beginInstances()), iend(Unkeyed::endInstances()); ii != iend; ++ii) - { - Unkeyed& ref = *ii; - ensure_equals("spurious instance", instances.erase(&ref), 1); - } + for (auto& ref : Unkeyed::instance_snapshot()) + { + ensure_equals("spurious instance", instances.erase(&ref), 1); + } ensure_equals("unreported instance", instances.size(), 0); } @@ -192,49 +187,49 @@ namespace tut template<> template<> void object::test<5>() { - set_test_name("delete Keyed with outstanding instance_iter"); - std::string what; - Keyed* keyed = new Keyed("delete Keyed with outstanding instance_iter"); - { - WrapLLErrs wrapper; - Keyed::instance_iter i(Keyed::beginInstances()); - what = wrapper.catch_llerrs([&keyed](){ - delete keyed; - }); - } - ensure(! what.empty()); + std::string desc("delete Keyed with outstanding instance_snapshot"); + set_test_name(desc); + Keyed* keyed = new Keyed(desc); + // capture a snapshot but do not yet traverse it + auto snapshot = Keyed::instance_snapshot(); + // delete the one instance + delete keyed; + // traversing the snapshot should reflect the deletion + // avoid ensure_equals() because it requires the ability to stream the + // two values to std::ostream + ensure(snapshot.begin() == snapshot.end()); } template<> template<> void object::test<6>() { - set_test_name("delete Keyed with outstanding key_iter"); - std::string what; - Keyed* keyed = new Keyed("delete Keyed with outstanding key_it"); - { - WrapLLErrs wrapper; - Keyed::key_iter i(Keyed::beginKeys()); - what = wrapper.catch_llerrs([&keyed](){ - delete keyed; - }); - } - ensure(! what.empty()); + std::string desc("delete Keyed with outstanding key_snapshot"); + set_test_name(desc); + Keyed* keyed = new Keyed(desc); + // capture a snapshot but do not yet traverse it + auto snapshot = Keyed::key_snapshot(); + // delete the one instance + delete keyed; + // traversing the snapshot should reflect the deletion + // avoid ensure_equals() because it requires the ability to stream the + // two values to std::ostream + ensure(snapshot.begin() == snapshot.end()); } template<> template<> void object::test<7>() { - set_test_name("delete Unkeyed with outstanding instance_iter"); + set_test_name("delete Unkeyed with outstanding instance_snapshot"); std::string what; Unkeyed* unkeyed = new Unkeyed; - { - WrapLLErrs wrapper; - Unkeyed::instance_iter i(Unkeyed::beginInstances()); - what = wrapper.catch_llerrs([&unkeyed](){ - delete unkeyed; - }); - } - ensure(! what.empty()); + // capture a snapshot but do not yet traverse it + auto snapshot = Unkeyed::instance_snapshot(); + // delete the one instance + delete unkeyed; + // traversing the snapshot should reflect the deletion + // avoid ensure_equals() because it requires the ability to stream the + // two values to std::ostream + ensure(snapshot.begin() == snapshot.end()); } template<> template<> @@ -246,11 +241,9 @@ namespace tut // We can't use the iterator-range InstanceSet constructor because // beginInstances() returns an iterator that dereferences to an // Unkeyed&, not an Unkeyed*. - for (Unkeyed::instance_iter uki(Unkeyed::beginInstances()), - ukend(Unkeyed::endInstances()); - uki != ukend; ++uki) + for (auto& ref : Unkeyed::instance_snapshot()) { - existing.insert(&*uki); + existing.insert(&ref); } try { @@ -273,11 +266,9 @@ namespace tut // instances was also present in the original set. If that's not true, // it's because our new Unkeyed ended up in the updated set despite // its constructor exception. - for (Unkeyed::instance_iter uki(Unkeyed::beginInstances()), - ukend(Unkeyed::endInstances()); - uki != ukend; ++uki) + for (auto& ref : Unkeyed::instance_snapshot()) { - ensure("failed to remove instance", existing.find(&*uki) != existing.end()); + ensure("failed to remove instance", existing.find(&ref) != existing.end()); } } } // namespace tut diff --git a/indra/llcommon/tests/llleap_test.cpp b/indra/llcommon/tests/llleap_test.cpp index bf0a74d10d..9d71e327d8 100644 --- a/indra/llcommon/tests/llleap_test.cpp +++ b/indra/llcommon/tests/llleap_test.cpp @@ -49,24 +49,28 @@ const size_t BUFFERED_LENGTH = 1023*1024; // try wrangling just under a megabyte #endif -void waitfor(const std::vector<LLLeap*>& instances, int timeout=60) +// capture std::weak_ptrs to LLLeap instances so we can tell when they expire +typedef std::vector<std::weak_ptr<LLLeap>> LLLeapVector; + +void waitfor(const LLLeapVector& instances, int timeout=60) { int i; for (i = 0; i < timeout; ++i) { // Every iteration, test whether any of the passed LLLeap instances // still exist (are still running). - std::vector<LLLeap*>::const_iterator vli(instances.begin()), vlend(instances.end()); - for ( ; vli != vlend; ++vli) + bool found = false; + for (auto& ptr : instances) { - // getInstance() returns NULL if it's terminated/gone, non-NULL if - // it's still running - if (LLLeap::getInstance(*vli)) + if (! ptr.expired()) + { + found = true; break; + } } // If we made it through all of 'instances' without finding one that's // still running, we're done. - if (vli == vlend) + if (! found) { /*==========================================================================*| std::cout << instances.size() << " LLLeap instances terminated in " @@ -86,8 +90,8 @@ void waitfor(const std::vector<LLLeap*>& instances, int timeout=60) void waitfor(LLLeap* instance, int timeout=60) { - std::vector<LLLeap*> instances; - instances.push_back(instance); + LLLeapVector instances; + instances.push_back(instance->getWeak()); waitfor(instances, timeout); } @@ -218,11 +222,11 @@ namespace tut NamedTempFile script("py", "import time\n" "time.sleep(1)\n"); - std::vector<LLLeap*> instances; + LLLeapVector instances; instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + sv(list_of(PYTHON)(script.getName())))->getWeak()); instances.push_back(LLLeap::create(get_test_name(), - sv(list_of(PYTHON)(script.getName())))); + sv(list_of(PYTHON)(script.getName())))->getWeak()); // In this case we're simply establishing that two LLLeap instances // can coexist without throwing exceptions or bombing in any other // way. Wait for them to terminate. |