From: Thomas Markwalder Date: Mon, 20 Oct 2014 14:27:45 +0000 (-0400) Subject: [trac3470] Replace static containers with shared pointers in SignalSet X-Git-Tag: trac3629a_base~10^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=790148904de54be7e9a7505938abffaada9729fc;p=thirdparty%2Fkea.git [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. --- diff --git a/src/bin/d2/d_controller.cc b/src/bin/d2/d_controller.cc index 748907963d..71c188f014 100644 --- a/src/bin/d2/d_controller.cc +++ b/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()); } diff --git a/src/lib/util/signal_set.cc b/src/lib/util/signal_set.cc index e39e1063aa..32051a0a51 100644 --- a/src/lib/util/signal_set.cc +++ b/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* getRegisteredSignals() { - static std::set registered_signals; - return (®istered_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* getSignalStates() { - static std::list 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* states = getSignalStates(); + SigIntListPtr states = getSignalStates(); for (std::list::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* states = getSignalStates(); - for (std::list::iterator it = states->begin(); - it != states->end(); ++it) { + for (std::list::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::iterator it = getSignalStates()->begin(); + for (std::list::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* 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::const_iterator it = getRegisteredSignals()->begin(); - it != getRegisteredSignals()->end(); ++it) { + for (std::set::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* states = getSignalStates(); - for (std::list::iterator it = states->begin(); - it != states->end(); ++it) { + for (std::list::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; } } diff --git a/src/lib/util/signal_set.h b/src/lib/util/signal_set.h index d10f0ce58a..7380f7db95 100644 --- a/src/lib/util/signal_set.h +++ b/src/lib/util/signal_set.h @@ -20,6 +20,7 @@ #include #include #include +#include #include 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 SigIntSet; +/// @gbrief Pointer to a set of signal identifiers +typedef boost::shared_ptr SigIntSetPtr; + +/// @brief Defines a list of integer signal identifiers: SIGHUP, SIGTERM... +typedef std::list SigIntList; +/// @gbrief Pointer to a list of signal identifiers +typedef boost::shared_ptr 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 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_; }; }