[trac3470] Replace static containers with shared pointers in SignalSet

Changed the functions, getRegisteredSignals() and getSignalStates()
to instantiate static shared pointers and return these, rather than
raw pointers to statically declared containers.

Added private shared pointer members to SignalSet, registered_signals_
and signal_states_.  These are initialized within SignalSet constructors
by calling the functions getRegisteredSignals() and getSignalStates()
respectively.  This ensures that both static lists remain inscope until
all SignalSets have been destroyed.

Replaced use of getRegisteredSignals() and getSignalStates() with
new private members, registered_signals_ and signal_states_.

Removed work-around code in src/bin/d2/d_controller.cc put in place
until this ticket was resolved.

src/bin/d2/d_controller.cc

@@ -109,22 +109,10 @@ DControllerBase::launch(int argc, char* argv[], const bool test_mode) {
     try {
         // Now that we have a proces, we can set up signal handling.
         initSignalHandling();
-
         runProcess();
-
-        /// @todo Once Trac #3470 is addressed this will not be necessary.
-        /// SignalSet uses statics which do not free in predicatable order.
-        if (signal_set_) {
-            signal_set_->clear();
-        }
     } catch (const std::exception& ex) {
         LOG_FATAL(dctl_logger, DCTL_PROCESS_FAILED)
                   .arg(app_name_).arg(ex.what());
-        /// @todo Once Trac #3470 is addressed this will not be necessary.
-        /// SignalSet uses statics which do not free in predicatable order.
-        if (signal_set_) {
-            signal_set_->clear();
-        }
         isc_throw (ProcessRunError,
                    "Application process event loop failed: " << ex.what());
     }

src/lib/util/signal_set.cc

@@ -25,25 +25,29 @@ namespace {
 /// @brief Returns a pointer to the global set of registered signals.
 ///
 /// Multiple instances of @c SignalSet may use this pointer to access
-/// and update the set.
+/// and update the set. Note we use a smart pointer so callers can ensure
+/// the static object doesn't go out of scope before they are done with it
+/// during process exit.
 ///
 /// @return Pointer to the global set of registered signals. This pointer
 /// is always initialized and points to a valid object.
-std::set<int>* getRegisteredSignals() {
-    static std::set<int> registered_signals;
-    return (&registered_signals);
+SigIntSetPtr getRegisteredSignals() {
+    static SigIntSetPtr registered_signals(new SigIntSet());
+    return (registered_signals);
 }
 
 /// @brief Returns a pointer to static collection of signals received.
 ///
 /// Multiple instances of @c SignalSet may use this pointer to access
-/// and update the queue of signals received.
+/// and update the queue of signals received.  Note we use a smart pointer
+/// so callers can ensure the static object doesn't go out of scope before
+/// they are done with it during process exit.
 ///
 /// @return Static collection of signals received. This pointer is always
 /// initialized and points to a valid object.
-std::list<int>* getSignalStates() {
-    static std::list<int> states;
-    return (&states);
+SigIntListPtr getSignalStates() {
+    static SigIntListPtr signal_states(new SigIntList());
+    return (signal_states);
 }
 
 /// @brief Internal signal handler for @c isc::util::io::SignalSet class.
@@ -66,7 +70,7 @@ void internalHandler(int sig) {
 
     // Signal is using post-receipt handling, see if we've
     // already received it.
-    std::list<int>* states = getSignalStates();
+    SigIntListPtr states = getSignalStates();
     for (std::list<int>::const_iterator it = states->begin();
         it != states->end(); ++it) {
             if (sig == *it) {
@@ -134,15 +138,22 @@ SignalSet::invokeOnReceiptHandler(int sig) {
 }
 
 SignalSet::SignalSet(const int sig0) {
+    // Copy static pointers to ensure they don't lose scope before we do.
+    registered_signals_ = getRegisteredSignals();
+    signal_states_ = getSignalStates();
     add(sig0);
 }
 
 SignalSet::SignalSet(const int sig0, const int sig1) {
+    registered_signals_ = getRegisteredSignals();
+    signal_states_ = getSignalStates();
     add(sig0);
     add(sig1);
 }
 
 SignalSet::SignalSet(const int sig0, const int sig1, const int sig2) {
+    registered_signals_ = getRegisteredSignals();
+    signal_states_ = getSignalStates();
     add(sig0);
     add(sig1);
     add(sig2);
@@ -188,9 +199,8 @@ SignalSet::clear() {
 
 int
 SignalSet::getNext() const {
-    std::list<int>* states = getSignalStates();
-    for (std::list<int>::iterator it = states->begin();
-         it != states->end(); ++it) {
+    for (std::list<int>::iterator it = signal_states_->begin();
+         it != signal_states_->end(); ++it) {
         if (local_signals_.find(*it) != local_signals_.end()) {
             return (*it);
         }
@@ -206,9 +216,9 @@ SignalSet::erase(const int sig) {
                   " signal set");
     }
     // Remove globally registered signal.
-    getRegisteredSignals()->erase(sig);
+    registered_signals_->erase(sig);
     // Remove unhandled signals from the queue.
-    for (std::list<int>::iterator it = getSignalStates()->begin();
+    for (std::list<int>::iterator it = signal_states_->begin();
          it != getSignalStates()->end(); ++it) {
         if (*it == sig) {
             it = getSignalStates()->erase(it);
@@ -236,13 +246,12 @@ SignalSet::handleNext(SignalHandler signal_handler) {
 
 void
 SignalSet::insert(const int sig) {
-    std::set<int>* global_signals = getRegisteredSignals();
-    if ((global_signals->find(sig) != global_signals->end()) ||
+    if ((registered_signals_->find(sig) != registered_signals_->end()) ||
         (local_signals_.find(sig) != local_signals_.end())) {
         isc_throw(SignalSetError, "attempt to register a duplicate signal "
                   << sig);
     }
-    global_signals->insert(sig);
+    registered_signals_->insert(sig);
     local_signals_.insert(sig);
 }
 
@@ -250,8 +259,8 @@ 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) {
+    for (std::set<int>::const_iterator it = registered_signals_->begin();
+         it != registered_signals_->end(); ++it) {
         sigaddset(&new_set, *it);
     }
     sigprocmask(mask, &new_set, 0);
@@ -259,11 +268,10 @@ SignalSet::maskSignals(const int mask) const {
 
 void
 SignalSet::popNext() {
-    std::list<int>* states = getSignalStates();
-    for (std::list<int>::iterator it = states->begin();
-         it != states->end(); ++it) {
+    for (std::list<int>::iterator it = signal_states_->begin();
+         it != signal_states_->end(); ++it) {
         if (local_signals_.find(*it) != local_signals_.end()) {
-            states->erase(it);
+            signal_states_->erase(it);
             return;
         }
     }

src/lib/util/signal_set.h

@@ -20,6 +20,7 @@
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
 #include <set>
+#include <list>
 #include <signal.h>
 
 namespace isc {
@@ -33,6 +34,18 @@ public:
         isc::Exception(file, line, what) { };
 };
 
+
+/// @brief Defines a set of integer signal identifiers: SIGHUP, SIGTERM...
+typedef std::set<int> SigIntSet;
+/// @gbrief Pointer to a set of signal identifiers
+typedef boost::shared_ptr<SigIntSet> SigIntSetPtr;
+
+/// @brief Defines a list of integer signal identifiers: SIGHUP, SIGTERM...
+typedef std::list<int> SigIntList;
+/// @gbrief Pointer to a list of signal identifiers
+typedef boost::shared_ptr<SigIntList> SigIntListPtr;
+
+
 /// @brief Forward declaration to the @c isc::util::io::SignalSet.
 class SignalSet;
 /// @brief Pointer to the @c isc::util::io::SignalSet.
@@ -229,6 +242,15 @@ private:
     /// @brief Stores the set of signals registered in this signal set.
     std::set<int> local_signals_;
 
+    /// @brief Shared pointer to static set of registered signals
+    /// Set during construction to ensure static set does not lose scope
+    /// before we do.
+    SigIntSetPtr registered_signals_;
+
+    /// @brief Shared pointer to static list of pending signals
+    /// Set during construction to ensure static list does not lose scope
+    /// before we do.
+    SigIntListPtr signal_states_;
 };
 
 }