IQA-463: LLError::addRecorder() claims ownership of passed Recorder*.

That is, when the underlying LLError::Settings object is destroyed -- possibly
at termination, possibly on LLError::restoreSettings() -- the passed Recorder*
is deleted.
There was much existing code that seemed as unaware of this alarming fact as I
was myself. Passing to addRecorder() a pointer to a stack object, or to a
member of some other object, is just Bad. It might be preferable to make
addRecorder() accept std::auto_ptr<Recorder> to make the ownership transfer
more explicit -- or even boost::shared_ptr<Recorder> instead, which would
allow the caller to either forget or retain the passed Recorder.
This preliminary pass retains the Recorder* dumb pointer API, but documents
the ownership issue, and eliminates known instances of passing pointers to
anything but a standalone heap Recorder subclass object.

3 files changed, 199 insertions, 150 deletions

diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index 1be49cebc8..d53a819d88 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -1,4 +1,4 @@
-/** 
+/**
  * @file llerrorcontrol.h
  * @date   December 2006
  * @brief error message system control
@@ -6,21 +6,21 @@
  * $LicenseInfo:firstyear=2007&license=viewerlgpl$
  * Second Life Viewer Source Code
  * Copyright (C) 2010, 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$
  */
@@ -38,7 +38,7 @@ class LLSD;
 	This is the part of the LLError namespace that manages the messages
 	produced by the logging.  The logging support is defined in llerror.h.
 	Most files do not need to include this.
-	
+
 	These implementations are in llerror.cpp.
 */
 
@@ -72,7 +72,7 @@ namespace LLError
 		Settings that control what is logged.
 		Setting a level means log messages at that level or above.
 	*/
-	
+
 	LL_COMMON_API void setPrintLocation(bool);
 	LL_COMMON_API void setDefaultLevel(LLError::ELevel);
 	LL_COMMON_API ELevel getDefaultLevel();
@@ -101,31 +101,31 @@ namespace LLError
 		// (by, for example, setting a class level to LEVEL_NONE), will keep
 		// the that message from causing the fatal funciton to be invoked.
 
-    LL_COMMON_API FatalFunction getFatalFunction();
-        // Retrieve the previously-set FatalFunction
-
-    /// temporarily override the FatalFunction for the duration of a
-    /// particular scope, e.g. for unit tests
-    class LL_COMMON_API OverrideFatalFunction
-    {
-    public:
-        OverrideFatalFunction(const FatalFunction& func):
-            mPrev(getFatalFunction())
-        {
-            setFatalFunction(func);
-        }
-        ~OverrideFatalFunction()
-        {
-            setFatalFunction(mPrev);
-        }
-
-    private:
-        FatalFunction mPrev;
-    };
+	LL_COMMON_API FatalFunction getFatalFunction();
+		// Retrieve the previously-set FatalFunction
+
+	/// temporarily override the FatalFunction for the duration of a
+	/// particular scope, e.g. for unit tests
+	class LL_COMMON_API OverrideFatalFunction
+	{
+	public:
+		OverrideFatalFunction(const FatalFunction& func):
+			mPrev(getFatalFunction())
+		{
+			setFatalFunction(func);
+		}
+		~OverrideFatalFunction()
+		{
+			setFatalFunction(mPrev);
+		}
+
+	private:
+		FatalFunction mPrev;
+	};
 
 	typedef std::string (*TimeFunction)();
 	LL_COMMON_API std::string utcTime();
-	
+
 	LL_COMMON_API void setTimeFunction(TimeFunction);
 		// The function is use to return the current time, formatted for
 		// display by those error recorders that want the time included.
@@ -137,19 +137,27 @@ namespace LLError
 		// An object that handles the actual output or error messages.
 	public:
 		virtual ~Recorder();
-		
+
 		virtual void recordMessage(LLError::ELevel, const std::string& message) = 0;
 			// use the level for better display, not for filtering
-			
+
 		virtual bool wantsTime(); // default returns false
 			// override and return true if the recorder wants the time string
 			// included in the text of the message
 	};
