rename crash handler to hook, and make them chainable by putting back the get and documenting it

8 files changed, 77 insertions, 42 deletions

diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 6fa9e590cb..5f3edf7f84 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -483,7 +483,7 @@ namespace LLError
 		LevelMap                            mTagLevelMap;
 		std::map<std::string, unsigned int> mUniqueLogMessages;
 		
-		LLError::FatalFunction              mCrashFunction;
+		LLError::FatalHook                  mFatalHook;
 		LLError::TimeFunction               mTimeFunction;
 		
 		Recorders                           mRecorders;
@@ -523,7 +523,7 @@ namespace LLError
 		mFileLevelMap(),
 		mTagLevelMap(),
 		mUniqueLogMessages(),
-		mCrashFunction(NULL),
+		mFatalHook(NULL),
 		mTimeFunction(NULL),
 		mRecorders(),
 		mFileRecorder(),
@@ -719,16 +719,16 @@ namespace LLError
 		commonInit(user_dir, app_dir, log_to_stderr);
 	}
 
-	void setFatalHandler(const FatalFunction& fatal_function)
+	void setFatalHook(const FatalHook& fatal_hook)
 	{
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
-		s->mCrashFunction = fatal_function;
+		s->mFatalHook = fatal_hook;
 	}
 
-    void restoreCrashOnError()
+	FatalHook getFatalHook()
 	{
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
-		s->mCrashFunction = NULL;
+		return s->mFatalHook;
 	}
 
 	std::string getFatalMessage()
@@ -1306,7 +1306,7 @@ namespace LLError
 		return ;
 	}
 
