SL-10297 merged 6.4.7

1 files changed, 386 insertions, 308 deletions

diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h
index 363d0bcbd5..402333cca7 100644
--- a/indra/llcommon/llinstancetracker.h
+++ b/indra/llcommon/llinstancetracker.h
@@ -28,354 +28,432 @@
 #ifndef LL_LLINSTANCETRACKER_H
 #define LL_LLINSTANCETRACKER_H
 
-#include <atomic>
 #include <map>
+#include <set>
+#include <vector>
 #include <typeinfo>
+#include <memory>
+#include <type_traits>
+
+#include "mutex.h"
 
-#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
+#include "lockstatic.h"
+#include "stringize.h"
 
-/**
- * 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.
- */
-class LL_COMMON_API LLInstanceTrackerBase
+/*****************************************************************************
+*   StaticBase
+*****************************************************************************/
+namespace LLInstanceTrackerPrivate
 {
-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
     {
-        StaticBase():
-            sIterationNestDepth(0)
-        {}
-
-		void incrementDepth();
-		void decrementDepth();
-		U32 getDepth();
-	private:
-#ifdef LL_WINDOWS
-		std::atomic_uint32_t sIterationNestDepth;
-#else
-		std::atomic_uint sIterationNestDepth;
-#endif
-	};
-};
+        // We need to be able to lock static data while manipulating it.
+        std::mutex mMutex;
+    };
 
-LL_COMMON_API void assert_main_thread();
+    void logerrs(const char* cls, const std::string&, const std::string&, const std::string&);
+} // namespace LLInstanceTrackerPrivate
 
+/*****************************************************************************
+*   LLInstanceTracker with key
+*****************************************************************************/
 enum EInstanceTrackerAllowKeyCollisions
 {
-	LLInstanceTrackerErrorOnCollision,
-	LLInstanceTrackerReplaceOnCollision
+    LLInstanceTrackerErrorOnCollision,
+    LLInstanceTrackerReplaceOnCollision
 };
 
 /// 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
 {
-	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 std::map<KEY, std::shared_ptr<T>> InstanceMap;
+    struct StaticData: public LLInstanceTrackerPrivate::StaticBase
+    {
+        InstanceMap mMap;
+    };
+    typedef llthread::LockStatic<StaticData> LockStatic;
 
 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));
+        }
+
+        // lock static data during construction
+#if ! LL_WINDOWS
+        LockStatic mLock;
+#else  // LL_WINDOWS
+        // We want to be able to use (e.g.) our instance_snapshot subclass as:
+        // for (auto& inst : T::instance_snapshot()) ...
+        // But when this snapshot base class directly contains LockStatic, as
+        // above, Visual Studio 2017 requires us to code instead:
+        // for (auto& inst : std::move(T::instance_snapshot())) ...
+        // nat thinks this should be unnecessary, as an anonymous class
+        // instance is already a temporary. It shouldn't need to be cast to
+        // rvalue reference (the role of std::move()). clang evidently agrees,
+        // as the short form works fine with Xcode on Mac.
+        // To support the succinct usage, instead of directly storing
+        // LockStatic, store std::shared_ptr<LockStatic>, which is copyable.
+        std::shared_ptr<LockStatic> mLockp{std::make_shared<LockStatic>()};
+        LockStatic& mLock{*mLockp};
+#endif // LL_WINDOWS
+        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 std::string report(K key) { return stringize(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)
+            {
+                LLInstanceTrackerPrivate::logerrs(typeid(*this).name(), " instance with key ",
+                                                  report(key), " already exists!");
+            }
+            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
+*****************************************************************************/
+// 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>
 {
-	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 std::set<std::shared_ptr<T>> InstanceSet;
+    struct StaticData: public LLInstanceTrackerPrivate::StaticBase
+    {
+        InstanceSet mSet;
+    };
+    typedef llthread::LockStatic<StaticData> LockStatic;
 
 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));
+        }
+
+        // lock static data during construction
+#if ! LL_WINDOWS
+        LockStatic mLock;
+#else  // LL_WINDOWS
+        // We want to be able to use our instance_snapshot subclass as:
+        // for (auto& inst : T::instance_snapshot()) ...
+        // But when this snapshot base class directly contains LockStatic, as
+        // above, Visual Studio 2017 requires us to code instead:
+        // for (auto& inst : std::move(T::instance_snapshot())) ...
+        // nat thinks this should be unnecessary, as an anonymous class
+        // instance is already a temporary. It shouldn't need to be cast to
+        // rvalue reference (the role of std::move()). clang evidently agrees,
+        // as the short form works fine with Xcode on Mac.
+        // To support the succinct usage, instead of directly storing
+        // LockStatic, store std::shared_ptr<LockStatic>, which is copyable.
+        std::shared_ptr<LockStatic> mLockp{std::make_shared<LockStatic>()};
+        LockStatic& mLock{*mLockp};
+#endif // LL_WINDOWS
+        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