-	
+
+	/**
+	 * @NOTE: addRecorder() conveys ownership to the underlying Settings
+	 * object -- when destroyed, it will @em delete the passed Recorder*!
+	 */
 	LL_COMMON_API void addRecorder(Recorder*);
+	/**
+	 * @NOTE: removeRecorder() reclaims ownership of the Recorder*: its
+	 * lifespan becomes the caller's problem.
+	 */
 	LL_COMMON_API void removeRecorder(Recorder*);
 		// each error message is passed to each recorder via recordMessage()
-	
+
 	LL_COMMON_API void logToFile(const std::string& filename);
 	LL_COMMON_API void logToFixedBuffer(LLLineBuffer*);
 		// Utilities to add recorders for logging to a file or a fixed buffer
@@ -167,10 +175,9 @@ namespace LLError
 	class Settings;
 	LL_COMMON_API Settings* saveAndResetSettings();
 	LL_COMMON_API void restoreSettings(Settings *);
-		
+
 	LL_COMMON_API std::string abbreviateFile(const std::string& filePath);
 	LL_COMMON_API int shouldLogCallCount();
-	
 };
 
 #endif // LL_LLERRORCONTROL_H
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index 09a20231de..279a90e51b 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -1,4 +1,4 @@
-/** 
+/**
  * @file llerror_test.cpp
  * @date   December 2006
  * @brief error unit tests
@@ -6,21 +6,21 @@
  * $LicenseInfo:firstyear=2006&license=viewerlgpl$
  * Second Life Viewer Source Code
  * Copyright (C) 2010, 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$
  */
@@ -49,7 +49,7 @@ namespace
 	static bool fatalWasCalled;
 	void fatalCall(const std::string&) { fatalWasCalled = true; }
 }
-	
+
 namespace tut
 {
 	class TestRecorder : public LLError::Recorder
@@ -57,59 +57,65 @@ namespace tut
 	public:
 		TestRecorder() : mWantsTime(false) { }
 		~TestRecorder() { LLError::removeRecorder(this); }
-		
+
 		void recordMessage(LLError::ELevel level,
 						   const std::string& message)
 		{
 			mMessages.push_back(message);
 		}
-		
+
 		int countMessages()			{ return (int) mMessages.size(); }
 		void clearMessages()		{ mMessages.clear(); }
-		
+
 		void setWantsTime(bool t)	{ mWantsTime = t; }
 		bool wantsTime()			{ return mWantsTime; }
-		
+
 		std::string message(int n)
 		{
 			std::ostringstream test_name;
 			test_name << "testing message " << n << ", not enough messages";
-			
+
 			tut::ensure(test_name.str(), n < countMessages());
 			return mMessages[n];
 		}
-		
+
 	private:
 		typedef std::vector<std::string> MessageVector;
 		MessageVector mMessages;
-		
+
 		bool mWantsTime;
 	};
 
 	struct ErrorTestData
 	{
-		TestRecorder mRecorder;
+		// addRecorder() expects to be able to later delete the passed
+		// Recorder*. Even though removeRecorder() reclaims ownership, passing
+		// a pointer to a data member rather than a heap Recorder subclass
+		// instance would just be Wrong.
+		TestRecorder* mRecorder;
 		LLError::Settings* mPriorErrorSettings;
-		
-		ErrorTestData()
+
+		ErrorTestData():
+			mRecorder(new TestRecorder)
 		{
 			fatalWasCalled = false;
-			
+
 			mPriorErrorSettings = LLError::saveAndResetSettings();
 			LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
 			LLError::setFatalFunction(fatalCall);
-			LLError::addRecorder(&mRecorder);
+			LLError::addRecorder(mRecorder);
 		}
-		
+
 		~ErrorTestData()
 		{
-			LLError::removeRecorder(&mRecorder);
+			LLError::removeRecorder(mRecorder);
+			delete mRecorder;
 			LLError::restoreSettings(mPriorErrorSettings);
 		}
-		
+
 		void ensure_message_count(int expectedCount)
 		{
-			ensure_equals("message count", mRecorder.countMessages(), expectedCount);
+			ensure_equals("message count", mRecorder->countMessages(), expectedCount);
 		}
 
 		void ensure_message_contains(int n, const std::string& expectedText)
@@ -117,7 +123,7 @@ namespace tut
 			std::ostringstream test_name;
 			test_name << "testing message " << n;
 
-			ensure_contains(test_name.str(), mRecorder.message(n), expectedText);
+			ensure_contains(test_name.str(), mRecorder->message(n), expectedText);
 		}
 
 		void ensure_message_does_not_contain(int n, const std::string& expectedText)