-	ErrCrashHandlerResult Log::flush(std::ostringstream* out, const CallSite& site)
+	ErrFatalHookResult Log::flush(std::ostringstream* out, const CallSite& site)
 	{
 		LLMutexTrylock lock(&gLogMutex,5);
 		if (!lock.isLocked())
@@ -1369,9 +1369,9 @@ namespace LLError
 
 		if (site.mLevel == LEVEL_ERROR)
 		{
-            if (s->mCrashFunction)
+            if (s->mFatalHook)
             {
-                return s->mCrashFunction(message);
+                return s->mFatalHook(message);
             }
             else
             {
diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h
index 2a73ccb36a..6bdb2e852f 100644
--- a/indra/llcommon/llerror.h
+++ b/indra/llcommon/llerror.h
@@ -194,7 +194,7 @@ namespace LLError
 	
 	struct CallSite;
 	
-    enum ErrCrashHandlerResult { ERR_DO_NOT_CRASH, ERR_CRASH };
+    enum ErrFatalHookResult { ERR_DO_NOT_CRASH, ERR_CRASH };
 
 	class LL_COMMON_API Log
 	{
@@ -205,7 +205,7 @@ namespace LLError
 		static void flush(std::ostringstream* out, char* message);
 
         // returns false iff the calling macro should crash
-		static ErrCrashHandlerResult flush(std::ostringstream*, const CallSite&);
+		static ErrFatalHookResult flush(std::ostringstream*, const CallSite&);
 
 		static std::string demangle(const char* mangled);
 	};
@@ -272,7 +272,7 @@ namespace LLError
    //when LLAppViewer::handleViewerCrash() is triggered.
    //
    //Note: to be simple, efficient and necessary to keep track of correct call stacks, 
-	//LLCallStacks is designed not to be thread-safe.
+   //LLCallStacks is designed not to be thread-safe.
    //so try not to use it in multiple parallel threads at same time.
    //Used in a single thread at a time is fine.
    class LL_COMMON_API LLCallStacks
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index 2fa220ba5a..e7c5241cc9 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -93,22 +93,24 @@ namespace LLError
 		Control functions.
 	*/
 
-    // A FatalFunction is called if set using setFatalHandler; its return controls
+    // A FatalHook is called if set using setFatalHook; its return controls
     // whether or not the calling error logging code should crash.
     // ERR_DO_NOT_CRASH should be used only in test code.
-	typedef boost::function<LLError::ErrCrashHandlerResult(const std::string&)> FatalFunction;
-    
-
-	/// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code
-	LL_COMMON_API void setFatalHandler(const FatalFunction&);
-		// The fatal function will be called when an message of LEVEL_ERROR
+	typedef boost::function<LLError::ErrFatalHookResult(const std::string&)> FatalHook;
+
+	/// Supplement and control the default behavior of crashing on LL_ERRS
+    /// This may be used to suppress crashes only in test code;
+    /// otherwise, the FatalHook should always either return 
+    /// the result from the previous hook (see getFatalHook below),
+    /// or return LLError::ERR_CRASH
+	LL_COMMON_API void setFatalHook(const FatalHook& fatal_hook);
+		// The fatal_hook function will be called when an message of LEVEL_ERROR
 		// is logged.  Note: supressing a LEVEL_ERROR message from being logged
-		// (by, for example, setting a class level to LEVEL_NONE), will keep
-		// the that message from causing the fatal funciton to be invoked.
-        // The 
+		// (by, for example, setting a class level to LEVEL_NONE), will also
+		// prevent the fatal_hook being called and the resulting deliberate crash
 
-    /// Undo the effect of the setFatalHandler above
-	LL_COMMON_API void restoreCrashOnError();
+    /// Retrieve the previously-set FatalHook
+	LL_COMMON_API FatalHook getFatalHook();
 
 	LL_COMMON_API std::string getFatalMessage();
 		// Retrieve the message last passed to LL_ERRS, if any
diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp
index bf20c87c89..161d25bc34 100644
--- a/indra/llcommon/llleap.cpp
+++ b/indra/llcommon/llleap.cpp
@@ -59,7 +59,7 @@ public:
         // pump name -- so it should NOT need tweaking for uniqueness.
         mReplyPump(LLUUID::generateNewID().asString()),
         mExpect(0),
-
+        mPrevFatalHook(LLError::getFatalHook()),
         // Instantiate a distinct LLLeapListener for this plugin. (Every
         // plugin will want its own collection of managed listeners, etc.)
         // Pass it a callback to our connect() method, so it can send events
@@ -145,6 +145,9 @@ public:
         mStderrConnection = childerr.getPump()
             .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1));
 
+        // For our lifespan, intercept any LL_ERRS so we can notify plugin
+        LLError::setFatalHook(boost::bind(&LLLeapImpl::fatalHook, this, _1));
+
         // Send child a preliminary event reporting our own reply-pump name --
         // which would otherwise be pretty tricky to guess!
         wstdin(mReplyPump.getName(),
@@ -159,6 +162,8 @@ public:
     virtual ~LLLeapImpl()
     {
         LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL;
+        // Restore original FatalHook
+        LLError::setFatalHook(mPrevFatalHook);
     }
 
     // Listener for failed launch attempt
@@ -372,6 +377,30 @@ public:
         return false;
     }
 
+    LLError::ErrFatalHookResult fatalHook(const std::string& error)
+    {
+        // Notify plugin
+        LLSD event;
+        event["type"] = "error";
+        event["error"] = error;
+        mReplyPump.post(event);
+
+        // All the above really accomplished was to buffer the serialized
+        // event in our WritePipe. Have to pump mainloop a couple times to
+        // really write it out there... but time out in case we can't write.
+        LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN));
+        LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop"));
+        LLSD nop;
+        F64 until = (LLTimer::getElapsedSeconds() + 2).value();
+        while (childin.size() && LLTimer::getElapsedSeconds() < until)
+        {
+            mainloop.post(nop);
+        }
+
+        // forward the call to the previous FatalHook, default to crashing if there isn't one
+        return mPrevFatalHook ? mPrevFatalHook(error) : LLError::ERR_CRASH;
+    }
+
 private:
     /// We always want to listen on mReplyPump with wstdin(); under some
     /// circumstances we'll also echo other LLEventPumps to the plugin.
@@ -392,6 +421,7 @@ private:
         mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection;
     boost::scoped_ptr<LLEventPump::Blocker> mBlocker;
     LLProcess::ReadPipe::size_type mExpect;
+    LLError::FatalHook mPrevFatalHook;
     boost::scoped_ptr<LLLeapListener> mListener;
 };
 
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index 9dcb5c145f..9895abc1f3 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -70,7 +70,7 @@ namespace
 namespace
 {
 	static bool fatalWasCalled;
-    LLError::ErrCrashHandlerResult fatalCall(const std::string&)
+    LLError::ErrFatalHookResult fatalHook(const std::string&)
     {
         fatalWasCalled = true;
         return LLError::ERR_DO_NOT_CRASH;
@@ -124,7 +124,7 @@ namespace tut
 
 			mPriorErrorSettings = LLError::saveAndResetSettings();
 			LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
-			LLError::setFatalHandler(fatalCall);
+			LLError::setFatalHook(fatalHook);
 			LLError::addRecorder(mRecorder);
 		}
 
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index a412e12a04..a6c44d5fdd 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -46,7 +46,7 @@
 
 // statically reference the function in test.cpp... it's short, we could
 // replicate, but better to reuse
-extern LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message);
+extern LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message);
 
 struct WrapLLErrs
 {
@@ -55,14 +55,15 @@ struct WrapLLErrs
         // stderr. Otherwise, expected llerrs (LL_ERRS) messages clutter the
         // console output of successful tests, potentially confusing things.
     :mPriorErrorSettings(LLError::saveAndResetSettings())
+    ,mPriorFatalHook(LLError::getFatalHook())
     {
         // Make LL_ERRS call our own operator() method
-        LLError::setFatalHandler(boost::bind(&WrapLLErrs::operator(), this, _1));
+        LLError::setFatalHook(boost::bind(&WrapLLErrs::operator(), this, _1));
     }
 
     ~WrapLLErrs()
     {
-        LLError::restoreCrashOnError();
+        LLError::setFatalHook(mPriorFatalHook);
         LLError::restoreSettings(mPriorErrorSettings);
     }
 
@@ -71,7 +72,7 @@ struct WrapLLErrs
         FatalException(const std::string& what): LLException(what) {}
     };
 
