From eb1bea222322385e6e5b05206f09f21bb891f3f7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 23 Apr 2012 11:26:18 -0400 Subject: 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 to make the ownership transfer more explicit -- or even boost::shared_ptr 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. --- indra/test/test.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'indra/test') diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 128d84e428..06128e0902 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -37,6 +37,7 @@ #include "linden_common.h" #include "llerrorcontrol.h" #include "lltut.h" +#include "tests/wrapllerrs.h" // RecorderProxy #include "stringize.h" #include "namedtempfile.h" @@ -91,22 +92,24 @@ public: virtual void replay(std::ostream&) {} }; -class LLReplayLogReal: public LLReplayLog, public LLError::Recorder +class LLReplayLogReal: public LLReplayLog, public LLError::Recorder, public boost::noncopyable { public: LLReplayLogReal(LLError::ELevel level, apr_pool_t* pool): mOldSettings(LLError::saveAndResetSettings()), + mProxy(new RecorderProxy(this)), mTempFile("log", "", pool), // create file mFile(mTempFile.getName().c_str()) // open it { LLError::setFatalFunction(wouldHaveCrashed); LLError::setDefaultLevel(level); - LLError::addRecorder(this); + LLError::addRecorder(mProxy); } virtual ~LLReplayLogReal() { - LLError::removeRecorder(this); + LLError::removeRecorder(mProxy); + delete mProxy; LLError::restoreSettings(mOldSettings); } @@ -134,6 +137,7 @@ public: private: LLError::Settings* mOldSettings; + LLError::Recorder* mProxy; NamedTempFile mTempFile; std::ofstream mFile; }; -- cgit v1.3