@@ -125,22 +131,22 @@ namespace tut
 			std::ostringstream test_name;
 			test_name << "testing message " << n;
 
-			ensure_does_not_contain(test_name.str(), mRecorder.message(n), expectedText);
+			ensure_does_not_contain(test_name.str(), mRecorder->message(n), expectedText);
 		}
 	};
-	
+
 	typedef test_group<ErrorTestData>	ErrorTestGroup;
 	typedef ErrorTestGroup::object		ErrorTestObject;
-	
+
 	ErrorTestGroup errorTestGroup("error");
-	
+
 	template<> template<>
 	void ErrorTestObject::test<1>()
 		// basic test of output
 	{
 		llinfos << "test" << llendl;
 		llinfos << "bob" << llendl;
-		
+
 		ensure_message_contains(0, "test");
 		ensure_message_contains(1, "bob");
 	}
@@ -159,7 +165,7 @@ namespace
 };
 
 namespace tut
-{	
+{
 	template<> template<>
 	void ErrorTestObject::test<2>()
 		// messages are filtered based on default level
@@ -172,7 +178,7 @@ namespace tut
 		ensure_message_contains(3, "error");
 		ensure_message_contains(4, "four");
 		ensure_message_count(5);
-	
+
 		LLError::setDefaultLevel(LLError::LEVEL_INFO);
 		writeSome();
 		ensure_message_contains(5, "two");
@@ -180,20 +186,20 @@ namespace tut
 		ensure_message_contains(7, "error");
 		ensure_message_contains(8, "four");
 		ensure_message_count(9);
-		
+
 		LLError::setDefaultLevel(LLError::LEVEL_WARN);
 		writeSome();
 		ensure_message_contains(9, "three");
 		ensure_message_contains(10, "error");
 		ensure_message_contains(11, "four");
 		ensure_message_count(12);
-		
+
 		LLError::setDefaultLevel(LLError::LEVEL_ERROR);
 		writeSome();
 		ensure_message_contains(12, "error");
 		ensure_message_contains(13, "four");
 		ensure_message_count(14);
-		
+
 		LLError::setDefaultLevel(LLError::LEVEL_NONE);
 		writeSome();
 		ensure_message_count(14);
@@ -218,14 +224,14 @@ namespace tut
 	{
 		std::string thisFile = __FILE__;
 		std::string abbreviateFile = LLError::abbreviateFile(thisFile);
-		
+
 		ensure_ends_with("file name abbreviation",
 			abbreviateFile,
 			"llcommon/tests/llerror_test.cpp"
 			);
 		ensure_does_not_contain("file name abbreviation",
 			abbreviateFile, "indra");
-			
+
 		std::string someFile =
 #if LL_WINDOWS
 			"C:/amy/bob/cam.cpp"
@@ -234,12 +240,12 @@ namespace tut
 #endif
 			;
 		std::string someAbbreviation = LLError::abbreviateFile(someFile);
-		
+
 		ensure_equals("non-indra file abbreviation",
 			someAbbreviation, someFile);
 	}
 }
-	
+
 namespace
 {
 	std::string locationString(int line)
@@ -247,22 +253,22 @@ namespace
 		std::ostringstream location;
 		location << LLError::abbreviateFile(__FILE__)
 				 << "(" << line << ") : ";
-				 
+
 		return location.str();
 	}
-	
+
 	std::string writeReturningLocation()
 	{
 		llinfos << "apple" << llendl;	int this_line = __LINE__;
 		return locationString(this_line);
 	}
-	
+
 	std::string writeReturningLocationAndFunction()
 	{
 		llinfos << "apple" << llendl;	int this_line = __LINE__;
 		return locationString(this_line) + __FUNCTION__;
 	}
-	
+
 	std::string errorReturningLocation()
 	{
 		llerrs << "die" << llendl;	int this_line = __LINE__;
@@ -271,20 +277,20 @@ namespace
 }
 
 namespace tut
-{	
+{
 	template<> template<>
 	void ErrorTestObject::test<5>()
 		// file and line information in log messages
 	{
 		std::string location = writeReturningLocation();
 			// expecting default to not print location information
-		
+
 		LLError::setPrintLocation(true);
 		writeReturningLocation();
-		
+
 		LLError::setPrintLocation(false);
 		writeReturningLocation();
-		
+
 		ensure_message_does_not_contain(0, location);
 		ensure_message_contains(1, location);
 		ensure_message_does_not_contain(2, location);
@@ -297,7 +303,7 @@ namespace tut
 	existing log messages often do.)  The functions all return their C++
 	name so that test can be substantial mechanized.
  */
- 
+
 std::string logFromGlobal(bool id)
 {
 	llinfos << (id ? "logFromGlobal: " : "") << "hi" << llendl;
@@ -345,7 +351,7 @@ namespace
 			return "ClassWithNoLogType::logFromStatic";
 		}
 	};
-	
+
 	class ClassWithLogType {
 		LOG_CLASS(ClassWithLogType);
 	public:
@@ -360,13 +366,13 @@ namespace
 			return "ClassWithLogType::logFromStatic";
 		}
 	};
-	
+
 	std::string logFromNamespace(bool id) { return Foo::logFromNamespace(id); }
 	std::string logFromClassWithNoLogTypeMember(bool id) { ClassWithNoLogType c; return c.logFromMember(id); }
 	std::string logFromClassWithNoLogTypeStatic(bool id) { return ClassWithNoLogType::logFromStatic(id); }
 	std::string logFromClassWithLogTypeMember(bool id) { ClassWithLogType c; return c.logFromMember(id); }
 	std::string logFromClassWithLogTypeStatic(bool id) { return ClassWithLogType::logFromStatic(id); }
-	
+
 	void ensure_has(const std::string& message,
 		const std::string& actual, const std::string& expected)
 	{
@@ -379,18 +385,18 @@ namespace
 			throw tut::failure(ss.str().c_str());
 		}
 	}
