merge

8 files changed, 250 insertions, 60 deletions

diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt
index 658eefbab2..6f39aba976 100644
--- a/indra/llcommon/CMakeLists.txt
+++ b/indra/llcommon/CMakeLists.txt
@@ -322,6 +322,7 @@ if (LL_TESTS)
   LL_ADD_INTEGRATION_TEST(llrand "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(llsdserialize "" "${test_libs}"
                           "${PYTHON_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/tests/setpython.py")
+  LL_ADD_INTEGRATION_TEST(llsingleton "" "${test_libs}")                          
   LL_ADD_INTEGRATION_TEST(llstring "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(lltreeiterators "" "${test_libs}")
   LL_ADD_INTEGRATION_TEST(lluri "" "${test_libs}")
diff --git a/indra/llcommon/llinstancetracker.cpp b/indra/llcommon/llinstancetracker.cpp
index f576204511..5dc3ea5d7b 100644
--- a/indra/llcommon/llinstancetracker.cpp
+++ b/indra/llcommon/llinstancetracker.cpp
@@ -35,14 +35,15 @@
 //static 
 void * & LLInstanceTrackerBase::getInstances(std::type_info const & info)
 {
-	static std::map<std::string, void *> instances;
+	typedef std::map<std::string, void *> InstancesMap;
+	static InstancesMap instances;
 
-	std::string k = info.name();
-	if(instances.find(k) == instances.end())
-	{
-		instances[k] = NULL;
-	}
-
-	return instances[k];
+	// std::map::insert() is just what we want here. You attempt to insert a
+	// (key, value) pair. If the specified key doesn't yet exist, it inserts
+	// the pair and returns a std::pair of (iterator, true). If the specified
+	// key DOES exist, insert() simply returns (iterator, false). One lookup
+	// handles both cases.
+	return instances.insert(InstancesMap::value_type(info.name(),
+													 InstancesMap::mapped_type()))
+		.first->second;
 }
-
diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h
index afb714c71c..5a3990a8df 100644
--- a/indra/llcommon/llinstancetracker.h
+++ b/indra/llcommon/llinstancetracker.h
@@ -29,6 +29,7 @@
 #define LL_LLINSTANCETRACKER_H
 
 #include <map>
+#include <typeinfo>
 
 #include "string_table.h"
 #include <boost/utility.hpp>
@@ -37,10 +38,40 @@
 #include <boost/iterator/transform_iterator.hpp>
 #include <boost/iterator/indirect_iterator.hpp>
 
+/**
+ * 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 : public boost::noncopyable
 {
-	protected:
-		static void * & getInstances(std::type_info const & info);
+protected:
+	/// Get a process-unique void* pointer slot for the specified type_info
+	static void * & getInstances(std::type_info const & info);
+
+	/// Find or create a STATICDATA instance for the specified TRACKED class.
+	/// STATICDATA must be default-constructible.
+	template<typename STATICDATA, class TRACKED>
+	static STATICDATA& getStatic()
+	{
+		void *& instances = getInstances(typeid(TRACKED));
+		if (! instances)
+		{
+			instances = new STATICDATA;
+		}
+		return *static_cast<STATICDATA*>(instances);
+	}
+
+    /// 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)
+        {}
+        S32 sIterationNestDepth;
+    };
 };
 
 /// This mix-in class adds support for tracking all instances of the specified class parameter T
@@ -50,8 +81,15 @@ class LL_COMMON_API LLInstanceTrackerBase : public boost::noncopyable
 template<typename T, typename KEY = T*>
 class LLInstanceTracker : public LLInstanceTrackerBase
 {
-	typedef typename std::map<KEY, T*> InstanceMap;
 	typedef LLInstanceTracker<T, KEY> MyT;
+	typedef typename std::map<KEY, T*> InstanceMap;
+	struct StaticData: public StaticBase
+	{
+		InstanceMap sMap;
+	};
+	static StaticData& getStatic() { return LLInstanceTrackerBase::getStatic<StaticData, MyT>(); }
+	static InstanceMap& getMap_() { return getStatic().sMap; }
+
 public:
 	class instance_iter : public boost::iterator_facade<instance_iter, T, boost::forward_traversal_tag>
 	{
@@ -61,12 +99,12 @@ public:
 		instance_iter(const typename InstanceMap::iterator& it)
 		:	mIterator(it)
 		{
-			++sIterationNestDepth;
+			++getStatic().sIterationNestDepth;
 		}
 
 		~instance_iter()
 		{
-			--sIterationNestDepth;
+			--getStatic().sIterationNestDepth;
 		}
 
 
@@ -95,18 +133,18 @@ public:
 		key_iter(typename InstanceMap::iterator it)
 			:	mIterator(it)
 		{
-			++sIterationNestDepth;
+			++getStatic().sIterationNestDepth;
 		}
 
 		key_iter(const key_iter& other)
 			:	mIterator(other.mIterator)
 		{
-			++sIterationNestDepth;
+			++getStatic().sIterationNestDepth;
 		}
 
 		~key_iter()
 		{
-			--sIterationNestDepth;
+			--getStatic().sIterationNestDepth;
 		}
 
 
@@ -159,8 +197,8 @@ protected:
 	virtual ~LLInstanceTracker() 
 	{ 
 		// it's unsafe to delete instances of this type while all instances are being iterated over.
-		llassert(sIterationNestDepth == 0);
-		remove_(); 		
+		llassert_always(getStatic().sIterationNestDepth == 0);
+		remove_();		
 	}
 	virtual void setKey(KEY key) { remove_(); add_(key); }
 	virtual const KEY& getKey() const { return mInstanceKey; }
@@ -176,31 +214,24 @@ private:
 		getMap_().erase(mInstanceKey);
 	}
 
-    static InstanceMap& getMap_()
-    {
-		void * & instances = getInstances(typeid(MyT));
-        if (! instances)
-        {
-            instances = new InstanceMap;
-        }
-        return * static_cast<InstanceMap*>(instances);
-    }
-
 private:
-
 	KEY mInstanceKey;
-	static S32 sIterationNestDepth;
 };
 
-template <typename T, typename KEY> S32 LLInstanceTracker<T, KEY>::sIterationNestDepth = 0;
-
 /// explicit specialization for default case where KEY is T*
 /// use a simple std::set<T*>
 template<typename T>
 class LLInstanceTracker<T, T*> : public LLInstanceTrackerBase
 {
-	typedef typename std::set<T*> InstanceSet;
 	typedef LLInstanceTracker<T, T*> MyT;
+	typedef typename std::set<T*> InstanceSet;
+	struct StaticData: public StaticBase
+	{
+		InstanceSet sSet;
+	};
+	static StaticData& getStatic() { return LLInstanceTrackerBase::getStatic<StaticData, MyT>(); }
+	static InstanceSet& getSet_() { return getStatic().sSet; }
+
 public:
 
 	/// for completeness of analogy with the generic implementation
@@ -213,18 +244,18 @@ public:
 		instance_iter(const typename InstanceSet::iterator& it)
 		:	mIterator(it)
 		{
-			++sIterationNestDepth;
+			++getStatic().sIterationNestDepth;
 		}
 
 		instance_iter(const instance_iter& other)
 		:	mIterator(other.mIterator)
 		{
-			++sIterationNestDepth;
+			++getStatic().sIterationNestDepth;
 		}
 
 		~instance_iter()
 		{
-			--sIterationNestDepth;
+			--getStatic().sIterationNestDepth;
 		}
 
 	private:
@@ -250,13 +281,13 @@ public:
 protected:
 	LLInstanceTracker()
 	{
-		// it's safe but unpredictable to create instances of this type while all instances are being iterated over.  I hate unpredictable.  This assert will probably be turned on early in the next development cycle.
+		// it's safe but unpredictable to create instances of this type while all instances are being iterated over.  I hate unpredictable.	 This assert will probably be turned on early in the next development cycle.
 		getSet_().insert(static_cast<T*>(this));
 	}
 	virtual ~LLInstanceTracker()
 	{
 		// it's unsafe to delete instances of this type while all instances are being iterated over.
-		llassert(sIterationNestDepth == 0);
+		llassert_always(getStatic().sIterationNestDepth == 0);
 		getSet_().erase(static_cast<T*>(this));
 	}
 
@@ -264,20 +295,6 @@ protected:
 	{
 		getSet_().insert(static_cast<T*>(this));
 	}
-
-	static InstanceSet& getSet_()
-	{
-		void * & instances = getInstances(typeid(MyT));
-		if (! instances)
-		{
-			instances = new InstanceSet;
-		}
-		return * static_cast<InstanceSet *>(instances);
-	}
-
-	static S32 sIterationNestDepth;
 };
 
-template <typename T> S32 LLInstanceTracker<T, T*>::sIterationNestDepth = 0;
-
 #endif
diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h
index 00757be277..49d99f2cd0 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -114,8 +114,7 @@ private:
 
 		~SingletonInstanceData()
 		{
-			SingletonInstanceData& data = getData();
-			if (data.mInitState != DELETED)
+			if (mInitState != DELETED)
 			{
 				deleteSingleton();
 			}
@@ -130,7 +129,26 @@ public:
 		data.mInitState = DELETED;
 	}
 
-	// Can be used to control when the singleton is deleted.  Not normally needed.
+	/**
+	 * @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 getData().mSingletonInstance;
diff --git a/indra/llcommon/llstring.cpp b/indra/llcommon/llstring.cpp
index f3b48b0156..e7fe656808 100644
--- a/indra/llcommon/llstring.cpp
+++ b/indra/llcommon/llstring.cpp
@@ -936,13 +936,18 @@ LLStringUtil::size_type LLStringUtil::getSubstitution(const std::string& instr,
 {
 	const std::string delims (",");
 	
-	// Find the first ]
-	size_type pos2 = instr.find(']', start);
+	// Find the first [
+	size_type pos1 = instr.find('[', start);
+	if (pos1 == std::string::npos)
+		return std::string::npos;
+
+	//Find the first ] after the initial [
+	size_type pos2 = instr.find(']', pos1);
 	if (pos2 == std::string::npos)
 		return std::string::npos;
 
-	// Find the last [ before ]
-	size_type pos1 = instr.find_last_of('[', pos2-1);
+	// Find the last [ before ] in case of nested [[]]
+	pos1 = instr.find_last_of('[', pos2-1);
 	if (pos1 == std::string::npos || pos1 < start)
 		return std::string::npos;
 	
diff --git a/indra/llcommon/tests/llinstancetracker_test.cpp b/indra/llcommon/tests/llinstancetracker_test.cpp
index 80b35bbdc3..b34d1c5fd3 100644
--- a/indra/llcommon/tests/llinstancetracker_test.cpp
+++ b/indra/llcommon/tests/llinstancetracker_test.cpp
@@ -40,6 +40,7 @@
 #include <boost/scoped_ptr.hpp>
 // other Linden headers
 #include "../test/lltut.h"
+#include "wrapllerrs.h"
 
 struct Keyed: public LLInstanceTracker<Keyed, std::string>
 {
@@ -165,4 +166,67 @@ namespace tut
 
         ensure_equals("unreported instance", instances.size(), 0);
     }
+
+    template<> template<>
+    void object::test<5>()
+    {
+        set_test_name("delete Keyed with outstanding instance_iter");
+        std::string what;
+        Keyed* keyed = new Keyed("one");
+        {
+            WrapLL_ERRS wrapper;
+            Keyed::instance_iter i(Keyed::beginInstances());
+            try
+            {
+                delete keyed;
+            }
+            catch (const WrapLL_ERRS::FatalException& e)
+            {
+                what = e.what();
+            }
+        }
+        ensure(! what.empty());
+    }
+
+    template<> template<>
+    void object::test<6>()
+    {
+        set_test_name("delete Keyed with outstanding key_iter");
+        std::string what;
+        Keyed* keyed = new Keyed("one");
+        {
+            WrapLL_ERRS wrapper;
+            Keyed::key_iter i(Keyed::beginKeys());
+            try
+            {
+                delete keyed;
+            }
+            catch (const WrapLL_ERRS::FatalException& e)
+            {
+                what = e.what();
+            }
+        }
+        ensure(! what.empty());
+    }
+
+    template<> template<>
+    void object::test<7>()
+    {
+        set_test_name("delete Unkeyed with outstanding instance_iter");
+        std::string what;
+        Unkeyed* unkeyed = new Unkeyed;
+        {
+            WrapLL_ERRS wrapper;
+            Unkeyed::instance_iter i(Unkeyed::beginInstances());
+            try
+            {
+                delete unkeyed;
+            }
+            catch (const WrapLL_ERRS::FatalException& e)
+            {
+                what = e.what();
+            }
+        }
+        ensure(! what.empty());
+    }
 } // namespace tut
diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp
new file mode 100644
index 0000000000..385289aefe
--- /dev/null
+++ b/indra/llcommon/tests/llsingleton_test.cpp
@@ -0,0 +1,76 @@
+/** 
+ * @file llsingleton_test.cpp
+ * @date 2011-08-11
+ * @brief Unit test for the LLSingleton class
+ *
+ * $LicenseInfo:firstyear=2011&license=viewerlgpl$
+ * Second Life Viewer Source Code
+ * Copyright (C) 2011, Linden Research, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License only.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
+ * $/LicenseInfo$
+ */
+
+#include "linden_common.h"
+
+#include "llsingleton.h"
+#include "../test/lltut.h"
+
+namespace tut
+{
+	struct singleton
+	{
+		// We need a class created with the LLSingleton template to test with.
+		class LLSingletonTest: public LLSingleton<LLSingletonTest>
+		{
+
+		};
+	};
+
+	typedef test_group<singleton> 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::destroyed());
+		ensure(!LLSingletonTest::instanceExists());
+
+		//Construct it again.
+		LLSingletonTest* singleton_test = LLSingletonTest::getInstance();
+		ensure(singleton_test);
+		ensure(LLSingletonTest::instanceExists());
+	}
+}
diff --git a/indra/llcommon/tests/llstring_test.cpp b/indra/llcommon/tests/llstring_test.cpp
index 304e91ed92..6a1cbf652a 100644
--- a/indra/llcommon/tests/llstring_test.cpp
+++ b/indra/llcommon/tests/llstring_test.cpp
@@ -624,6 +624,14 @@ namespace tut
 		subcount = LLStringUtil::format(s, fmt_map);
 		ensure_equals("LLStringUtil::format: Assorted Test2 result", s, "?Am I not a long string?short[A]bbbaaaba[A]");
 		ensure_equals("LLStringUtil::format: Assorted Test2 result count", 9, subcount);
+		
+		// Test on nested brackets
+		std::string srcs6 = "[[TRICK1]][[A]][[B]][[AAA]][[BBB]][[TRICK2]][[KEYLONGER]][[KEYSHORTER]]?[[DELETE]]";
+		s = srcs6;
+		subcount = LLStringUtil::format(s, fmt_map);
+		ensure_equals("LLStringUtil::format: Assorted Test2 result", s, "[[A]][a][b][aaa][bbb][[A]][short][Am I not a long string?]?[]");
+		ensure_equals("LLStringUtil::format: Assorted Test2 result count", 9, subcount);
+
 
 		// Test an assorted substitution
 		std::string srcs8 = "foo[DELETE]bar?";