-    LLError::ErrCrashHandlerResult operator()(const std::string& message)
+    LLError::ErrFatalHookResult operator()(const std::string& message)
     {
         // Save message for later in case consumer wants to sense the result directly
         error = message;
@@ -107,7 +108,7 @@ struct WrapLLErrs
 
     std::string error;
     LLError::SettingsStoragePtr mPriorErrorSettings;
-    LLError::FatalFunction mPriorFatal;
+    LLError::FatalHook mPriorFatalHook;
 };
 
 /**
@@ -197,11 +198,12 @@ 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.
-		: boost::noncopyable(),
-        mOldSettings(LLError::saveAndResetSettings()),
-		mRecorder(new CaptureLogRecorder())
+		: boost::noncopyable()
+        , mOldSettings(LLError::saveAndResetSettings())
+		, mPriorFatalHook(LLError::getFatalHook())
+		, mRecorder(new CaptureLogRecorder())
     {
-        LLError::setFatalHandler(wouldHaveCrashed);
+        LLError::setFatalHook(wouldHaveCrashed);
         LLError::setDefaultLevel(level);
         LLError::addRecorder(mRecorder);
     }
@@ -210,7 +212,7 @@ public:
     {
         LLError::removeRecorder(mRecorder);
         LLError::restoreSettings(mOldSettings);
-        LLError::restoreCrashOnError();
+        LLError::setFatalHook(mPriorFatalHook);
     }
 
     /// Don't assume the message we want is necessarily the LAST log message
@@ -228,6 +230,7 @@ public:
 
 private:
     LLError::SettingsStoragePtr mOldSettings;
+    LLError::FatalHook   mPriorFatalHook;
 	LLError::RecorderPtr mRecorder;
 };
 
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index 8b9f9085b1..8616fe7c76 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -2134,7 +2134,7 @@ bool LLAppViewer::initThreads()
 	return true;
 }
 
-LLError::ErrCrashHandlerResult fatalErrorHandler(const std::string &error_string)
+LLError::ErrFatalHookResult fatalErrorHook(const std::string &error_string)
 {
 #ifndef LL_RELEASE_FOR_DOWNLOAD
 	OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK);
@@ -2166,7 +2166,7 @@ void LLAppViewer::initLoggingAndGetLastDuration()
 	LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "")
                                 ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "")
                                 );
-	LLError::setFatalHandler(fatalErrorHandler);
+	LLError::setFatalHook(fatalErrorHook);
 
 	// Remove the last ".old" log file.
 	std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS,
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index 4f966aede8..52ac855d9f 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -75,7 +75,7 @@
 
 #include <fstream>
 
-LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message)
+LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message)
 {
 	tut::fail("fatal error message: " + message);
     return LLError::ERR_DO_NOT_CRASH;
@@ -150,7 +150,7 @@ public:
 		mOldSettings(LLError::saveAndResetSettings()),
 		mRecorder(new RecordToTempFile(pool))
 	{
-		LLError::setFatalHandler(wouldHaveCrashed);
+		LLError::setFatalHook(wouldHaveCrashed);
 		LLError::setDefaultLevel(level);
 		LLError::addRecorder(mRecorder);
 	}
@@ -531,7 +531,7 @@ int main(int argc, char **argv)
 		LLError::initForApplication(".", ".", false /* do not log to stderr */);
 		LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
 	}	
-	LLError::setFatalHandler(wouldHaveCrashed);
+	LLError::setFatalHook(wouldHaveCrashed);
 	std::string test_app_name(argv[0]);
 	std::string test_log = test_app_name + ".log";
 	LLFile::remove(test_log);