[3407] Added on-receipt signal handling to util::SignalSet

util::SignalSet now supports registering a signal handler
from within it's internal handler.  This allows a custom
handler to process signals as soon as they occur, rather
than through deferred processing.

src/lib/util/signal_set.cc

@@ -48,29 +48,87 @@ std::list<int>* getSignalStates() {
 
 /// @brief Internal signal handler for @c isc::util::io::SignalSet class.
 ///
-/// This signal handler adds a signal number for which it is being
-/// invoked to the queue of received signals. It prevents adding duplicated
-/// signals. All duplicated signals are dropped. This prevents hammering
-/// a process to invoke handlers (e.g. DHCP server reconfiguration), when
-/// many the same signals are received one after another.
+/// This handler catches all registered signals. When a signal arrives it
+/// passes the signal to invokeOnReceiptHandler for "on-receipt" processing.
+/// If this processing returns true if exists without further action,
+/// otherwise it adds the signal number to the queue of received signals.
+/// It prevents adding duplicated signals. All duplicated signals are dropped.
+/// This prevents hammering a process to invoke handlers (e.g. DHCP server
+/// reconfiguration), when many of the same signals are received one after
+/// another.
 ///
 /// @param sig Signal number.
 void internalHandler(int sig) {
+    if (SignalSet::invokeOnReceiptHandler(sig)) {
+        // Signal has been handled by the on-receipt handler.
+        return;
+    }
+
+    // Signal is using post-receipt handling, see if we've
+    // already received it.
     std::list<int>* states = getSignalStates();
     for (std::list<int>::const_iterator it = states->begin();
-         it != states->end(); ++it) {
-        if (sig == *it) {
-            return;
+        it != states->end(); ++it) {
+            if (sig == *it) {
+                return;
         }
     }
+
+    // First occurrence, so save it.
     states->push_back(sig);
 }
 
-}
+}; // end anon namespace
 
 namespace isc {
 namespace util {
 
+const BoolSignalHandler SignalSet::EMPTY_BOOL_HANDLER = BoolSignalHandler();
+BoolSignalHandler SignalSet::onreceipt_handler_ = EMPTY_BOOL_HANDLER;
+
+bool
+SignalSet::invokeOnReceiptHandler(int sig) {
+    if (!SignalSet::onreceipt_handler_) {
+        return (false);
+    }
+
+    // First we set the signal to SIG_IGN.  This causes any repeat occurrences
+    // to be discarded, not deferred as they would be if blocked.   Note that
+    // we save the current sig action so we can restore it later.
+    struct sigaction sa;
+    struct sigaction prev_sa;
+    memset(&sa, 0, sizeof(sa));
+    sa.sa_handler = SIG_IGN;
+    if (sigaction(sig, &sa, &prev_sa) < 0) {
+        // Highly unlikely we can get here.
+        const char* errmsg = strerror(errno);
+        isc_throw(SignalSetError, "failed to set SIG_IGN for signal "
+                  << sig << ": " << errmsg);
+    }
+
+    // Call the registered handler.
+    bool signal_processed = false;
+    try {
+        signal_processed = SignalSet::onreceipt_handler_(sig);
+    } catch (const std::exception& ex) {
+        // Restore the handler.  We might fail to restore it, but we likely
+        // have bigger issues anyway.
+        sigaction(sig, &prev_sa, 0);
+        isc_throw(SignalSetError, "onreceipt_handler failed for signal "
+                  << sig << ": " << ex.what());
+    }
+
+    // Restore the sig action to reenable handling this signal.
+    if (sigaction(sig, &prev_sa, 0) < 0) {
+        // Highly unlikely we can get here.
+        const char* errmsg = strerror(errno);
+        isc_throw(SignalSetError, "failed to restore handler for signal "
+                  << sig << ": " << errmsg);
+    }
+
+    return (signal_processed);
+}
+
 SignalSet::SignalSet(const int sig0) {
     add(sig0);
 }
@@ -187,6 +245,7 @@ SignalSet::insert(const int sig) {
 void
 SignalSet::maskSignals(const int mask) const {
     sigset_t new_set;
+    sigemptyset(&new_set);
     for (std::set<int>::const_iterator it = getRegisteredSignals()->begin();
          it != getRegisteredSignals()->end(); ++it) {
         sigaddset(&new_set, *it);
@@ -225,5 +284,15 @@ SignalSet::remove(const int sig) {
     }
 }
 
+void
+SignalSet::setOnReceiptHandler(BoolSignalHandler handler) {
+    onreceipt_handler_ = handler;
+}
+
+void
+SignalSet::clearOnReceiptHandler() {
+    onreceipt_handler_ = EMPTY_BOOL_HANDLER;
+}
+
 } // end of isc::util
 } // end of isc

src/lib/util/signal_set.h

@@ -40,26 +40,48 @@ typedef boost::shared_ptr<SignalSet> SignalSetPtr;
 /// @brief Pointer to the signal handling function.
 typedef boost::function<void(int signum)> SignalHandler;
 
+/// @brief Pointer to a signal handling function which returns bool result.
+///
+/// The handler is expected to return true if the signal it was given has
+/// been processed (i.e. should not be recorded for deferred processing) or
+/// false in which case it will be recorded.
+typedef boost::function<bool(int signum)> BoolSignalHandler;
+
 /// @brief Represents a collection of signals handled in a customized way.
 ///
 /// Kea processes must handle selected signals in a specialized way. For
 /// example: SIGINT and SIGTERM must perform a graceful shut down of the
 /// server. The SIGHUP signal is used to trigger server's reconfiguration.
 ///
-/// This class allows specifying signals which should be handled in a
-/// specialized way as well as specifying a signal handler function.
-/// When a signal is received the signal handler function is called and
-/// the code of the received signal is recorded. This function doesn't
-/// do anything beyond recording the signal number to minimize the time
-/// spent on handling the signal and process interruption. The process
-/// can later check the signals received and call the handlers on its
-/// descretion by calling a @c isc::util::io::SignalSet::handleNext function.
+/// This class allows the caller to register one or more signals to catch
+/// and process.  Signals may be handled either immediately upon arrival and/or
+/// recorded and processed later.  To process signals immediately, the caller
+/// must register an "on-receipt" handler.  This handler is expected to return
+/// a true or false indicating whether or not the signal has been processed.
+/// Signal occurrences that are not processed by the on-receipt handler are
+/// remembered by SignalSet for deferred processing.  The caller can then query
+/// SignalSet at their discretion to determine if any signal occurrences are
+/// pending and process them.
+///
+/// SignalSet uses an internal handler to catch all registered signals. When
+/// a signal arrives the internal handler will first attempt to invoke the
+/// on-receipt handler.  If one has been registered it is passed the
+/// signal value as an argument and if it returns true upon completion, the
+/// internal handler will exit without further action.  If the on-receipt
+/// handler returned false or one is not registered, then internal handler
+/// will record the signal value for deferred processing.  Note that once a
+/// particular signal has been recorded, any further occurrences of that signal
+/// will be discarded until the original occurrence has been processed.  This
+/// guards against rapid-fire occurrences of the same signal.
 ///
 /// @note This class is not thread safe. It uses static variables and
 /// functions to track a global state of signal registration and received
 /// signals' queue.
 class SignalSet : public boost::noncopyable {
 public:
+    /// @brief Optional handler to execute at the time of signal receipt
+    static BoolSignalHandler onreceipt_handler_;
+    static const BoolSignalHandler EMPTY_BOOL_HANDLER;
 
     /// @brief Constructor installing one signal.
     ///
@@ -128,6 +150,38 @@ public:
     /// @param sig A code of the signal to be removed.
     void remove(const int sig);
 
+    /// @brief Registers a handler as the onreceipt signal handler
+    ///
+    /// Sets the given handler as the handler to invoke immediately
+    /// upon receipt of a a registered signal.
+    ///
+    /// @note Currently, the on-receipt handler is stored as a static
+    /// value and hence there may only be one such handler at a time
+    /// for a given process.
+    ///
+    /// @param handler the signal handler to register
+    static void setOnReceiptHandler(BoolSignalHandler handler);
+
+    /// @brief Unregeisters the onreceipt signal handler
+    static void clearOnReceiptHandler();
+
+    /// @brief Invokes the onreceipt handler if it exists
+    ///
+    /// This static method is used by @c isc::util::io::SignalSet class to
+    /// invoke the registered handler (if one) immediately upon receipt of
+    /// a registered signal.
+    ///
+    /// Prior to invoking the handler, it sets signal action for the given
+    /// signal to SIG_IGN which prevents any repeat signal occurences from
+    /// queuing while the handler is executing.  Upon completion of the handler,
+    /// the signal action is restored which reenables receipt and handling of
+    /// the signal.
+    ///
+    /// @param sig Signal number.
+    /// @return Boolean false if no on-receipt handler was registered,
+    /// otherwise it is the value returned by the on-receipt handler.
+    static bool invokeOnReceiptHandler(int sig);
+
 private:
 
     /// @brief Blocks signals in the set.
@@ -178,6 +232,7 @@ private:
 
     /// @brief Stores the set of signals registered in this signal set.
     std::set<int> local_signals_;
+
 };
 
 }

src/lib/util/tests/signal_set_unittest.cc

@@ -37,7 +37,8 @@ public:
     /// signal handler function.
     SignalSetTest()
         : signal_set_(),
-          secondary_signal_set_() {
+          secondary_signal_set_(),
+          onreceipt_trues_(0) {
         handler_calls_ = 0;
         signum_ = -1;
     }
@@ -54,7 +55,7 @@ public:
         }
     }
 
