[3970] Use thread to run handlers in TimerMgr.

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 <asio.hpp>
 #include <config.h>
 #include <dhcpsrv/timer_mgr.h>
 #include <boost/bind.hpp>
@@ -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<void>(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

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 <asio.hpp>
 #include <dhcpsrv/timer_mgr.h>
 #include <exceptions/exceptions.h>
+#include <boost/bind.hpp>
+#include <iostream>
 
 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<std::string, TimerInfo>(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

src/lib/dhcpsrv/timer_mgr.h

@@ -17,8 +17,10 @@
 
 #include <asiolink/interval_timer.h>
 #include <asiolink/io_service.h>
+#include <util/threads/thread.h>
 #include <util/watch_socket.h>
 #include <boost/noncopyable.hpp>
+#include <boost/scoped_ptr.hpp>
 #include <map>
 #include <string>
 
@@ -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<util::thread::Thread> thread_;
+
     /// @brief Structure holding information for a single timer.
     ///
     /// This structure holds the instance of the watch socket being used to