From: Marcin Siodelski Date: Tue, 3 Jun 2014 15:42:52 +0000 (+0200) Subject: [3405] Signal set can handle a signal it owns. X-Git-Tag: trac3434_base~11^2~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7d55bfdbf5b70049a4a48646f2e1f03a751c126b;p=thirdparty%2Fkea.git [3405] Signal set can handle a signal it owns. --- diff --git a/src/lib/util/io/signal_set.cc b/src/lib/util/io/signal_set.cc index 87b9e1a4bb..694b77d3bb 100644 --- a/src/lib/util/io/signal_set.cc +++ b/src/lib/util/io/signal_set.cc @@ -65,18 +65,6 @@ void internalHandler(int sig) { states->push_back(sig); } -/// @brief Pops a next signal number from the static collection of signals. -/// -/// The static collection of signals is updated by the internal signal -/// handler being invoked when one of the installed signals is received by -/// the process. This function removes the first element of the collection. -void popNext() { - std::list* states = getSignalStates(); - if (!states->empty()) { - states->pop_front(); - } -} - } namespace isc { @@ -137,10 +125,13 @@ SignalSet::clear() { int SignalSet::getNext() const { std::list* states = getSignalStates(); - if (states->empty()) { - return (-1); + for (std::list::iterator it = states->begin(); + it != states->end(); ++it) { + if (local_signals_.find(*it) != local_signals_.end()) { + return (*it); + } } - return (*states->begin()); + return (-1); } void @@ -150,7 +141,16 @@ SignalSet::erase(const int sig) { << " from a signal set: signal is not owned by the" " signal set"); } + // Remove globally registered signal. getRegisteredSignals()->erase(sig); + // Remove unhandled signals from the queue. + for (std::list::iterator it = getSignalStates()->begin(); + it != getSignalStates()->end(); ++it) { + if (*it == sig) { + it = getSignalStates()->erase(it); + } + } + // Remove locally registered signal. local_signals_.erase(sig); } @@ -192,6 +192,18 @@ SignalSet::maskSignals(const int mask) const { sigprocmask(mask, &new_set, 0); } +void +SignalSet::popNext() { + std::list* states = getSignalStates(); + for (std::list::iterator it = states->begin(); + it != states->end(); ++it) { + if (local_signals_.find(*it) != local_signals_.end()) { + states->erase(it); + return; + } + } +} + void SignalSet::remove(const int sig) { // Unregister only if we own this signal. diff --git a/src/lib/util/io/signal_set.h b/src/lib/util/io/signal_set.h index 914a06cd06..5d52559568 100644 --- a/src/lib/util/io/signal_set.h +++ b/src/lib/util/io/signal_set.h @@ -26,6 +26,8 @@ namespace isc { namespace util { namespace io { +/// @brief Exception thrown when the @c isc::util::io::SignalSet class +/// experiences an error. class SignalSetError : public Exception { public: SignalSetError(const char* file, size_t line, const char* what) : @@ -161,6 +163,13 @@ private: /// @param mask A mask to be applied to all signals. void maskSignals(const int mask) const; + /// @brief Pops a next signal number from the static collection of signals. + /// + /// The static collection of signals is updated by the internal signal + /// handler being invoked when one of the installed signals is received by + /// the process. This function removes the first element of the collection. + void popNext(); + /// @brief Unblocks signals in the set. /// /// This function unblocks the signals in a set. diff --git a/src/lib/util/io/tests/signal_set_unittest.cc b/src/lib/util/io/tests/signal_set_unittest.cc index 27cc24bac4..9fcd4fda47 100644 --- a/src/lib/util/io/tests/signal_set_unittest.cc +++ b/src/lib/util/io/tests/signal_set_unittest.cc @@ -23,34 +23,53 @@ namespace { using namespace isc; using namespace isc::util::io; +/// @brief Test fixture class for @c isc::util::io::SignalSet class. +/// +/// This class contains a handler function which records the signal +/// being handled. It allows for checking whether the signal set +/// has invoked the handler for the expected signal. class SignalSetTest : public ::testing::Test { public: + /// @brief Constructor. + /// + /// Resets the signal sets and variables being modified by the + /// signal handler function. SignalSetTest() - : signal_set_() { + : signal_set_(), + secondary_signal_set_() { handler_calls_ = 0; signum_ = -1; } + /// @brief Destructor. + /// + /// Uninstalls the signals from the signal sets. ~SignalSetTest() { if (signal_set_) { signal_set_->clear(); } + if (secondary_signal_set_) { + secondary_signal_set_->clear(); + } } - void handleNext() { - signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, _1)); - } - + /// @brief Signal handler used by unit tests. + /// + /// @param signum Signal being handled. static void testHandler(int signum) { signum_ = signum; ++handler_calls_; } + /// @brief Number of handler calls so far. static int handler_calls_; + /// @brief The last signal handled. static int signum_; + /// @brief Test signal set object. SignalSetPtr signal_set_; - + /// @brief Second signal set object. + SignalSetPtr secondary_signal_set_; }; int SignalSetTest::handler_calls_ = 0; @@ -77,14 +96,14 @@ TEST_F(SignalSetTest, twoSignals) { // second one should be dropped. ASSERT_EQ(0, raise(SIGHUP)); // Execute the first handler (for SIGHUP). - handleNext(); + signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, _1)); // The handler should have been called once and the signal // handled should be SIGHUP. EXPECT_EQ(1, handler_calls_); EXPECT_EQ(SIGHUP, signum_); // Next signal to be handled should be SIGINT. EXPECT_EQ(SIGINT, signal_set_->getNext()); - handleNext(); + signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, _1)); EXPECT_EQ(2, handler_calls_); EXPECT_EQ(SIGINT, signum_); // There should be no more waiting handlers. @@ -94,28 +113,58 @@ TEST_F(SignalSetTest, twoSignals) { EXPECT_NO_THROW(signal_set_->remove(SIGINT)); } +/// Check that the signal set can only handle signals owned by it. +TEST_F(SignalSetTest, twoSignalSets) { + // Register handler for SIGHUP in the first signal set. + signal_set_.reset(new SignalSet(SIGHUP)); + // Register handler for SIGINT in the second signal set. + secondary_signal_set_.reset(new SignalSet(SIGINT)); + // Send SIGHUP. + ASSERT_EQ(0, raise(SIGHUP)); + // Send SIGINT. + ASSERT_EQ(0, raise(SIGINT)); + // Although the SIGHUP is the first signal received by the process + // it is not owned by the secondary signal set. The first signal + // to be handled by the secondary signal set is SIGINT. + EXPECT_EQ(SIGINT, secondary_signal_set_->getNext()); + // The signal set owns SIGHUP so it should be the next to handle. + EXPECT_EQ(SIGHUP, signal_set_->getNext()); + // Handle next signal owned by the secondary signal set. + secondary_signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, + _1)); + EXPECT_EQ(1, handler_calls_); + EXPECT_EQ(SIGINT, signum_); + // No more signals to be handled for this signal set. + EXPECT_EQ(-1, secondary_signal_set_->getNext()); + // Handle next signal owned by the signal set. + signal_set_->handleNext(boost::bind(&SignalSetTest::testHandler, _1)); + EXPECT_EQ(2, handler_calls_); + EXPECT_EQ(SIGHUP, signum_); + // No more signals to be handled by this signal set. + EXPECT_EQ(-1, signal_set_->getNext()); +} + // Check that each signal set removes only the signals that it has been used // to register. -TEST_F(SignalSetTest, twoSignalSets) { +TEST_F(SignalSetTest, remove) { // Register handlers for SIGHUP using one signal set. ASSERT_NO_THROW(signal_set_.reset(new SignalSet(SIGHUP))); // Define another signal set and register a different signal. - SignalSetPtr other_set; - ASSERT_NO_THROW(other_set.reset(new SignalSet(SIGINT))); + ASSERT_NO_THROW(secondary_signal_set_.reset(new SignalSet(SIGINT))); // The SIGHUP has been already registsred with the other signal // set, so it should not be possible to register it again. - ASSERT_THROW(other_set->add(SIGHUP), SignalSetError); + ASSERT_THROW(secondary_signal_set_->add(SIGHUP), SignalSetError); // It shouldn't be possible to remove the signal registered in a different // signal set. - ASSERT_THROW(other_set->remove(SIGHUP), SignalSetError); + ASSERT_THROW(secondary_signal_set_->remove(SIGHUP), SignalSetError); // Remove all signals from the first signal set. The signals registered // with the other signal signal set should be preserved. ASSERT_NO_THROW(signal_set_->clear()); // Check indirectly that the SIGINT is still registered. An attempt to // register registered signal should result in failure. - EXPECT_THROW(other_set->add(SIGINT), SignalSetError); + EXPECT_THROW(secondary_signal_set_->add(SIGINT), SignalSetError); // But, we should be able to regsiter SIGHUP. - EXPECT_NO_THROW(other_set->add(SIGHUP)); + EXPECT_NO_THROW(secondary_signal_set_->add(SIGHUP)); } /// Check that it is not possible to duplicate signals.