-    /// @brief Signal handler used by unit tests.
+    /// @brief Deferred processing signal handler used by unit tests.
     ///
     /// @param signum Signal being handled.
     static void testHandler(int signum) {
@@ -62,6 +63,22 @@ public:
         ++handler_calls_;
     }
 
+    /// @brief Immediate processing signal handler used by unit tests.
+    ///
+    /// The handler processes only SIGHUP.  All others will pass through.
+    ///
+    /// @param signum Signal being handled.
+    /// @return Boolean true if the handler has processed the signal, false
+    /// otherwise.
+    bool onReceiptHandler(int signum) {
+        if (signum == SIGHUP) {
+            ++onreceipt_trues_;
+            return (true);
+        }
+
+        return (false);
+    }
+
     /// @brief Number of handler calls so far.
     static int handler_calls_;
     /// @brief The last signal handled.
@@ -70,6 +87,8 @@ public:
     SignalSetPtr signal_set_;
     /// @brief Second signal set object.
     SignalSetPtr secondary_signal_set_;
+    /// @brief Number of true returns from onReceiptHandler so far.
+    int onreceipt_trues_;
 };
 
 int SignalSetTest::handler_calls_ = 0;
@@ -184,5 +203,47 @@ TEST_F(SignalSetTest, duplicates) {
     EXPECT_THROW(other->add(SIGHUP), SignalSetError);
 }
 