-	
+
 	typedef std::string (*LogFromFunction)(bool);
-	void testLogName(tut::TestRecorder& recorder, LogFromFunction f,
+	void testLogName(tut::TestRecorder* recorder, LogFromFunction f,
 		const std::string& class_name = "")
 	{
-		recorder.clearMessages();
+		recorder->clearMessages();
 		std::string name = f(false);
 		f(true);
-		
-		std::string messageWithoutName = recorder.message(0);
-		std::string messageWithName = recorder.message(1);
-		
+
+		std::string messageWithoutName = recorder->message(0);
+		std::string messageWithName = recorder->message(1);
+
 		ensure_has(name + " logged without name",
 			messageWithoutName, name);
 		ensure_has(name + " logged with name",
@@ -431,18 +437,18 @@ namespace
 		llinfos << "inside" << llendl;
 		return "moo";
 	}
-	
+
 	std::string outerLogger()
 	{
 		llinfos << "outside(" << innerLogger() << ")" << llendl;
 		return "bar";
 	}
-	
+
 	void uberLogger()
 	{
 		llinfos << "uber(" << outerLogger() << "," << innerLogger() << ")" << llendl;
 	}
-	
+
 	class LogWhileLogging
 	{
 	public:
@@ -461,11 +467,11 @@ namespace
 		LogWhileLogging l;
 		llinfos << "meta(" << l << ")" << llendl;
 	}
-	
+
 }
 
 namespace tut
-{			
+{
 	template<> template<>
 		// handle nested logging
 	void ErrorTestObject::test<7>()
@@ -474,31 +480,31 @@ namespace tut
 		ensure_message_contains(0, "inside");
 		ensure_message_contains(1, "outside(moo)");
 		ensure_message_count(2);
-		
+
 		uberLogger();
 		ensure_message_contains(2, "inside");
 		ensure_message_contains(3, "inside");
 		ensure_message_contains(4, "outside(moo)");
 		ensure_message_contains(5, "uber(bar,moo)");
 		ensure_message_count(6);
-		
+
 		metaLogger();
 		ensure_message_contains(6, "logging");
 		ensure_message_contains(7, "meta(baz)");
 		ensure_message_count(8);
 	}
-	
+
 	template<> template<>
 		// special handling of llerrs calls
 	void ErrorTestObject::test<8>()
 	{
 		LLError::setPrintLocation(false);
 		std::string location = errorReturningLocation();
-		
+
 		ensure_message_contains(0, location + "error");
 		ensure_message_contains(1, "die");
 		ensure_message_count(2);
-		
+
 		ensure("fatal callback called", fatalWasCalled);
 	}
 }
@@ -509,7 +515,7 @@ namespace
 	{
 		return "1947-07-08T03:04:05Z";
 	}
-	
+
 	void ufoSighting()
 	{
 		llinfos << "ufo" << llendl;
@@ -517,35 +523,35 @@ namespace
 }
 
 namespace tut
-{	
+{
 	template<> template<>
 		// time in output (for recorders that need it)
 	void ErrorTestObject::test<9>()
 	{
 		LLError::setTimeFunction(roswell);
 
-		mRecorder.setWantsTime(false);
+		mRecorder->setWantsTime(false);
 		ufoSighting();
 		ensure_message_contains(0, "ufo");
 		ensure_message_does_not_contain(0, roswell());
-		
-		mRecorder.setWantsTime(true);
+
+		mRecorder->setWantsTime(true);
 		ufoSighting();
 		ensure_message_contains(1, "ufo");
 		ensure_message_contains(1, roswell());
 	}
-	
+
 	template<> template<>
 		// output order
 	void ErrorTestObject::test<10>()
 	{
 		LLError::setPrintLocation(true);
 		LLError::setTimeFunction(roswell);
-		mRecorder.setWantsTime(true);
+		mRecorder->setWantsTime(true);
 		std::string locationAndFunction = writeReturningLocationAndFunction();
-		
+
 		ensure_equals("order is time type location function message",
-			mRecorder.message(0),
+			mRecorder->message(0),
 			roswell() + " INFO: " + locationAndFunction + ": apple");
 	}
 
@@ -553,30 +559,30 @@ namespace tut
 		// multiple recorders
 	void ErrorTestObject::test<11>()
 	{
-		TestRecorder altRecorder;
-		LLError::addRecorder(&altRecorder);
-		
+		TestRecorder* altRecorder(new TestRecorder);
+		LLError::addRecorder(altRecorder);
+
 		llinfos << "boo" << llendl;
 
 		ensure_message_contains(0, "boo");
-		ensure_equals("alt recorder count", altRecorder.countMessages(), 1);
-		ensure_contains("alt recorder message 0", altRecorder.message(0), "boo");
-		
+		ensure_equals("alt recorder count", altRecorder->countMessages(), 1);
+		ensure_contains("alt recorder message 0", altRecorder->message(0), "boo");
+
 		LLError::setTimeFunction(roswell);
 
-		TestRecorder anotherRecorder;
-		anotherRecorder.setWantsTime(true);
-		LLError::addRecorder(&anotherRecorder);
-		
+		TestRecorder* anotherRecorder(new TestRecorder);
+		anotherRecorder->setWantsTime(true);
+		LLError::addRecorder(anotherRecorder);
+
 		llinfos << "baz" << llendl;
 
 		std::string when = roswell();
-		
+
 		ensure_message_does_not_contain(1, when);
-		ensure_equals("alt recorder count", altRecorder.countMessages(), 2);
-		ensure_does_not_contain("alt recorder message 1", altRecorder.message(1), when);
-		ensure_equals("another recorder count", anotherRecorder.countMessages(), 1);
-		ensure_contains("another recorder message 0", anotherRecorder.message(0), when);
+		ensure_equals("alt recorder count", altRecorder->countMessages(), 2);
+		ensure_does_not_contain("alt recorder message 1", altRecorder->message(1), when);
+		ensure_equals("another recorder count", anotherRecorder->countMessages(), 1);
+		ensure_contains("another recorder message 0", anotherRecorder->message(0), when);
 	}
 }
 
@@ -610,10 +616,10 @@ namespace tut
 	{
 		LLError::setDefaultLevel(LLError::LEVEL_WARN);
 		LLError::setClassLevel("TestBeta", LLError::LEVEL_INFO);
-		
+
 		TestAlpha::doAll();
 		TestBeta::doAll();
-		
+
 		ensure_message_contains(0, "aim west");
 		ensure_message_contains(1, "error");
 		ensure_message_contains(2, "ate eels");
@@ -623,7 +629,7 @@ namespace tut
 		ensure_message_contains(6, "big easy");
 		ensure_message_count(7);
 	}
-	
+
 	template<> template<>
 		// filtering by function, and that it will override class filtering
 	void ErrorTestObject::test<13>()
@@ -632,13 +638,13 @@ namespace tut
 		LLError::setClassLevel("TestBeta", LLError::LEVEL_WARN);
 		LLError::setFunctionLevel("TestBeta::doInfo", LLError::LEVEL_DEBUG);
 		LLError::setFunctionLevel("TestBeta::doError", LLError::LEVEL_NONE);
-		
+
 		TestBeta::doAll();
 		ensure_message_contains(0, "buy iron");
 		ensure_message_contains(1, "bad word");
 		ensure_message_count(2);
 	}
-	
+
 	template<> template<>
 		// filtering by file
 		// and that it is overridden by both class and function filtering
@@ -652,7 +658,7 @@ namespace tut
 									LLError::LEVEL_NONE);
 		LLError::setFunctionLevel("TestBeta::doError",
 									LLError::LEVEL_NONE);
