From: Marcin Siodelski Date: Wed, 2 Sep 2015 09:10:17 +0000 (+0200) Subject: [3970] Improved test coverage for the TimerMgr. X-Git-Tag: fd4o6_base~5^2~9 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=93752c1aa01db0003e7f327cff66c0fa73452f0e;p=thirdparty%2Fkea.git [3970] Improved test coverage for the TimerMgr. --- diff --git a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc index 1d9600e572..473fd71b17 100644 --- a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc @@ -19,6 +19,8 @@ #include #include #include +#include +#include using namespace isc; using namespace isc::dhcp; @@ -38,6 +40,8 @@ private: /// @brief IO service used by the test fixture class. IOService io_service_; + /// @brief Boolean flag which indicates that the timeout + /// for the @c doWait function has been reached. bool timeout_; public: @@ -53,11 +57,11 @@ public: /// @param mode Interval timer mode, which defaults to /// @c IntervalTimer::ONE_SHOT. void registerTimer(const std::string& timer_name, const long timer_interval, - const IntervalTimer::Mode& timer_mode = IntervalTimer::ONE_SHOT); + const IntervalTimer::Mode& timer_mode = IntervalTimer::REPEATING); /// @brief Wait for one or many ready handlers. /// - /// @param timeout Wait timeout. + /// @param timeout Wait timeout in milliseconds. void doWait(const long timeout); /// @brief Generic callback for timers under test. @@ -68,16 +72,27 @@ public: /// be increased. void timerCallback(const std::string& timer_name); + /// @brief Create a generic callback function for the timer. + /// + /// This is just a wrapped to make it a bit more convenient + /// in the test. boost::function makeCallback(const std::string& timer_name); /// @brief Callback for timeout. + /// + /// This callback indicates the test timeout by setting the + /// @c timeout_ member. void timeoutCallback(); + /// @brief Type definition for a map holding calls counters for + /// timers. + typedef std::map CallsCount; + /// @brief Holds the calls count for test timers. /// /// The key of this map holds the timer names. The value holds the number /// of calls to the timer handlers. - std::map calls_count_; + CallsCount calls_count_; }; @@ -93,6 +108,7 @@ void TimerMgrTest::TearDown() { // Make sure there are no dangling threads. TimerMgr::instance().stopThread(); + // Remove all timers. TimerMgr::instance().deregisterTimers(); } @@ -101,6 +117,8 @@ TimerMgrTest::registerTimer(const std::string& timer_name, const long timer_inte const IntervalTimer::Mode& timer_mode) { TimerMgr& timer_mgr = TimerMgr::instance(); + // Register the timer with the generic callback that counts the + // number of callback invocations. ASSERT_NO_THROW( timer_mgr.registerTimer(timer_name, makeCallback(timer_name), timer_interval, timer_mode) @@ -116,8 +134,13 @@ TimerMgrTest::doWait(const long timeout) { timeout_timer.setup(boost::bind(&TimerMgrTest::timeoutCallback, this), timeout, IntervalTimer::ONE_SHOT); + // The timeout flag will be set by the timeoutCallback if the test + // lasts for too long. In this case we will return from here. while (!timeout_) { - IfaceMgr::instance().receive6(0, timeout * 1000); + // Block for one 1 millisecond. + IfaceMgr::instance().receive6(0, 1000); + // Run ready handlers from the local IO service to execute + // the timeout callback if necessary. io_service_.get_io_service().poll_one(); } @@ -129,7 +152,9 @@ TimerMgrTest::timerCallback(const std::string& timer_name) { // Accumulate the number of calls to the timer handler. ++calls_count_[timer_name]; - TimerMgr::instance().setup(timer_name); +/* // The timer installed is the ONE_SHOT timer, so we have + // to reschedule the timer. + TimerMgr::instance().setup(timer_name); */ } boost::function @@ -140,11 +165,16 @@ TimerMgrTest::makeCallback(const std::string& timer_name) { void TimerMgrTest::timeoutCallback() { + // Timeout occurred. Stop and reset IO service and mark + // the timeout flag. io_service_.stop(); io_service_.get_io_service().reset(); timeout_ = true; } +// This test checks that certain errors are returned when invalid +// parameters are specified when registering a timer, or when +// the registration can't be made. TEST_F(TimerMgrTest, registerTimer) { TimerMgr& timer_mgr = TimerMgr::instance(); @@ -164,37 +194,204 @@ TEST_F(TimerMgrTest, registerTimer) { // Start worker thread. ASSERT_NO_THROW(timer_mgr.startThread()); + + // Can't register the timer when the thread is running. + ASSERT_THROW(timer_mgr.registerTimer("timer1", makeCallback("timer1"), 1, + IntervalTimer::ONE_SHOT), + InvalidOperation); + + // Stop the thread and retry. + ASSERT_NO_THROW(timer_mgr.stopThread()); + EXPECT_NO_THROW(timer_mgr.registerTimer("timer1", makeCallback("timer1"), 1, + IntervalTimer::ONE_SHOT)); + +} + +// This test verifies that it is possible to deregister a timer from +// the TimerMgr. +TEST_F(TimerMgrTest, deregisterTimer) { + TimerMgr& timer_mgr = TimerMgr::instance(); + + // Register a timer and start it. + ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1)); + ASSERT_NO_THROW(timer_mgr.setup("timer1")); + ASSERT_NO_THROW(timer_mgr.startThread()); + + // Wait for the timer to execute several times. + doWait(100); + + // Stop the thread. + ASSERT_NO_THROW(timer_mgr.stopThread()); + + // Remember how many times the timer's callback was executed. + const unsigned int calls_count = calls_count_["timer1"]; + ASSERT_GT(calls_count, 0); + + // Check that an attempt to deregister a non-existing timerm would + // result in execption. + EXPECT_THROW(timer_mgr.deregisterTimer("timer2"), BadValue); + + // Now deregister the correct one. + ASSERT_NO_THROW(timer_mgr.deregisterTimer("timer1")); + + // Start the thread again and wait another 100ms. + ASSERT_NO_THROW(timer_mgr.startThread()); + doWait(100); + + // The number of calls for the timer1 shouldn't change as the + // timer had been deregistered. + EXPECT_EQ(calls_count_["timer1"], calls_count); +} + +// This test verifies taht it is possible to deregister all timers. +TEST_F(TimerMgrTest, deregisterTimers) { + TimerMgr& timer_mgr = TimerMgr::instance(); + + // Register 10 timers. + for (int i = 1; i <= 20; ++i) { + std::ostringstream s; + s << "timer" << i; + ASSERT_NO_FATAL_FAILURE(registerTimer(s.str(), 1)) + << "fatal failure occurred while registering " + << s.str(); + ASSERT_NO_THROW(timer_mgr.setup(s.str())) + << "exception thrown while calling setup() for the " + << s.str(); + } + + // Start worker thread and wait for 500ms. + ASSERT_NO_THROW(timer_mgr.startThread()); + doWait(500); + ASSERT_NO_THROW(timer_mgr.stopThread()); + + // Make sure that all timers have been executed at least once. + for (CallsCount::iterator it = calls_count_.begin(); + it != calls_count_.end(); ++it) { + unsigned int calls_count = it->second; + ASSERT_GT(calls_count, 0) + << "expected calls counter for timer" + << (std::distance(calls_count_.begin(), it) + 1) + << " greater than 0"; + } + + // Copy counters for all timers. + CallsCount calls_count(calls_count_); + + // Let's deregister all timers. + ASSERT_NO_THROW(timer_mgr.deregisterTimers()); + + // Start worker thread again and wait for 500ms. + ASSERT_NO_THROW(timer_mgr.startThread()); + doWait(500); + ASSERT_NO_THROW(timer_mgr.stopThread()); + + // The calls counter shouldn't change because there are + // no timers registered. + EXPECT_TRUE(calls_count == calls_count_); +} + +// This test verifies that the timer execution can be cancelled. +TEST_F(TimerMgrTest, cancel) { + TimerMgr& timer_mgr = TimerMgr::instance(); + + // Register timer. + ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1)); + + // We can start the worker thread before we even kick in the timers. + ASSERT_NO_THROW(timer_mgr.startThread()); + + // Kick in the timer and wait for 500ms. + ASSERT_NO_THROW(timer_mgr.setup("timer1")); + doWait(500); + + // Cancelling non-existing timer should fail. + EXPECT_THROW(timer_mgr.cancel("timer2"), BadValue); + + // Cancelling the good one should pass, even when the worker + // thread is running. + ASSERT_NO_THROW(timer_mgr.cancel("timer1")); + + // Remember how many calls have been invoked and wait for + // another 500ms. + unsigned int calls_count = calls_count_["timer1"]; + doWait(500); + + // The number of calls shouldn't change because the timer had been + // cancelled. + ASSERT_EQ(calls_count, calls_count_["timer1"]); + + // Setup the timer again. + ASSERT_NO_THROW(timer_mgr.setup("timer1")); + doWait(500); + + // New calls should be recorded. + EXPECT_GT(calls_count_["timer1"], calls_count); } +// This test verifies that the callbacks for the scheduled timers are +// actually called. TEST_F(TimerMgrTest, scheduleTimers) { TimerMgr& timer_mgr = TimerMgr::instance(); + // Register two timers: 'timer1' and 'timer2'. The first timer will + // be executed at the 1ms interval. The second one at the 5ms + // interval. ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1)); ASSERT_NO_FATAL_FAILURE(registerTimer("timer2", 5)); - timer_mgr.startThread(); - timer_mgr.setup("timer1"); - timer_mgr.setup("timer2"); + // We can start the worker thread before we even kick in the timers. + ASSERT_NO_THROW(timer_mgr.startThread()); + // Kick in the timers. The timers have been registered so there + // should be no exception. + ASSERT_NO_THROW(timer_mgr.setup("timer1")); + ASSERT_NO_THROW(timer_mgr.setup("timer2")); + + // Run IfaceMgr::receive6() in the loop for 500ms. This function + // will read data from the watch sockets created when the timers + // were registered. The data is delivered to the watch sockets + // at the interval of the timers, which should break the blocking + // call to receive6(). As a result, the callbacks associated + // with the watch sockets should be called. doWait(500); + // Stop the worker thread, which would halt the execution of + // the timers. timer_mgr.stopThread(); - EXPECT_GT(calls_count_["timer1"], 1); + // We have been running the timer for 500ms at the interval of + // 1 ms. The maximum number of callbacks is 500. However, the + // callback itself takes time. Stoping the thread takes time. + // So, the real number differs significantly. We don't know + // exactly how many have been executed. It should be more + // than 10 for sure. But we really made up the numbers here. + EXPECT_GT(calls_count_["timer1"], 25); + // For the second timer it should be more than 5. EXPECT_GT(calls_count_["timer2"], 5); + // Because the interval of the 'timer1' is lower than the + // interval of the 'timer2' the number of calls should + // be higher for the 'timer1'. EXPECT_GT(calls_count_["timer1"], calls_count_["timer2"]); - ASSERT_NO_THROW(timer_mgr.deregisterTimer("timer1")); - + // Remember the number of calls from 'timer1' and 'timer2'. unsigned int calls_count_timer1 = calls_count_["timer1"]; unsigned int calls_count_timer2 = calls_count_["timer2"]; - timer_mgr.startThread(); + // Deregister the 'timer1'. + ASSERT_NO_THROW(timer_mgr.deregisterTimer("timer1")); + + // Restart the thread. + ASSERT_NO_THROW(timer_mgr.startThread()); + // Wait another 500ms. The 'timer1' was deregistered so it + // should not make any more calls. The 'timer2' should still + // work as previously. doWait(500); + // The number of calls shouldn't have changed. EXPECT_EQ(calls_count_timer1, calls_count_["timer1"]); + // There should be some new calls registered for the 'timer2'. EXPECT_GT(calls_count_["timer2"], calls_count_timer2); } diff --git a/src/lib/dhcpsrv/timer_mgr.cc b/src/lib/dhcpsrv/timer_mgr.cc index e4db08203a..c54f614b8b 100644 --- a/src/lib/dhcpsrv/timer_mgr.cc +++ b/src/lib/dhcpsrv/timer_mgr.cc @@ -74,10 +74,13 @@ TimerMgr::registerTimer(const std::string& timer_name, // Create a structure holding the configuration for the timer. It will // create the instance if the IntervalTimer and WatchSocket. It will // also hold the callback, interval and scheduling mode parameters. + // This may throw a WatchSocketError if the socket creation fails. TimerInfo timer_info(getIOService(), callback, interval, scheduling_mode); // Register the WatchSocket in the IfaceMgr and register our own callback - // to be executed when the data is received over this socket. + // to be executed when the data is received over this socket. The only time + // this may fail is when the socket failed to open which would have caused + // an exception in the previous call. So we should be safe here. IfaceMgr::instance().addExternalSocket(timer_info.watch_socket_->getSelectFd(), boost::bind(&TimerMgr::ifaceMgrCallback, this, timer_name)); @@ -97,10 +100,10 @@ TimerMgr::deregisterTimer(const std::string& timer_name) { << timer_name << "'"); } - TimerInfo& timer_info = timer_info_it->second; - // Cancel any pending asynchronous operation and stop the timer. - timer_info.interval_timer_->cancel(); + cancel(timer_name); + + TimerInfo& timer_info = timer_info_it->second; // Unregister the watch socket from the IfaceMgr. IfaceMgr::instance().deleteExternalSocket(timer_info.watch_socket_->getSelectFd()); @@ -156,6 +159,8 @@ TimerMgr::cancel(const std::string& timer_name) { } // Cancel the timer. timer_info_it->second.interval_timer_->cancel(); + // Clear watch socket, if ready. + timer_info_it->second.watch_socket_->clearReady(); } void @@ -173,9 +178,15 @@ void TimerMgr::stopThread() { // If thread is not running, this is no-op. if (thread_) { + std::cout << "stopThread" << std::endl; // Stop the IO Service. This will break the IOService::run executed in the // worker thread. The thread will now terminate. getIOService().post(boost::bind(&IOService::stop, &getIOService())); + // When the worker thread may be waiting on the call to + // WatchSocket::markReady until main thread clears the socket. + // To unblock the thread we have to clear all sockets to make + // sure that the thread doesn't remain blocked. + clearReadySockets(); // Wait for the thread to terminate. thread_->wait(); // Set the thread pointer to NULL to indicate that the thread is not running. @@ -223,9 +234,12 @@ TimerMgr::watchSocketCallback(const std::string& timer_name, const bool mark_rea // from the worker thrad and will likely block the thread until the socket // is cleared. if (mark_ready) { + std::cout << "markReady" << std::endl; timer_info.watch_socket_->markReady(); - + std::cout << "markReady ended" << std::endl; } else { + + std::cout << "clearReady" << std::endl; // We're executing a callback function from the Interface Manager. // This callback function is executed when the call to select() is // interrupted as a result of receiving some data over the watch @@ -239,5 +253,15 @@ TimerMgr::watchSocketCallback(const std::string& timer_name, const bool mark_rea } } +void +TimerMgr::clearReadySockets() { + for (TimerInfoMap::iterator timer_info_it = registered_timers_.begin(); + timer_info_it != registered_timers_.end(); ++timer_info_it) { + { + timer_info_it->second.watch_socket_->clearReady(); + } + } +} + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcpsrv/timer_mgr.h b/src/lib/dhcpsrv/timer_mgr.h index e1a97fe034..c2c7289b0d 100644 --- a/src/lib/dhcpsrv/timer_mgr.h +++ b/src/lib/dhcpsrv/timer_mgr.h @@ -237,6 +237,8 @@ private: //@} + void clearReadySockets(); + /// @brief Pointer to the worker thread. /// /// This is initially set to NULL until the thread is started using the @@ -295,6 +297,7 @@ private: /// the map. The timer is associated with an instance of the @c WatchSocket /// which is marked ready when the interval for the particular elapses. TimerInfoMap registered_timers_; + }; } // end of namespace isc::dhcp