+/// Check that on-receipt processing works.
+TEST_F(SignalSetTest, onReceiptTests) {
+    // Install an on-receipt handler.
+    SignalSet::setOnReceiptHandler(boost::bind(&SignalSetTest::onReceiptHandler,
+                                               this, _1));
+    // Create a SignalSet for SIGHUP and SIGUSR1.
+    ASSERT_NO_THROW(signal_set_.reset(new SignalSet(SIGHUP, SIGUSR1)));
+
+    // Generate SIGHUP, which the on-receipt handler should process.
+    // Verify that the on-receipt handler processed the signal and that
+    // no signals are pending.
+    ASSERT_EQ(0, raise(SIGHUP));
+    EXPECT_EQ(1, onreceipt_trues_);
+    EXPECT_EQ(-1, signal_set_->getNext());
+
+    // Generate SIGHUP, which the on-receipt handler should NOT process.
+    // Verify the on-receipt handler did not the signal and that SIGUSR1
+    // is pending.
+    ASSERT_EQ(0, raise(SIGUSR1));
+    EXPECT_EQ(1, onreceipt_trues_);
+    EXPECT_EQ(SIGUSR1, signal_set_->getNext());
+
+    // Verify we can process SIGUSR1 with the deferred handler.
+    signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, _1));
+    EXPECT_EQ(1, handler_calls_);
+    EXPECT_EQ(SIGUSR1, signum_);
+
+    // Unregister the on-receipt handler.
+    SignalSet::clearOnReceiptHandler();
+
+    // Generate SIGHUP.
+    // Verify that the on-receipt handler did not process the signal, and
+    // that SIGHUP is pending.
+    ASSERT_EQ(0, raise(SIGHUP));
+    EXPECT_EQ(1, onreceipt_trues_);
+    EXPECT_EQ(SIGHUP, signal_set_->getNext());
+
+    // Verify we can process it with deferred handler.
+    signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, _1));
+    EXPECT_EQ(2, handler_calls_);
+    EXPECT_EQ(SIGHUP, signum_);
+}
 
 } // end of anonymous namespace