-		
+
 		TestAlpha::doAll();
 		TestBeta::doAll();
 		ensure_message_contains(0, "any idea");
@@ -660,7 +666,7 @@ namespace tut
 		ensure_message_contains(2, "bad word");
 		ensure_message_count(3);
 	}
-	
+
 	template<> template<>
 		// proper cached, efficient lookup of filtering
 	void ErrorTestObject::test<15>()
@@ -690,7 +696,7 @@ namespace tut
 		ensure_message_count(2);
 		ensure_equals("sixth check", LLError::shouldLogCallCount(), 3);
 	}
-	
+
 	template<> template<>
 		// configuration from LLSD
 	void ErrorTestObject::test<16>()
@@ -699,26 +705,26 @@ namespace tut
 		LLSD config;
 		config["print-location"] = true;
 		config["default-level"] = "DEBUG";
-		
+
 		LLSD set1;
 		set1["level"] = "WARN";
 		set1["files"][0] = this_file;
-		
+
 		LLSD set2;
 		set2["level"] = "INFO";
 		set2["classes"][0] = "TestAlpha";
-		
+
 		LLSD set3;
 		set3["level"] = "NONE";
 		set3["functions"][0] = "TestAlpha::doError";
 		set3["functions"][1] = "TestBeta::doError";
