From: Marcin Siodelski Date: Tue, 25 Aug 2015 14:49:59 +0000 (+0200) Subject: [3970] Use thread to run handlers in TimerMgr. X-Git-Tag: fd4o6_base~5^2~12 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=ff141355b9d82c60f93a81b5429c1f99ad78ddc0;p=thirdparty%2Fkea.git [3970] Use thread to run handlers in TimerMgr. --- diff --git a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc index ae207f2573..d89575f3d2 100644 --- a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc @@ -12,6 +12,7 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include #include #include #include @@ -32,6 +33,9 @@ private: /// @brief Cleans up after the test. virtual void TearDown(); + /// @brief IO service used by the test fixture class. + IOService io_service_; + public: /// @brief Wrapper method for registering a new timer. @@ -47,37 +51,10 @@ public: void registerTimer(const std::string& timer_name, const long timer_interval, const IntervalTimer::Mode& timer_mode = IntervalTimer::ONE_SHOT); - /// @brief Waits for one ready handler to be executed. - /// - /// @param timeout Wait timeout. - /// @return false if the timeout has occurred, true otherwise. The returned - /// value of true indicates that the test was successful, i.e. the timer - /// ready handler had been executed before the timeout occurred. - bool waitForOne(const long timeout); - - /// @brief Waits for a specified amount of time to execute ready handlers. - /// - /// This method waits for exactly @c timeout amount of time for all ready - /// handlers to be executed. A caller can determine whether the expected - /// handlers have been executed by checking the @c calls_count_ entries. - /// - /// @param timeout Wait timeout. - void waitForMany(const long timeout); - -private: - /// @brief Wait for one or many ready handlers. /// - /// This method is called internally by the public methods - /// @c waitForOne and @c waitForMany. - /// /// @param timeout Wait timeout. - /// @param wait_for_many A boolean flag indicating if the method should - /// wait for the specified amount of time to execute all handlers (if true) - /// or should wait for one ready handlers (if false). - /// - /// @return false if the timeout has occurred, true otherwise. - bool doWait(const long timeout, const bool wait_for_many); + void doWait(const long timeout); /// @brief Generic callback for timers under test. /// @@ -88,20 +65,7 @@ private: void timerCallback(const std::string& timer_name); /// @brief Callback for timeout. - /// - /// This callback is installed when the @c waitForOne or @c waitForMany - /// is executed to stop waiting after a given amount of time. It stops - /// the io service in the @c TimerMgr. - void timeoutCallback(asiolink::IOService& io_service); - - /// @brief Internal flag indicating if test timeout occurred. - /// - /// This flag is set by the @c timeoutCallback function when the timeout - /// has occurred. The @c waitWithTimeout returns 'false' if this value - /// is 'true', and 'true' if this value is 'false'. - bool timeout_occurred_; - -public: + void timeoutCallback(); /// @brief Holds the calls count for test timers. /// @@ -114,11 +78,12 @@ public: void TimerMgrTest::SetUp() { calls_count_.clear(); - timeout_occurred_ = false; +// TimerMgr::instance().startThread(); } void TimerMgrTest::TearDown() { +// TimerMgr::instance().stopThread(); } void @@ -136,65 +101,41 @@ TimerMgrTest::registerTimer(const std::string& timer_name, const long timer_inte } -bool -TimerMgrTest::waitForOne(const long timeout) { - return (doWait(timeout, false)); -} - void -TimerMgrTest::waitForMany(const long timeout) { - static_cast(doWait(timeout, true)); -} - -bool -TimerMgrTest::doWait(const long timeout, const bool wait_for_many) { - IOService& io_service = TimerMgr::instance().getIOService(); - IntervalTimer timeout_timer(io_service); - timeout_timer.setup(boost::bind(&TimerMgrTest::timeoutCallback, this, - boost::ref(io_service)), timeout, +TimerMgrTest::doWait(const long timeout) { + IntervalTimer timeout_timer(io_service_); + timeout_timer.setup(boost::bind(&TimerMgrTest::timeoutCallback, this), timeout, IntervalTimer::ONE_SHOT); - if (wait_for_many) { - io_service.run(); - - } else { - io_service.run_one(); - } - - if (timeout_occurred_) { - // Reset the flag so as it is set to false for another test. - timeout_occurred_ = false; - return (false); - } - - // No timeout, some ready handlers have been executed. - return (true); + io_service_.run_one(); } void 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); } void -TimerMgrTest::timeoutCallback(asiolink::IOService& io_service) { - // Timeout has occurred. Stop the io service to stop waiting for - // ready handlers. - io_service.stop(); - // Indicate that we hit the timeout. - timeout_occurred_ = true; +TimerMgrTest::timeoutCallback() { + io_service_.stop(); } TEST_F(TimerMgrTest, registerTimer) { TimerMgr& timer_mgr = TimerMgr::instance(); ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1)); + + TimerMgr::instance().startThread(); TimerMgr::instance().setup("timer1"); - EXPECT_TRUE(waitForOne(5)); + doWait(500); + + TimerMgr::instance().stopThread(); - EXPECT_EQ(1, calls_count_["timer1"]); + EXPECT_GT(calls_count_["timer1"], 1); } } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/timer_mgr.cc b/src/lib/dhcpsrv/timer_mgr.cc index dfbf61d745..447c72d545 100644 --- a/src/lib/dhcpsrv/timer_mgr.cc +++ b/src/lib/dhcpsrv/timer_mgr.cc @@ -12,12 +12,16 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include #include #include +#include +#include using namespace isc; using namespace isc::asiolink; using namespace isc::util; +using namespace isc::util::thread; namespace isc { namespace dhcp { @@ -29,7 +33,7 @@ TimerMgr::instance() { } TimerMgr::TimerMgr() - : io_service_(new IOService()) { + : io_service_(new IOService()), thread_() { } void @@ -47,8 +51,14 @@ TimerMgr::registerTimer(const std::string& timer_name, << timer_name << "'"); } + if (thread_) { + isc_throw(InvalidOperation, "unable to register new timer when the" + " timer manager's worker thread is running"); + } + + IntervalTimerPtr interval_timer(new IntervalTimer(*io_service_)); + WatchSocket watch_socket; - IntervalTimerPtr interval_timer(new IntervalTimer(getIOService())); TimerInfo timer_info(watch_socket, interval_timer, callback, interval, scheduling_mode); registered_timers_.insert(std::pair(timer_name, timer_info)); @@ -72,10 +82,23 @@ TimerMgr::setup(const std::string& timer_name) { } void -TimerMgr::localCallback(const std::string& timer_name) const { +TimerMgr::startThread() { + thread_.reset(new Thread(boost::bind(&IOService::run, &getIOService()))); } +void +TimerMgr::stopThread() { + if (thread_) { + io_service_->post(boost::bind(&IOService::stop, &getIOService())); + thread_->wait(); + thread_.reset(); + io_service_->get_io_service().reset(); + } +} +void +TimerMgr::localCallback(const std::string& timer_name) const { +} } // 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 5f1eb02b98..7f3c0c572e 100644 --- a/src/lib/dhcpsrv/timer_mgr.h +++ b/src/lib/dhcpsrv/timer_mgr.h @@ -17,8 +17,10 @@ #include #include +#include #include #include +#include #include #include @@ -39,6 +41,10 @@ public: void setup(const std::string& timer_name); + void startThread(); + + void stopThread(); + asiolink::IOService& getIOService() const { return (*io_service_); } @@ -61,6 +67,8 @@ private: /// @brief Holds the pointer to the io service object. asiolink::IOServicePtr io_service_; + boost::scoped_ptr thread_; + /// @brief Structure holding information for a single timer. /// /// This structure holds the instance of the watch socket being used to