restore original sigaction in restoreSignalHandler (#254)

* restore original sigaction in restoreSignalHandler

- Save original sigactions in a map called gSavedSigActions
- In restoreSignalHandler, do nothing if there is no saved sigaction.
  If there is a saved sigaction, then re-install it.
- Fixes issue #253

* fix bug found in code review of PR #254

* add functions for reporting sigaction errors

- Factor out reportSigactionError functions from functions that
  call sigaction to allow unit testing.
- Use strsignal libc function to convert from signal number to name
  to eliminate chance of not finding the name in {g,k}Signals maps.

* cleanup. perror doesn't need a wrapper. put code under test

scripts/runAllTests.sh

@@ -4,7 +4,8 @@ set -v
 #set -x
 
 
-test_execs=`find ./test_* -perm /u=x` 
+# test_execs=`find ./test_* -perm /u=x` 
+test_execs=`find ./test_* -type f -perm +ugo+x -print`
 
 echo "Tests to run: $test_execs"
 

src/crashhandler_unix.cpp

@@ -48,7 +48,7 @@ namespace {
    };
 
    std::map<int, std::string> gSignals = kSignals;
-
+   std::map<int, struct sigaction> gSavedSigActions;
 
    bool shouldDoExit() {
       static std::atomic<uint64_t> firstExit{0};
@@ -56,17 +56,6 @@ namespace {
       return (0 == count);
    }
 
-   void restoreSignalHandler(int signal_number) {
-#if !(defined(DISABLE_FATAL_SIGNALHANDLING))
-      struct sigaction action;
-      memset(&action, 0, sizeof (action)); //
-      sigemptyset(&action.sa_mask);
-      action.sa_handler = SIG_DFL; // take default action for the signal
-      sigaction(signal_number, &action, NULL);
-#endif
-   }
-
-
    // Dump of stack,. then exit through g3log background worker
    // ALL thanks to this thread at StackOverflow. Pretty much borrowed from:
    // Ref: http://stackoverflow.com/questions/77005/how-to-generate-a-stacktrace-when-my-gcc-c-app-crashes
@@ -93,15 +82,12 @@ namespace {
       // wait to die
    }
 
-
-
    //
    // Installs FATAL signal handler that is enough to handle most fatal events
    //  on *NIX systems
    void installSignalHandler() {
 #if !(defined(DISABLE_FATAL_SIGNALHANDLING))
       struct sigaction action;
-      memset(&action, 0, sizeof (action));
       sigemptyset(&action.sa_mask);
       action.sa_sigaction = &signalHandler; // callback to crashHandler for fatal signals
       // sigaction to use sa_sigaction file. ref: http://www.linuxprogrammingblog.com/code-examples/sigaction
@@ -109,9 +95,14 @@ namespace {
 
       // do it verbose style - install all signal actions
       for (const auto& sig_pair : gSignals) {
-         if (sigaction(sig_pair.first, &action, nullptr) < 0) {
-            const std::string error = "sigaction - " + sig_pair.second;
-            perror(error.c_str());
+         struct sigaction old_action;
+         memset(&old_action, 0, sizeof (old_action));
+
+         if (sigaction(sig_pair.first, &action, &old_action) < 0) {
+            std::string signalerror = "sigaction - " +  sig_pair.second;
+            perror(signalerror.c_str());
+         } else {
+            gSavedSigActions[sig_pair.first] = old_action;
          }
       }
 #endif
@@ -245,6 +236,38 @@ namespace g3 {
    } // end g3::internal
 
 
+   std::string signalToStr(int signal_number) {
+      std::string signal_name;
+      const char* signal_name_sz = strsignal(signal_number);
+
+      // From strsignal(3): On some systems (but not on Linux), NULL may instead
+      // be returned for an invalid signal number.
+      if (nullptr == signal_name_sz) {
+         signal_name = "Unknown signal " + std::to_string(signal_number);
+      } else {
+         signal_name = signal_name_sz;
+      }
+      return signal_name;
+   }
+
+
+   void restoreSignalHandler(int signal_number) {
+#if !(defined(DISABLE_FATAL_SIGNALHANDLING))
+      auto old_action_it = gSavedSigActions.find(signal_number);
+      if (old_action_it == gSavedSigActions.end()) {
+         return;
+      }
+
+      if (sigaction(signal_number, &(old_action_it->second), nullptr) < 0) {
+         auto signalname = std::string("sigaction - ") + signalToStr(signal_number);
+         perror(signalname.c_str());
+      }
+
+      gSavedSigActions.erase(old_action_it);
+#endif
+   }
+
+
    // This will override the default signal handler setup and instead
    // install a custom set of signals to handle
    void overrideSetupSignals(const std::map<int, std::string> overrideSignals) {

src/g3log/crashhandler.hpp

@@ -38,12 +38,18 @@ namespace g3 {
    void installSignalHandlerForThread();
 #else
    typedef int SignalType;
-
    /// Probably only needed for unit testing. Resets the signal handling back to default
    /// which might be needed in case it was previously overridden
    /// The default signals are: SIGABRT, SIGFPE, SIGILL, SIGSEGV, SIGTERM
    void restoreSignalHandlerToDefault();
 
+
+   std::string signalToStr(int signal_number);
+
+   // restore to whatever signal handler was used before signal handler installation 
+   void restoreSignalHandler(int signal_number);
+
+
    /// Overrides the existing signal handling for custom signals
    /// For example: usage of zcmq relies on its own signal handler for SIGTERM
    ///     so users of g3log with zcmq should then use the @ref overrideSetupSignals

test_unit/test_io.cpp

@@ -376,6 +376,22 @@ namespace {
 	  action.sa_flags = SA_SIGINFO;
 	  sigaction(SIGTERM, &action, nullptr);
    }
+
+
+   std::atomic<bool>  oldSigTermCheck = {false};
+   void customOldSignalHandler(int signal_number, siginfo_t* info, void* unused_context) {
+      lastEncounteredSignal.store(signal_number);
+      oldSigTermCheck.store(true);
+   }
+   void installCustomOldSIGTERM() {
+      struct sigaction action;
+     memset(&action, 0, sizeof(action));
+     sigemptyset(&action.sa_mask);
+     action.sa_sigaction = &customOldSignalHandler;
+     action.sa_flags = SA_SIGINFO;
+     sigaction(SIGTERM, &action, nullptr);
+   }
+
 } // anonymous
 
 // Override of signal handling and testing of it should be fairly easy to port to windows
@@ -412,7 +428,6 @@ TEST(LogTest, FatalSIGTERM__UsingCustomHandler) {
    installCustomSIGTERM();
    EXPECT_EQ(customFatalCounter.load(), size_t{0});
    EXPECT_EQ(lastEncounteredSignal.load(), 0);
-   
 
    raise(SIGTERM);
    logger.reset();
@@ -420,6 +435,32 @@ TEST(LogTest, FatalSIGTERM__UsingCustomHandler) {
    EXPECT_EQ(lastEncounteredSignal.load(), SIGTERM);
    EXPECT_EQ(customFatalCounter.load(), size_t{1});
 }
+
+TEST(LogTest, FatalSIGTERM__VerifyingOldCustomHandler) {
+   RestoreFileLogger logger(log_directory);
+   g_fatal_counter.store(0);
+   customFatalCounter.store(0);
+   lastEncounteredSignal.store(0);
+
+   g3::setFatalPreLoggingHook(fatalCounter);
+   installCustomOldSIGTERM();
+   g3::overrideSetupSignals({ {SIGABRT, "SIGABRT"}, {SIGFPE, "SIGFPE"}, {SIGILL, "SIGILL"}, {SIGTERM, "SIGTERM"}});
+   g3::restoreSignalHandler(SIGTERM); // revert SIGTERM installation
+
+   EXPECT_EQ(customFatalCounter.load(), size_t{0});
+   EXPECT_EQ(lastEncounteredSignal.load(), 0);
+   EXPECT_FALSE(oldSigTermCheck.load());
+   raise(SIGTERM);
+   logger.reset();
+   EXPECT_EQ(g_fatal_counter.load(), size_t{0});
+   EXPECT_EQ(lastEncounteredSignal.load(), SIGTERM);
+   EXPECT_TRUE(oldSigTermCheck.load()); 
+}
+
+
+
+
+
 #endif
 
 #endif