-		
+
 		config["settings"][0] = set1;
 		config["settings"][1] = set2;
 		config["settings"][2] = set3;
-		
+
 		LLError::configure(config);
-		
+
 		TestAlpha::doAll();
 		TestBeta::doAll();
 		ensure_message_contains(0, "any idea");
@@ -726,13 +732,13 @@ namespace tut
 		ensure_message_contains(1, "aim west");
 		ensure_message_contains(2, "bad word");
 		ensure_message_count(3);
-		
+
 		// make sure reconfiguring works
 		LLSD config2;
 		config2["default-level"] = "WARN";
-		
+
 		LLError::configure(config2);
-		
+
 		TestAlpha::doAll();
 		TestBeta::doAll();
 		ensure_message_contains(3, "aim west");
@@ -744,13 +750,13 @@ namespace tut
 		ensure_message_contains(8, "big easy");
 		ensure_message_count(9);
 	}
-}	
+}
 
 /* Tests left:
 	handling of classes without LOG_CLASS
 
-	live update of filtering from file	
-	
+	live update of filtering from file
+
 	syslog recorder
 	file recorder
 	cerr/stderr recorder
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index f79acacb22..a4d3a4e026 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -29,10 +29,15 @@
 #if ! defined(LL_WRAPLLERRS_H)
 #define LL_WRAPLLERRS_H
 
+#if LL_WINDOWS
+#pragma warning (disable : 4355) // 'this' used in initializer list: yes, intentionally
+#endif
+
 #include <tut/tut.hpp>
 #include "llerrorcontrol.h"
 #include "stringize.h"
 #include <boost/bind.hpp>
+#include <boost/noncopyable.hpp>
 #include <list>
 #include <string>
 #include <stdexcept>
@@ -81,10 +86,38 @@ struct WrapLL_ERRS
 };
 
 /**
+ * LLError::addRecorder() accepts ownership of the passed Recorder* -- it
+ * expects to be able to delete it later. CaptureLog isa Recorder whose
+ * pointer we want to be able to pass without any ownership implications.
+ * For such cases, instantiate a new RecorderProxy(yourRecorder) and pass
+ * that. Your heap RecorderProxy might later be deleted, but not yourRecorder.
+ */
+class RecorderProxy: public LLError::Recorder
+{
+public:
+    RecorderProxy(LLError::Recorder* recorder):
+        mRecorder(recorder)
+    {}
+
+    virtual void recordMessage(LLError::ELevel level, const std::string& message)
+    {
+        mRecorder->recordMessage(level, message);
+    }
+
+    virtual bool wantsTime()
+    {
+        return mRecorder->wantsTime();
+    }
+
+private:
+    LLError::Recorder* mRecorder;
+};
+
+/**
  * Capture log messages. This is adapted (simplified) from the one in
  * llerror_test.cpp.
  */
-class CaptureLog : public LLError::Recorder
+class CaptureLog : public LLError::Recorder, public boost::noncopyable
 {
 public:
     CaptureLog(LLError::ELevel level=LLError::LEVEL_DEBUG):
@@ -97,16 +130,18 @@ public:
         // with that output. If it turns out that saveAndResetSettings() has
         // some bad effect, give up and just let the DEBUG level log messages
         // display.
-        mOldSettings(LLError::saveAndResetSettings())
+        mOldSettings(LLError::saveAndResetSettings()),
+        mProxy(new RecorderProxy(this))
     {
         LLError::setFatalFunction(wouldHaveCrashed);
         LLError::setDefaultLevel(level);
-        LLError::addRecorder(this);
+        LLError::addRecorder(mProxy);
     }
 
     ~CaptureLog()
     {
-        LLError::removeRecorder(this);
+        LLError::removeRecorder(mProxy);
+        delete mProxy;
         LLError::restoreSettings(mOldSettings);
     }
 
@@ -133,7 +168,7 @@ public:
 
         throw tut::failure(STRINGIZE("failed to find '" << search
                                      << "' in captured log messages:\n"
-                                     << *this));
+                                     << boost::ref(*this)));
     }
 
     std::ostream& streamto(std::ostream& out) const
@@ -155,6 +190,7 @@ public:
     typedef std::list<std::string> MessageList;
     MessageList mMessages;
     LLError::Settings* mOldSettings;
+    LLError::Recorder* mProxy;
 };
 
 inline