Browse Source

[5317] TimerMgr's handlers are now ran in the main thread.

Marcin Siodelski 7 years ago
parent
commit
ca4049e6cc

+ 2 - 24
src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -494,16 +494,6 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
         return (isc::config::createAnswer(1, err.str()));
     }
 
-    // We're going to modify the timers configuration. This is not allowed
-    // when the thread is running.
-    try {
-        TimerMgr::instance()->stopThread();
-    } catch (const std::exception& ex) {
-        err << "Unable to stop worker thread running timers: "
-            << ex.what() << ".";
-        return (isc::config::createAnswer(1, err.str()));
-    }
-
     ConstElementPtr answer = configureDhcp4Server(*srv, config);
 
     // Check that configuration was successful. If not, do not reopen sockets
@@ -573,17 +563,6 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
         return (isc::config::createAnswer(1, err.str()));
     }
 
-    // Start worker thread if there are any timers installed.
-    if (TimerMgr::instance()->timersCount() > 0) {
-        try {
-            TimerMgr::instance()->startThread();
-        } catch (const std::exception& ex) {
-            err << "Unable to start worker thread running timers: "
-                << ex.what() << ".";
-            return (isc::config::createAnswer(1, err.str()));
-        }
-    }
-
     return (answer);
 }
 
@@ -614,6 +593,8 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
     }
     server_ = this; // remember this instance for later use in handlers
 
+    TimerMgr::instance()->setIOService(getIOService());
+
     // These are the commands always supported by the DHCPv4 server.
     // Please keep the list in alphabetic order.
     CommandMgr::instance().registerCommand("build-report",
@@ -676,9 +657,6 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
     try {
         cleanup();
 
-        // Stop worker thread running timers, if it is running. Then
-        // unregister any timers.
-        timer_mgr_->stopThread();
         timer_mgr_->unregisterTimers();
 
         // Close the command socket (if it exists).

+ 6 - 3
src/bin/dhcp4/dhcp4_srv.cc

@@ -5,7 +5,6 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 #include <config.h>
-#include <asiolink/io_address.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/duid.h>
 #include <dhcp/hwaddr.h>
@@ -418,7 +417,7 @@ const std::string Dhcpv4Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 
 Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const bool use_bcast,
                      const bool direct_response_desired)
-    : shutdown_(true), alloc_engine_(), port_(port),
+    : io_service_(new IOService()), shutdown_(true), alloc_engine_(), port_(port),
       use_bcast_(use_bcast) {
 
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_OPEN_SOCKET).arg(port);
@@ -717,6 +716,7 @@ Dhcpv4Srv::run() {
     while (!shutdown_) {
         try {
             run_one();
+            getIOService()->poll();
         } catch (const std::exception& e) {
             // General catch-all exception that are not caught by more specific
             // catches. This one is for exceptions derived from std::exception.
@@ -740,7 +740,10 @@ Dhcpv4Srv::run_one() {
     Pkt4Ptr rsp;
 
     try {
-        uint32_t timeout = 1000;
+        // Set select() timeout to 1s. This value should not be modified
+        // because it is important that the select() returns control
+        // frequently so as the IOService can be polled for ready handlers.
+        uint32_t timeout = 1;
         LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL, DHCP4_BUFFER_WAIT).arg(timeout);
         query = receivePacket(timeout);
 

+ 11 - 1
src/bin/dhcp4/dhcp4_srv.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2011-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2011-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -7,6 +7,7 @@
 #ifndef DHCPV4_SRV_H
 #define DHCPV4_SRV_H
 
+#include <asiolink/io_service.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/pkt4.h>
 #include <dhcp/option.h>
@@ -188,6 +189,10 @@ typedef boost::shared_ptr<Dhcpv4Exchange> Dhcpv4ExchangePtr;
 /// See the derived \ref ControlledDhcpv4Srv class for support for
 /// command and configuration updates over msgq.
 class Dhcpv4Srv : public Daemon {
+private:
+
+    /// @brief Pointer to IO service used by the server.
+    asiolink::IOServicePtr io_service_;
 
 public:
 
@@ -222,6 +227,11 @@ public:
     /// @brief Destructor. Used during DHCPv4 service shutdown.
     virtual ~Dhcpv4Srv();
 
+    /// @brief Returns pointer to the IO service used by the server.
+    asiolink::IOServicePtr& getIOService() {
+        return (io_service_);
+    }
+
     /// @brief returns Kea version on stdout and exit.
     /// redeclaration/redefinition. @ref Daemon::getVersion()
     static std::string getVersion(bool extended);

+ 11 - 11
src/bin/dhcp4/tests/kea_controller_unittest.cc

@@ -6,7 +6,9 @@
 
 #include <config.h>
 
+#include <asiolink/interval_timer.h>
 #include <asiolink/io_address.h>
+#include <asiolink/io_service.h>
 #include <cc/command_interpreter.h>
 #include <dhcp/dhcp4.h>
 #include <dhcp/hwaddr.h>
@@ -85,17 +87,15 @@ public:
 
     /// @brief Runs timers for specified time.
     ///
-    /// Internally, this method calls @c IfaceMgr::receive4 to run the
-    /// callbacks for the installed timers.
-    ///
+    /// @param io_service Pointer to the IO service to be ran.
     /// @param timeout_ms Amount of time after which the method returns.
-    void runTimersWithTimeout(const long timeout_ms) {
-        isc::util::Stopwatch stopwatch;
-        while (stopwatch.getTotalMilliseconds() < timeout_ms) {
-            // Block for up to one millisecond waiting for the timers'
-            // callbacks to be executed.
-            IfaceMgr::instancePtr()->receive4(0, 1000);
-        }
+    void runTimersWithTimeout(const IOServicePtr& io_service, const long timeout_ms) {
+        IntervalTimer timer(*io_service);
+        timer.setup([this, &io_service]() {
+            io_service->stop();
+        }, timeout_ms, IntervalTimer::ONE_SHOT);
+        io_service->run();
+        io_service->get_io_service().reset();
     }
 
     /// Name of a config file used during tests
@@ -571,7 +571,7 @@ TEST_F(JSONFileBackendTest, timers) {
 
     // Poll the timers for a while to make sure that each of them is executed
     // at least once.
-    ASSERT_NO_THROW(runTimersWithTimeout(5000));
+    ASSERT_NO_THROW(runTimersWithTimeout(srv->getIOService(), 5000));
 
     // Verify that the leases in the database have been processed as expected.
 

+ 2 - 26
src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -495,17 +495,6 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
         return (no_srv);
     }
 
-    // We're going to modify the timers configuration. This is not allowed
-    // when the thread is running.
-    try {
-        TimerMgr::instance()->stopThread();
-    } catch (const std::exception& ex) {
-        std::ostringstream err;
-        err << "Unable to stop worker thread running timers: "
-            << ex.what() << ".";
-        return (isc::config::createAnswer(1, err.str()));
-    }
-
     ConstElementPtr answer = configureDhcp6Server(*srv, config);
 
     // Check that configuration was successful. If not, do not reopen sockets
@@ -594,18 +583,6 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
         return (isc::config::createAnswer(1, err.str()));
     }
 
-    // Start worker thread if there are any timers installed.
-    if (TimerMgr::instance()->timersCount() > 0) {
-        try {
-            TimerMgr::instance()->startThread();
-        } catch (const std::exception& ex) {
-            std::ostringstream err;
-            err << "Unable to start worker thread running timers: "
-                << ex.what() << ".";
-            return (isc::config::createAnswer(1, err.str()));
-        }
-    }
-
     // Finally, we can commit runtime option definitions in libdhcp++. This is
     // exception free.
     LibDHCP::commitRuntimeOptionDefs();
@@ -638,6 +615,8 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
     }
     server_ = this; // remember this instance for use in callback
 
+    TimerMgr::instance()->setIOService(getIOService());
+
     // These are the commands always supported by the DHCPv6 server.
     // Please keep the list in alphabetic order.
     CommandMgr::instance().registerCommand("build-report",
@@ -699,9 +678,6 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() {
     try {
         cleanup();
 
-        // Stop worker thread running timers, if it is running. Then
-        // unregister any timers.
-        timer_mgr_->stopThread();
         timer_mgr_->unregisterTimers();
 
         // Close the command socket (if it exists).

+ 7 - 2
src/bin/dhcp6/dhcp6_srv.cc

@@ -177,7 +177,8 @@ namespace dhcp {
 const std::string Dhcpv6Srv::VENDOR_CLASS_PREFIX("VENDOR_CLASS_");
 
 Dhcpv6Srv::Dhcpv6Srv(uint16_t port)
-    : port_(port), serverid_(), shutdown_(true), alloc_engine_()
+    : io_service_(new IOService()), port_(port), serverid_(), shutdown_(true),
+      alloc_engine_()
 {
 
     LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_OPEN_SOCKET).arg(port);
@@ -374,6 +375,7 @@ bool Dhcpv6Srv::run() {
     while (!shutdown_) {
         try {
             run_one();
+            getIOService()->poll();
         } catch (const std::exception& e) {
             // General catch-all standard exceptions that are not caught by more
             // specific catches.
@@ -395,7 +397,10 @@ void Dhcpv6Srv::run_one() {
     Pkt6Ptr rsp;
 
     try {
-        uint32_t timeout = 1000;
+        // Set select() timeout to 1s. This value should not be modified
+        // because it is important that the select() returns control
+        // frequently so as the IOService can be polled for ready handlers.
+        uint32_t timeout = 1;
         LOG_DEBUG(packet6_logger, DBG_DHCP6_DETAIL, DHCP6_BUFFER_WAIT).arg(timeout);
         query = receivePacket(timeout);
 

+ 10 - 0
src/bin/dhcp6/dhcp6_srv.h

@@ -7,6 +7,7 @@
 #ifndef DHCPV6_SRV_H
 #define DHCPV6_SRV_H
 
+#include <asiolink/io_service.h>
 #include <dhcp_ddns/ncr_msg.h>
 #include <dhcp/dhcp6.h>
 #include <dhcp/duid.h>
@@ -53,6 +54,10 @@ public:
 /// packets, processes them, manages leases assignment and generates
 /// appropriate responses.
 class Dhcpv6Srv : public Daemon {
+private:
+
+    /// @brief Pointer to IO service used by the server.
+    asiolink::IOServicePtr io_service_;
 
 public:
     /// @brief defines if certain option may, must or must not appear
@@ -78,6 +83,11 @@ public:
     /// @brief Destructor. Used during DHCPv6 service shutdown.
     virtual ~Dhcpv6Srv();
 
+    /// @brief Returns pointer to the IO service used by the server.
+    asiolink::IOServicePtr& getIOService() {
+        return (io_service_);
+    }
+
     /// @brief returns Kea version on stdout and exit.
     /// redeclaration/redefinition. @ref Daemon::getVersion()
     static std::string getVersion(bool extended);

+ 9 - 11
src/bin/dhcp6/tests/kea_controller_unittest.cc

@@ -74,17 +74,15 @@ public:
 
     /// @brief Runs timers for specified time.
     ///
-    /// Internally, this method calls @c IfaceMgr::receive6 to run the
-    /// callbacks for the installed timers.
-    ///
+    /// @param io_service Pointer to the IO service to be ran.
     /// @param timeout_ms Amount of time after which the method returns.
-    void runTimersWithTimeout(const long timeout_ms) {
-        isc::util::Stopwatch stopwatch;
-        while (stopwatch.getTotalMilliseconds() < timeout_ms) {
-            // Block for up to one millisecond waiting for the timers'
-            // callbacks to be executed.
-            IfaceMgr::instancePtr()->receive6(0, 1000);
-        }
+    void runTimersWithTimeout(const IOServicePtr& io_service, const long timeout_ms) {
+        IntervalTimer timer(*io_service);
+        timer.setup([this, &io_service]() {
+            io_service->stop();
+        }, timeout_ms, IntervalTimer::ONE_SHOT);
+        io_service->run();
+        io_service->get_io_service().reset();
     }
 
     static const char* TEST_FILE;
@@ -521,7 +519,7 @@ TEST_F(JSONFileBackendTest, timers) {
 
     // Poll the timers for a while to make sure that each of them is executed
     // at least once.
-    ASSERT_NO_THROW(runTimersWithTimeout(5000));
+    ASSERT_NO_THROW(runTimersWithTimeout(srv->getIOService(), 5000));
 
     // Verify that the leases in the database have been processed as expected.
 

+ 2 - 2
src/lib/asiolink/interval_timer.cc

@@ -76,8 +76,8 @@ IntervalTimerImpl::setup(const IntervalTimer::Callback& cbfunc,
                          const long interval,
                          const IntervalTimer::Mode& mode)
 {
-    // Interval should not be less than or equal to 0.
-    if (interval <= 0) {
+    // Interval should not be less than 0.
+    if (interval < 0) {
         isc_throw(isc::BadValue, "Interval should not be less than or "
                                  "equal to 0");
     }

+ 1 - 2
src/lib/asiolink/tests/interval_timer_unittest.cc

@@ -154,8 +154,7 @@ TEST_F(IntervalTimerTest, invalidArgumentToIntervalTimer) {
     // expect throw if call back function is empty
     EXPECT_THROW(itimer.setup(IntervalTimer::Callback(), 1),
                  isc::InvalidParameter);
-    // expect throw if interval is not greater than 0
-    EXPECT_THROW(itimer.setup(TimerCallBack(this), 0), isc::BadValue);
+    // expect throw if interval is negative.
     EXPECT_THROW(itimer.setup(TimerCallBack(this), -1), isc::BadValue);
 }
 

+ 0 - 33
src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -878,45 +878,12 @@ An example of such operation is a periodic cleanup of
 expired leases. The name of the timer is included in the
 message.
 
-% DHCPSRV_TIMERMGR_SOCKET_CLEAR_FAILED clearing watch socket for timer %1 failed: %2
-An error message indicating that the specified timer elapsed,
-the operation associated with the timer was executed but the
-server was unable to signal this to the worker thread responsible
-for dispatching timers. The thread will continue but it will
-not be able to dispatch any operations for this timer. The
-server reconfiguration or restart may solve the problem
-but the situation may repeat.
-
-% DHCPSRV_TIMERMGR_SOCKET_MARK_FAILED marking watch socket for timer %1 failed: %2
-An error message indicating that the specified timer elapsed,
-but the server was unable to flag that the handler function
-should be executed for this timer. The callback will not
-be executed this time and most likely the subsequent attempts
-will not be successful too. This error is highly unlikely.
-The name of the timer and the reason for failure is included
-in the message.
-
-% DHCPSRV_TIMERMGR_START_THREAD starting thread for timers
-A debug message issued when the Timer Manager is starting a
-worker thread to run started timers. The worker thread is
-typically started right after all timers have been registered
-and runs until timers need to be reconfigured, e.g. their
-interval is changed, new timers are registered or existing
-timers are unregistered.
-
 % DHCPSRV_TIMERMGR_START_TIMER starting timer: %1
 A debug message issued when the registered interval timer is
 being started. If this operation is successful the timer will
 periodically execute the operation associated with it. The
 name of the started timer is included in the message.
 
-% DHCPSRV_TIMERMGR_STOP_THREAD stopping thread for timers
-A debug message issued when the Timer Manager is stopping
-the worker thread which executes interval timers. When the
-thread is stopped no timers will be executed. The thread is
-typically stopped at the server reconfiguration or when the
-server shuts down.
-
 % DHCPSRV_TIMERMGR_STOP_TIMER stopping timer: %1
 A debug message issued when the registered interval timer is
 being stopped. The timer remains registered and can be restarted

+ 22 - 48
src/lib/dhcpsrv/libdhcpsrv.dox

@@ -272,54 +272,28 @@ the server.
 
 @section timerManager Timer Manager
 
-The fundamental role of the DHCP server is to receive and process DHCP
-messages received over the sockets opened on network interfaces. The
-servers' include the main loops in which the servers passively wait
-for the messages. This is done by calling the
-@c isc::dhcp::IfaceMgr::receive4 and/or @c isc::dhcp::IfaceMgr::receive6
-methods for DHCPv4 and DHCPv6 server respectively. Internally, these
-methods call @c select() on open sockets, which blocks for a
-specified amount of time.
-
-The implication of using the @c select() is that the server has no
-means to process any "events" while it is waiting for the @c select()
-to return. An example of such an event is the expiration of the timer
-which controls when the server should detect and process expired
-leases.
-
-The @c isc::dhcp::TimerMgr has been created to address the issue of
-processing expired leases according to the the dedicated timer.
-Nevertheless, this concept is universal and should be used for
-all timers which need to be triggered asynchronously, i.e. independently
-from processing the DHCP messages.
-
-The @c TimerMgr allows for registering timers and associating them with
-user callback functions, which are executed without waiting for the
-call to the @c select() function to return as a result of the timeout.
-When the particular timer elapses, the blocking call to select is
-interrupted by sending data over a dedicated (for a timer)
-@c isc::util::WatchSocket. Each timer has an instance of
-@c isc::util::WatchSocket associated with it, and each such socket
-is registered with the @c IfaceMgr using the @c IfaceMgr::addExternalSocket.
-When the transmission of the data over the watch socket interrupts the
-@c select() call, the user callback is executed by
-@c isc::dhcp::IfaceMgr and the watch socket is cleared to accept
-subsequent events for that particular timer.
-
-The timers are implemented using the @c isc::asiolink::IntervalTimer class.
-They are run in a dedicated thread which is owned (created and destroyed)
-by @c isc::dhcp::TimerMgr. This worker thread runs an instance
-of @c isc::asiolink::IOService object which is associated with all
-registered timers. The thread uses a common callback function which
-is executed when a timer elapses. This callback function receives
-a name of the elapsed timer as an argument and, based on that, selects the
-appropriate @c isc::util::WatchSocket to be marked as ready. In order to
-overcome the race conditions with the main thread, the worker thread blocks
-right after it marks the watch socket as ready, and waits for this
-socket to be cleared by the main thread. This is the indication
-that the timer specific callback function has been invoked and the
-worker thread may continue monitoring registered timers and signal
-their readiness when they elapse.
+The @c isc::dhcp::TimerMgr is a singleton class used throughout the
+server process to register and unregister timers triggering periodic
+tasks such as lease file cleanup, reclamation of expired leases etc.
+
+The Timer Manger is using ASIO deadline timers (wrapped in
+@c isc::asiolink::IntervalTimer class) to execute tasks according to
+the configured periods. Therefore, the server process must provide the
+Timer Manager with the pointer to the @c isc::asiolink::IOService which
+the server is using to run asynchronous tasks.
+
+Current implementation of the DHCP servers uses synchronous calls to
+@c select() function to check if any transmission has been received
+on any socket. This poses a problem with running asynchronous calls
+via @c IOService in the main server loop because the @c select()
+blocks for a specified amount of time while asynchronous calls
+are not triggered. In the future we should migrate from the synchronous
+@c select() calls into asynchonous calls using ASIO. Currently,
+we mitigate the problem by lowering the @c select() timeout to 1s,
+and polling @c IOService for "ready" timers (handlers) after
+@c select() returns. This may cause delays of "ready" handlers
+execution by around 1s. However, this is acceptable for the current
+applications of the periodic timers.
 
 @section leaseReclamationRoutine Leases Reclamation Routine
 

+ 0 - 6
src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -127,12 +127,6 @@ LFCSetup::LFCSetup(asiolink::IntervalTimer::Callback callback)
 
 LFCSetup::~LFCSetup() {
     try {
-        // If we're here it means that either the process is terminating
-        // or we're reconfiguring the server. In both cases the thread has
-        // probably been stopped already, but we make sure by calling
-        // stopThread explicitly here.
-        timer_mgr_->stopThread();
-
         // Remove the timer. This will throw an exception if the timer does not
         // exist.  There are several possible reasons for this:
         // a) It hasn't been registered (although if the LFC Setup instance

+ 16 - 13
src/lib/dhcpsrv/tests/cfg_expiration_unittest.cc

@@ -5,20 +5,20 @@
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 #include <config.h>
-#include <dhcp/iface_mgr.h>
+#include <asiolink/asio_wrapper.h>
+#include <asiolink/interval_timer.h>
 #include <dhcpsrv/cfg_expiration.h>
 #include <dhcpsrv/timer_mgr.h>
 #include <exceptions/exceptions.h>
 #include <testutils/test_to_element.h>
-#include <util/stopwatch.h>
 #include <boost/function.hpp>
 #include <boost/shared_ptr.hpp>
 #include <gtest/gtest.h>
 #include <stdint.h>
 
 using namespace isc;
+using namespace isc::asiolink;
 using namespace isc::dhcp;
-using namespace isc::util;
 
 namespace {
 
@@ -297,7 +297,8 @@ public:
     /// of the class members, it also stops the @c TimerMgr worker thread
     /// and removes any registered timers.
     CfgExpirationTimersTest()
-        : timer_mgr_(TimerMgr::instance()),
+        : io_service_(new IOService()),
+          timer_mgr_(TimerMgr::instance()),
           stub_(new LeaseReclamationStub()),
           cfg_(true) {
         cleanupTimerMgr();
@@ -313,8 +314,8 @@ public:
 
     /// @brief Stop @c TimerMgr worker thread and remove the timers.
     void cleanupTimerMgr() const {
-        timer_mgr_->stopThread();
         timer_mgr_->unregisterTimers();
+        timer_mgr_->setIOService(io_service_);
     }
 
     /// @brief Runs timers for specified time.
@@ -324,12 +325,13 @@ public:
     ///
     /// @param timeout_ms Amount of time after which the method returns.
     void runTimersWithTimeout(const long timeout_ms) {
-        Stopwatch stopwatch;
-        while (stopwatch.getTotalMilliseconds() < timeout_ms) {
-            // Block for up to one millisecond waiting for the timers'
-            // callbacks to be executed.
-            IfaceMgr::instancePtr()->receive6(0, 1000);
-        }
+        IntervalTimer timer(*io_service_);
+        timer.setup([this]() {
+                io_service_->stop();
+        }, timeout_ms, IntervalTimer::ONE_SHOT);
+
+        io_service_->run();
+        io_service_->get_io_service().reset();
     }
 
     /// @brief Setup timers according to the configuration and run them
@@ -342,12 +344,13 @@ public:
                          stub_.get());
         // Run timers.
         ASSERT_NO_THROW({
-            timer_mgr_->startThread();
             runTimersWithTimeout(timeout_ms);
-            timer_mgr_->stopThread();
         });
     }
 
+    /// @brief Pointer to the IO service used by the tests.
+    IOServicePtr io_service_;
+
     /// @brief Pointer to the @c TimerMgr.
     TimerMgrPtr timer_mgr_;
 

+ 14 - 21
src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <config.h>
 #include <asiolink/asio_wrapper.h>
+#include <asiolink/interval_timer.h>
 #include <asiolink/io_address.h>
 #include <dhcp/duid.h>
 #include <dhcp/iface_mgr.h>
@@ -102,8 +103,11 @@ public:
     MemfileLeaseMgrTest() :
         io4_(getLeaseFilePath("leasefile4_0.csv")),
         io6_(getLeaseFilePath("leasefile6_0.csv")),
+        io_service_(new IOService()),
         timer_mgr_(TimerMgr::instance()) {
 
+        timer_mgr_->setIOService(io_service_);
+
         std::ostringstream s;
         s << KEA_LFC_BUILD_DIR << "/kea-lfc";
         setenv("KEA_LFC_EXECUTABLE", s.str().c_str(), 1);
@@ -130,7 +134,6 @@ public:
     /// destroys lease manager backend.
     virtual ~MemfileLeaseMgrTest() {
         // Stop TimerMgr worker thread if it is running.
-        timer_mgr_->stopThread();
         // Make sure there are no timers registered.
         timer_mgr_->unregisterTimers();
         LeaseMgrFactory::destroy();
@@ -207,12 +210,13 @@ public:
     ///
     /// @param ms Duration in milliseconds.
     void setTestTime(const uint32_t ms) {
-        // Measure test time and exit if timeout hit.
-        Stopwatch stopwatch;
-        while (stopwatch.getTotalMilliseconds() < ms) {
-            // Block for one 1 millisecond.
-            IfaceMgr::instance().receive6(0, 1000);
-        }
+        IntervalTimer timer(*io_service_);
+        timer.setup([this]() {
+                io_service_->stop();
+        }, ms, IntervalTimer::ONE_SHOT);
+
+        io_service_->run();
+        io_service_->get_io_service().reset();
     }
 
     /// @brief Waits for the specified process to finish.
@@ -344,6 +348,9 @@ public:
     /// @brief Object providing access to v6 lease IO.
     LeaseFileIO io6_;
 
+    /// @brief Pointer to the IO service used by the tests.
+    IOServicePtr io_service_;
+
     /// @brief Pointer to the instance of the @c TimerMgr.
     TimerMgrPtr timer_mgr_;
 };
@@ -453,15 +460,9 @@ TEST_F(MemfileLeaseMgrTest, lfcTimer) {
     boost::scoped_ptr<LFCMemfileLeaseMgr>
         lease_mgr(new LFCMemfileLeaseMgr(pmap));
 
-    // Start worker thread to execute LFC periodically.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-
     // Run the test for at most 2.9 seconds.
     setTestTime(2900);
 
-    // Stop worker thread.
-    ASSERT_NO_THROW(timer_mgr_->stopThread());
-
     // Within 2.9 we should record two LFC executions.
     EXPECT_EQ(2, lease_mgr->getLFCCount());
 }
@@ -480,17 +481,9 @@ TEST_F(MemfileLeaseMgrTest, lfcTimerDisabled) {
     boost::scoped_ptr<LFCMemfileLeaseMgr>
         lease_mgr(new LFCMemfileLeaseMgr(pmap));
 
-    // Start worker thread to execute LFC periodically.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-
     // Run the test for at most 1.9 seconds.
     setTestTime(1900);
 
-    // Stop worker thread to make sure it is not running when lease
-    // manager is destroyed. The lease manager will be unable to
-    // unregster timer when the thread is active.
-    ASSERT_NO_THROW(timer_mgr_->stopThread());
-
     // There should be no LFC execution recorded.
     EXPECT_EQ(0, lease_mgr->getLFCCount());
 }

+ 14 - 116
src/lib/dhcpsrv/tests/timer_mgr_unittest.cc

@@ -6,10 +6,10 @@
 
 #include <config.h>
 #include <asiolink/asio_wrapper.h>
-#include <dhcp/iface_mgr.h>
+#include <asiolink/interval_timer.h>
+#include <asiolink/io_service.h>
 #include <dhcpsrv/timer_mgr.h>
 #include <exceptions/exceptions.h>
-#include <util/stopwatch.h>
 
 #include <boost/bind.hpp>
 #include <gtest/gtest.h>
@@ -88,6 +88,9 @@ public:
     /// timers.
     typedef std::map<std::string, unsigned int> CallsCount;
 
+    /// @brief Pointer to IO service used by the tests.
+    IOServicePtr io_service_;
+
     /// @brief Holds the calls count for test timers.
     ///
     /// The key of this map holds the timer names. The value holds the number
@@ -100,16 +103,14 @@ public:
 
 void
 TimerMgrTest::SetUp() {
+    io_service_.reset(new IOService());
     timer_mgr_ = TimerMgr::instance();
+    timer_mgr_->setIOService(io_service_);
     calls_count_.clear();
-    // Make sure there are no dangling threads.
-    timer_mgr_->stopThread();
 }
 
 void
 TimerMgrTest::TearDown() {
-    // Make sure there are no dangling threads.
-    timer_mgr_->stopThread();
     // Remove all timers.
     timer_mgr_->unregisterTimers();
 }
@@ -130,13 +131,12 @@ TimerMgrTest::registerTimer(const std::string& timer_name, const long timer_inte
 
 void
 TimerMgrTest::doWait(const long timeout, const bool call_receive) {
-    util::Stopwatch stopwatch;
-    while (stopwatch.getTotalMilliseconds() < timeout) {
-        if (call_receive) {
-            // Block for one 1 millisecond.
-            IfaceMgr::instancePtr()->receive6(0, 1000);
-        }
-    }
+    IntervalTimer timer(*io_service_);
+    timer.setup([this]() {
+        io_service_->stop();
+    }, timeout, IntervalTimer::ONE_SHOT);
+    io_service_->run();
+    io_service_->get_io_service().reset();
 }
 
 void
@@ -181,20 +181,6 @@ TEST_F(TimerMgrTest, registerTimer) {
     ASSERT_THROW(timer_mgr_->registerTimer("timer2", makeCallback("timer2"), 1,
                                            IntervalTimer::ONE_SHOT),
                  BadValue);
-
-    // 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 unregister a timer from
@@ -204,14 +190,10 @@ TEST_F(TimerMgrTest, unregisterTimer) {
     ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1));
     ASSERT_EQ(1, timer_mgr_->timersCount());
     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 but execute pending callbacks.
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
-
     // Remember how many times the timer's callback was executed.
     const unsigned int calls_count = calls_count_["timer1"];
     ASSERT_GT(calls_count, 0);
@@ -226,10 +208,7 @@ TEST_F(TimerMgrTest, unregisterTimer) {
     ASSERT_NO_THROW(timer_mgr_->unregisterTimer("timer1"));
     ASSERT_EQ(0, timer_mgr_->timersCount());
 
-    // Start the thread again and wait another 100ms.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
     doWait(100);
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
 
     // The number of calls for the timer1 shouldn't change as the
     // timer had been unregistered.
@@ -252,10 +231,7 @@ TEST_F(TimerMgrTest, unregisterTimers) {
             << s.str();
     }
 
-    // Start worker thread and wait for 500ms.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
     doWait(500);
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
 
     // Make sure that all timers have been executed at least once.
     for (CallsCount::iterator it = calls_count_.begin();
@@ -276,32 +252,13 @@ TEST_F(TimerMgrTest, unregisterTimers) {
     // Make sure there are no timers registered.
     ASSERT_EQ(0, timer_mgr_->timersCount());
 
-    // Start worker thread again and wait for 500ms.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
     doWait(500);
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
 
     // The calls counter shouldn't change because there are
     // no timers registered.
     EXPECT_TRUE(calls_count == calls_count_);
 }
 
-// This test checks that it is not possible to unregister timers
-// while the thread is running.
-TEST_F(TimerMgrTest, unregisterTimerWhileRunning) {
-    // Register two timers.
-    ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1));
-    ASSERT_NO_FATAL_FAILURE(registerTimer("timer2", 1));
-
-    // Start the thread and make sure we can't unregister them.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-    EXPECT_THROW(timer_mgr_->unregisterTimer("timer1"), InvalidOperation);
-    EXPECT_THROW(timer_mgr_->unregisterTimers(), InvalidOperation);
-
-    // No need to stop the thread as it will be stopped by the
-    // test fixture destructor.
-}
-
 // This test verifies that the timer execution can be cancelled.
 TEST_F(TimerMgrTest, cancel) {
     // Register timer.
@@ -309,9 +266,8 @@ TEST_F(TimerMgrTest, cancel) {
 
     // Kick in the timer and wait for 500ms.
     ASSERT_NO_THROW(timer_mgr_->setup("timer1"));
-    ASSERT_NO_THROW(timer_mgr_->startThread());
+
     doWait(500);
-    ASSERT_NO_THROW(timer_mgr_->stopThread());
 
     // Cancelling non-existing timer should fail.
     EXPECT_THROW(timer_mgr_->cancel("timer2"), BadValue);
@@ -324,10 +280,7 @@ TEST_F(TimerMgrTest, cancel) {
     // another 500ms.
     unsigned int calls_count = calls_count_["timer1"];
 
-    ASSERT_NO_THROW(timer_mgr_->startThread());
     doWait(500);
-    // Stop thread before we setup again.
-    ASSERT_NO_THROW(timer_mgr_->stopThread());
 
     // The number of calls shouldn't change because the timer had been
     // cancelled.
@@ -337,7 +290,6 @@ TEST_F(TimerMgrTest, cancel) {
     ASSERT_NO_THROW(timer_mgr_->setup("timer1"));
 
     // Restart the thread.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
     doWait(500);
 
     // New calls should be recorded.
@@ -357,9 +309,6 @@ TEST_F(TimerMgrTest, scheduleTimers) {
     ASSERT_NO_THROW(timer_mgr_->setup("timer1"));
     ASSERT_NO_THROW(timer_mgr_->setup("timer2"));
 
-    // We can start the worker thread before we even kick in the timers.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-
     // Run IfaceMgr::receive6() in the loop for 1000ms. This function
     // will read data from the watch sockets created when the timers
     // were registered. The data is delivered to the watch sockets
@@ -368,10 +317,6 @@ TEST_F(TimerMgrTest, scheduleTimers) {
     // with the watch sockets should be called.
     doWait(1000);
 
-    // Stop the worker thread, which would halt the execution of
-    // the timers.
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
-
     // We have been running the timer for 1000ms at the interval of
     // 50ms. The maximum number of callbacks is 20. However, the
     // callback itself takes time. Stopping the thread takes time.
@@ -394,9 +339,6 @@ TEST_F(TimerMgrTest, scheduleTimers) {
     // Unregister the 'timer1'.
     ASSERT_NO_THROW(timer_mgr_->unregisterTimer("timer1"));
 
-    // Restart the thread.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-
     // Wait another 500ms. The 'timer1' was unregistered so it
     // should not make any more calls. The 'timer2' should still
     // work as previously.
@@ -408,48 +350,6 @@ TEST_F(TimerMgrTest, scheduleTimers) {
     EXPECT_GT(calls_count_["timer2"], calls_count_timer2);
 }
 
-// This test verifies that it is possible to force that the pending
-// timer callbacks are executed when the worker thread is stopped.
-TEST_F(TimerMgrTest, stopThreadWithRunningHandlers) {
-    // Register 'timer1'.
-    ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1));
-
-    // Kick in the timer.
-    ASSERT_NO_THROW(timer_mgr_->setup("timer1"));
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-
-    // Run the thread for 100ms. This should run some timers. The 'false'
-    // value indicates that the IfaceMgr::receive6 is not called, so the
-    // watch socket is never cleared.
-    doWait(100, false);
-
-    // There should be no calls registered for the timer1.
-    EXPECT_EQ(0, calls_count_["timer1"]);
-
-    // Stop the worker thread without completing pending callbacks.
-    ASSERT_NO_THROW(timer_mgr_->stopThread(false));
-
-    // There should be still not be any calls registered.
-    EXPECT_EQ(0, calls_count_["timer1"]);
-
-    // We can restart the worker thread before we even kick in the timers.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
-
-    // Run the thread for 100ms. This should run some timers. The 'false'
-    // value indicates that the IfaceMgr::receive6 is not called, so the
-    // watch socket is never cleared.
-    doWait(100, false);
-
-    // There should be no calls registered for the timer1.
-    EXPECT_EQ(0, calls_count_["timer1"]);
-
-    // Stop the worker thread with completing pending callbacks.
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
-
-    // There should be one call registered.
-    EXPECT_EQ(1, calls_count_["timer1"]);
-}
-
 // This test verifies that exceptions emitted from the callback would
 // be handled by the TimerMgr.
 TEST_F(TimerMgrTest, callbackWithException) {
@@ -464,9 +364,7 @@ TEST_F(TimerMgrTest, callbackWithException) {
 
     // Start thread. We hope that exception will be caught by the @c TimerMgr
     // and will not kill the process.
-    ASSERT_NO_THROW(timer_mgr_->startThread());
     doWait(500);
-    ASSERT_NO_THROW(timer_mgr_->stopThread(true));
 }
 
 } // end of anonymous namespace

+ 25 - 353
src/lib/dhcpsrv/timer_mgr.cc

@@ -7,13 +7,9 @@
 #include <config.h>
 #include <asiolink/asio_wrapper.h>
 #include <asiolink/io_service.h>
-#include <dhcp/iface_mgr.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/timer_mgr.h>
 #include <exceptions/exceptions.h>
-#include <util/threads/sync.h>
-#include <util/threads/thread.h>
-#include <util/watch_socket.h>
 
 #include <boost/bind.hpp>
 
@@ -21,57 +17,15 @@
 
 using namespace isc;
 using namespace isc::asiolink;
-using namespace isc::util;
-using namespace isc::util::thread;
 
 namespace {
 
-/// @brief Simple RAII object setting value to true while in scope.
-///
-/// This class is useful to temporarily set the value to true and
-/// automatically reset it to false when the object is destroyed
-/// as a result of return or exception.
-class ScopedTrue {
-public:
-
-    /// @brief Constructor.
-    ///
-    /// Sets the boolean value to true.
-    ///
-    /// @param value reference to the value to be set to true.
-    ScopedTrue(bool& value, Mutex& mutex)
-        : value_(value), mutex_(mutex) {
-        Mutex::Locker lock(mutex_);
-        value_ = true;
-    }
-
-    /// @brief Destructor.
-    ///
-    /// Sets the value to false.
-    ~ScopedTrue() {
-        Mutex::Locker lock(mutex_);
-        value_ = false;
-    }
-
-private:
-
-    /// @brief Reference to the controlled value.
-    bool& value_;
-
-    /// @brief Mutex to be used to lock while performing write
-    /// operations.
-    Mutex& mutex_;
-};
-
 /// @brief Structure holding information for a single timer.
 ///
 /// This structure holds the instance of the watch socket being used to
 /// signal that the timer is "ready". It also holds the instance of the
 /// interval timer and other parameters pertaining to it.
 struct TimerInfo {
-    /// @brief Instance of the watch socket.
-    util::WatchSocket watch_socket_;
-
     /// @brief Instance of the interval timer.
     asiolink::IntervalTimer interval_timer_;
 
@@ -97,8 +51,7 @@ struct TimerInfo {
               const asiolink::IntervalTimer::Callback& user_callback,
               const long interval,
               const asiolink::IntervalTimer::Mode& mode)
-        : watch_socket_(),
-          interval_timer_(io_service),
+        : interval_timer_(io_service),
           user_callback_(user_callback),
           interval_(interval),
           scheduling_mode_(mode) { };
@@ -123,12 +76,12 @@ public:
     /// @brief Constructor.
     TimerMgrImpl();
 
-    /// @brief Returns a reference to IO service used by the @c TimerMgr.
-    asiolink::IOService& getIOService() const {
-        return (*io_service_);
-    }
+    /// @brief Sets IO service to be used by the Timer Manager.
+    ///
+    /// @param io_service Pointer to the new IO service.
+    void setIOService(const IOServicePtr& io_service);
 
-    /// @brief Registers new timers in the @c TimerMgr.
+    /// @brief Registers new timer in the @c TimerMgr.
     ///
     /// @param timer_name Unique name for the timer.
     /// @param callback Pointer to the callback function to be invoked
@@ -139,7 +92,6 @@ public:
     /// @c asiolink::IntervalTimer::Mode.
     ///
     /// @throw BadValue if the timer name is invalid or duplicate.
-    /// @throw InvalidOperation if the worker thread is running.
     void registerTimer(const std::string& timer_name,
                        const asiolink::IntervalTimer::Callback& callback,
                        const long interval,
@@ -148,9 +100,8 @@ public:
 
     /// @brief Unregisters specified timer.
     ///
-    /// This method cancels the timer if it is setup. It removes the external
-    /// socket from the @c IfaceMgr and closes it. It finally removes the
-    /// timer from the internal collection of timers.
+    /// This method cancels the timer if it is setup and removes the timer
+    /// from the internal collection of timers.
     ///
     /// @param timer_name Name of the timer to be unregistered.
     ///
@@ -187,127 +138,29 @@ public:
     /// @throw BadValue if the timer hasn't been registered.
     void cancel(const std::string& timer_name);
 
-    /// @brief Checks if the thread is running.
-    ///
-    /// @return true if the thread is running.
-    bool threadRunning() const;
-
-    /// @brief Starts thread.
-    void createThread();
-
-    /// @brief Stops thread gracefully.
-    ///
-    /// This methods unblocks worker thread if it is blocked waiting for
-    /// any handlers and stops it. Outstanding handlers are later executed
-    /// in the main thread and all watch sockets are cleared.
-    ///
-    /// @param run_pending_callbacks Indicates if the pending callbacks
-    /// should be executed (if true).
-    void stopThread(const bool run_pending_callbacks);
-
 private:
 
-    /// @name Internal callbacks.
-    //@{
-    ///
     /// @brief Callback function to be executed for each interval timer when
     /// its scheduled interval elapses.
     ///
-    /// This method marks the @c util::Watch socket associated with the
-    /// timer as ready (writes data to a pipe). This method will BLOCK until
-    /// @c TimerMgrImpl::ifaceMgrCallback is executed from the main thread by
-    /// the @c IfaceMgr.
-    ///
-    /// @param timer_name Unique timer name to be passed to the callback.
-    void timerCallback(const std::string& timer_name);
-
-    /// @brief Callback function installed on the @c IfaceMgr and associated
-    /// with the particular timer.
-    ///
-    /// This callback function is executed by the @c IfaceMgr when the data
-    /// over the specific @c util::WatchSocket is received. This method clears
-    /// the socket (reads the data from the pipe) and executes the callback
-    /// supplied when the timer was registered.
-    ///
     /// @param timer_name Unique timer name.
-    void ifaceMgrCallback(const std::string& timer_name);
-
-    //@}
-
-    /// @name Methods to handle ready sockets.
-    //@{
-    ///
-    /// @brief Clear ready sockets and optionally run callbacks.
-    ///
-    /// This method is called by the @c TimerMgr::stopThread method
-    /// to clear watch sockets which may be marked as ready. It will
-    /// also optionally run callbacks installed for the timers which
-    /// marked sockets as ready.
-    ///
-    /// @param run_pending_callbacks Indicates if the callbacks should
-    /// be executed for the sockets being cleared (if true).
-    void clearReadySockets(const bool run_pending_callbacks);
-
-    /// @brief Clears a socket and optionally runs a callback.
-    ///
-    /// This method clears the ready socket pointed to by the specified
-    /// iterator. If the @c run_callback is set, the callback will
-    /// also be executed.
-    ///
-    /// @param timer_info_iterator Iterator pointing to the timer
-    /// configuration structure.
-    /// @param run_callback Boolean value indicating if the callback
-    /// should be executed for the socket being cleared (if true).
-    ///
-    /// @tparam Iterator Iterator pointing to the timer configuration
-    /// structure.
-    template<typename Iterator>
-    void handleReadySocket(Iterator timer_info_iterator,
-                           const bool run_callback);
-
-    //@}
-
-    /// @brief Blocking wait for the socket to be cleared.
-    void waitForSocketClearing(WatchSocket& watch_socket);
-
-    /// @brief Signals that a watch socket has been cleared.
-    void signalSocketClearing();
-
-    /// @brief Pointer to the @c IfaceMgr.
-    IfaceMgrPtr iface_mgr_;
+    void timerCallback(const std::string& timer_name);
 
     /// @brief Pointer to the io service.
     asiolink::IOServicePtr io_service_;
 
-    /// @brief Pointer to the worker thread.
-    ///
-    /// This is initially set to NULL until the thread is started using the
-    /// @c TimerMgr::startThread. The @c TimerMgr::stopThread sets it back
-    /// to NULL.
-    boost::shared_ptr<util::thread::Thread> thread_;
-
-    /// @brief Mutex used to synchronize main thread and the worker thread.
-    util::thread::Mutex mutex_;
-
-    /// @brief Conditional variable used to synchronize main thread and
-    /// worker thread.
-    util::thread::CondVar cond_var_;
-
-    /// @brief Boolean value indicating if the thread is being stopped.
-    bool stopping_;
-
-    /// @brief Holds mapping of the timer name to the watch socket, timer
-    /// instance and other parameters pertaining to the timer.
-    ///
-    /// Each registered timer has a unique name which is used as a key to
-    /// the map. The timer is associated with an instance of the @c WatchSocket
-    /// which is marked ready when the interval for the particular elapses.
+    /// @brief Holds mapping of the timer name to timer instance and other
+    /// parameters pertaining to the timer.
     TimerInfoMap registered_timers_;
 };
 
 TimerMgrImpl::TimerMgrImpl() :
-    iface_mgr_(IfaceMgr::instancePtr()), io_service_(new IOService()), thread_(),
-    mutex_(), cond_var_(), stopping_(false), registered_timers_() {
+    io_service_(new IOService()), registered_timers_() {
+}
+
+void
+TimerMgrImpl::setIOService(const IOServicePtr& io_service) {
+    io_service_ = io_service;
 }
 
 void
@@ -327,29 +180,12 @@ TimerMgrImpl::registerTimer(const std::string& timer_name,
                   << timer_name << "'");
     }
 
-    // Must not register new timer when the worker thread is running. Note
-    // that worker thread is using IO service and trying to register a new
-    // timer while IO service is being used would result in hang.
-    if (thread_) {
-        isc_throw(InvalidOperation, "unable to register new timer when the"
-                  " timer manager's worker thread is running");
-    }
-
     // 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.
-    TimerInfoPtr timer_info(new TimerInfo(getIOService(), callback,
+    // create the instance if the IntervalTimer. It will also hold the
+    // callback, interval and scheduling mode parameters.
+    TimerInfoPtr timer_info(new TimerInfo(*io_service_, 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. 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.
-    iface_mgr_->addExternalSocket(timer_info->watch_socket_.getSelectFd(),
-                                  boost::bind(&TimerMgrImpl::ifaceMgrCallback,
-                                              this, timer_name));
-
     // Actually register the timer.
     registered_timers_.insert(std::pair<std::string, TimerInfoPtr>(timer_name,
                                                                    timer_info));
@@ -358,11 +194,6 @@ TimerMgrImpl::registerTimer(const std::string& timer_name,
 void
 TimerMgrImpl::unregisterTimer(const std::string& timer_name) {
 
-    if (thread_) {
-        isc_throw(InvalidOperation, "unable to unregister timer "
-                  << timer_name << " while worker thread is running");
-    }
-
     // Find the timer with specified name.
     TimerInfoMap::iterator timer_info_it = registered_timers_.find(timer_name);
 
@@ -377,9 +208,6 @@ TimerMgrImpl::unregisterTimer(const std::string& timer_name) {
 
     const TimerInfoPtr& timer_info = timer_info_it->second;
 
-    // Unregister the watch socket from the IfaceMgr.
-    iface_mgr_->deleteExternalSocket(timer_info->watch_socket_.getSelectFd());
-
     // Remove the timer.
     registered_timers_.erase(timer_info_it);
 }
@@ -439,45 +267,6 @@ TimerMgrImpl::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();
-}
-
-
-bool
-TimerMgrImpl::threadRunning() const {
-    return (static_cast<bool>(thread_));
-}
-
-void
-TimerMgrImpl::createThread() {
-    thread_.reset(new Thread(boost::bind(&IOService::run, &getIOService())));
-}
-
-void
-TimerMgrImpl::stopThread(const bool run_pending_callbacks) {
-    // Set the stopping flag to true while we're stopping. This will be
-    // automatically reset to false when the function exits or exception
-    // is thrown.
-    ScopedTrue scoped_true(stopping_, mutex_);
-
-    // Stop the IO Service. This will break the IOService::run executed in the
-    // worker thread. The thread will now terminate.
-    getIOService().stop();
-
-    // Some of the watch sockets may be already marked as ready and
-    // have some pending callbacks to be executed. If the caller
-    // wants us to run the callbacks we clear the sockets and run
-    // them. If pending callbacks shouldn't be executed, this will
-    // only clear the sockets (which should be substantially faster).
-    clearReadySockets(run_pending_callbacks);
-    // Wait for the thread to terminate.
-    thread_->wait();
-    // Set the thread pointer to NULL to indicate that the thread is not running.
-    thread_.reset();
-    // IO Service has to be reset so as we can call run() on it again if we
-    // desire (using the startThread()).
-    getIOService().get_io_service().reset();
 }
 
 void
@@ -485,74 +274,17 @@ TimerMgrImpl::timerCallback(const std::string& timer_name) {
     // Find the specified timer setup.
     TimerInfoMap::iterator timer_info_it = registered_timers_.find(timer_name);
     if (timer_info_it != registered_timers_.end()) {
-        // We will mark watch socket ready - write data to a socket to
-        // interrupt the execution of the select() function. This is
-        // executed from the worker thread.
-        const TimerInfoPtr& timer_info = timer_info_it->second;
-
-        // This function is called from the worker thread and we don't want
-        // the worker thread to get exceptions out of here. It is unlikely
-        // that markReady() would cause any problems but theoretically
-        // possible. Hence, we rather log an error and leave.
-        try {
-            timer_info->watch_socket_.markReady();
-
-        } catch (const std::exception& ex) {
-            LOG_ERROR(dhcpsrv_logger, DHCPSRV_TIMERMGR_SOCKET_MARK_FAILED)
-                .arg(timer_name)
-                .arg(ex.what());
 
-            // Do not wait for clearing the socket because we were unable
-            // to mark it ready anyway.
-            return;
-        }
-
-        // Blocking wait for the socket to be cleared on the other
-        // end. This may be interrupted both if the socket is cleared
-        // or if the stopThread() has been called on the TimerMgr.
-        waitForSocketClearing(timer_info->watch_socket_);
-    }
-}
-
-void
-TimerMgrImpl::ifaceMgrCallback(const std::string& timer_name) {
-    // Find the specified timer setup.
-    TimerInfoMap::iterator timer_info_it = registered_timers_.find(timer_name);
-    if (timer_info_it != registered_timers_.end()) {
-        // 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
-        // socket. We have to clear the socket which has been marked as
-        // ready. Then execute the callback function supplied by the
-        // TimerMgr user to perform custom actions on the expiration of
-        // the given timer.
-        handleReadySocket(timer_info_it, true);
-    }
-}
-
-void
-TimerMgrImpl::clearReadySockets(const bool run_pending_callbacks) {
-    for (TimerInfoMap::iterator timer_info_it = registered_timers_.begin();
-         timer_info_it != registered_timers_.end(); ++timer_info_it) {
-        handleReadySocket(timer_info_it, run_pending_callbacks);
-   }
-}
-
-template<typename Iterator>
-void
-TimerMgrImpl::handleReadySocket(Iterator timer_info_iterator,
-                            const bool run_callback) {
-    if (run_callback) {
         // Running user-defined operation for the timer. Logging it
         // on the slightly lower debug level as there may be many
         // such traces.
         LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
                   DHCPSRV_TIMERMGR_RUN_TIMER_OPERATION)
-            .arg(timer_info_iterator->first);
+            .arg(timer_info_it->first);
 
         std::string error_string;
         try {
-            timer_info_iterator->second->user_callback_();
+            timer_info_it->second->user_callback_();
 
         } catch (const std::exception& ex){
             error_string = ex.what();
@@ -564,46 +296,10 @@ TimerMgrImpl::handleReadySocket(Iterator timer_info_iterator,
         // Exception was thrown. Log an error.
         if (!error_string.empty()) {
             LOG_ERROR(dhcpsrv_logger, DHCPSRV_TIMERMGR_CALLBACK_FAILED)
-                .arg(timer_info_iterator->first)
+                .arg(timer_info_it->first)
                 .arg(error_string);
         }
     }
-
-    try {
-        // This shouldn't really fail, but if it does we want to log an
-        // error and make sure that the thread is not stuck waiting for
-        // the socket to clear.
-        timer_info_iterator->second->watch_socket_.clearReady();
-
-    } catch (const std::exception& ex) {
-        LOG_ERROR(dhcpsrv_logger, DHCPSRV_TIMERMGR_SOCKET_CLEAR_FAILED)
-            .arg(timer_info_iterator->first)
-            .arg(ex.what());
-    }
-
-    // Whether it succeeded or not, clear the socket to make sure that
-    // the worker thread is not stuck waiting for the main thread.
-    signalSocketClearing();
-}
-
-void
-TimerMgrImpl::waitForSocketClearing(WatchSocket& watch_socket) {
-    // Waiting for the specific socket to be cleared.
-    while (watch_socket.isReady()) {
-        Mutex::Locker lock(mutex_);
-        // If stopThread has been called there is no sense to further
-        // wait for the socket clearing. Leave from here to unblock the
-        // worker thread.
-        if (stopping_) {
-            break;
-        }
-        cond_var_.wait(mutex_);
-    }
-}
-
-void
-TimerMgrImpl::signalSocketClearing() {
-    cond_var_.signal();
 }
 
 const TimerMgrPtr&
@@ -617,7 +313,6 @@ TimerMgr::TimerMgr()
 }
 
 TimerMgr::~TimerMgr() {
-    stopThread();
     unregisterTimers();
     delete impl_;
 }
@@ -681,31 +376,8 @@ TimerMgr::cancel(const std::string& timer_name) {
 }
 
 void
-TimerMgr::startThread() {
-    // Do not start the thread if the thread is already there.
-    if (!impl_->threadRunning()) {
-        // Only log it if we really start the thread.
-        LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
-                  DHCPSRV_TIMERMGR_START_THREAD);
-
-        impl_->createThread();
-    }
-}
-
-void
-TimerMgr::stopThread(const bool run_pending_callbacks) {
-    // If thread is not running, this is no-op.
-    if (impl_->threadRunning()) {
-        // Only log it if we really have something to stop.
-        LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE,
-                  DHCPSRV_TIMERMGR_STOP_THREAD);
-
-        impl_->stopThread(run_pending_callbacks);
-    }
-}
-IOService&
-TimerMgr::getIOService() const {
-    return (impl_->getIOService());
+TimerMgr::setIOService(const IOServicePtr& io_service) {
+    impl_->setIOService(io_service);
 }
 
 

+ 14 - 126
src/lib/dhcpsrv/timer_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -24,13 +24,9 @@ class TimerMgr;
 /// @brief Type definition of the shared pointer to @c TimerMgr.
 typedef boost::shared_ptr<TimerMgr> TimerMgrPtr;
 
-/// @brief Manages a pool of asynchronous interval timers for DHCP server.
+/// @brief Manages a pool of asynchronous interval timers for Kea server.
 ///
-/// This class holds a pool of asynchronous interval timers which are
-/// capable of interrupting the blocking call to @c select() function in
-/// the main threads of the DHCP servers. The main thread can then
-/// timely execute the callback function associated with the particular
-/// timer.
+/// This class holds a pool of asynchronous interval timers.
 ///
 /// This class is useful for performing periodic actions at the specified
 /// intervals, e.g. act upon expired leases (leases reclamation) or
@@ -52,64 +48,9 @@ typedef boost::shared_ptr<TimerMgr> TimerMgrPtr;
 /// The registered timer's interval does not begin to elapse until the
 /// @c TimerMgr::setup method is called for it.
 ///
-/// The @c TimerMgr uses worker thread to run the timers. The thread is
-/// started and stopped using the @c TimerMgr::startThread and
-/// @c TimerMgr::stopThread respectively. The thread calls the blocking
-/// @c IOService::run. All the registered timers are associated with
-/// this instance of the @c IOService that the thread is running.
-/// When the timer elapses a generic callback function is executed
-/// @c TimerMgr::timerCallback with the parameter giving the name
-/// of the timer for which it has been executed.
-///
-/// Every registered timer is associated with an instance of the
-/// @c util::WatchSocket object. The socket is registered in the
-/// @c IfaceMgr as an "external" socket. When the generic callback
-/// function is invoked for the timer, it obtains the instance of the
-/// @c util::WatchSocket and marks it "ready". This call effectively
-/// writes the data to a socket (pipe) which interrupts the call
-/// to the @c select() function in the main thread. When the
-/// @c IfaceMgr (in the main thread) detects data transmitted over
-/// the external socket it will invoke a callback function
-/// associated with this socket. This is the
-/// @c TimerMgr::ifaceMgrCallback associated with the socket when the
-/// timer is registered. This callback function is executed in the
-/// main thread. It clears the socket, which unblocks the worker
-/// thread. It also invokes the user callback function specified
-/// for a given timer.
-///
-/// The @c TimerMgr::timerCallback function searches for the
-/// registered timer for which it has been called. This may cause
-/// race conditions between the worker thread and the main thread
-/// if the latter is modifying the collection of the registered
-/// timers. Therefore, the @c TimerMgr does not allow for
-/// registering or unregistering the timers when the worker thread
-/// is running. The worker thread must be stopped first.
-/// It is possible to call @c TimerMgr::setup and @c TimerMgr::cancel
-/// while the worker thread is running but this is considered
-/// unreliable (may cause race conditions) except the case when the
-/// @c TimerMgr::setup is called from the installed callback
-/// function to reschedule the ONE_SHOT timer. This is thread safe
-/// because the worker thread is blocked while the callback function
-/// is executed.
-///
-/// The worker thread is blocked when it executes a generic callback
-/// function in the @c TimerMgr, which marks the watch socket
-/// associated with the elapsed timer as "ready". The thread waits
-/// in the callback function until it is notified by the main thread
-/// (via conditional variable), that one of the watch sockets has
-/// been cleared. It then checks if the main thread cleared the
-/// socket that the worker thread had set. It continues to block
-/// if this was a different socket. It returns (unblocks) otherwise.
-/// The main thread clears the socket when the @c IfaceMgr detects
-/// that this socket has been marked ready by the worker thread.
-/// This is triggered only when the @c IfaceMgr::receive4 or
-/// @c IfaceMgr::receive6 is called. They are called in the main
-/// loops of the DHCP servers, which are also responsible for
-/// processing received packets. Therefore it may take some
-/// time for the main loop to detect that the socket has been
-/// marked ready, call appropriate handler for it and clear it.
-/// In the mean time, the worker thread will remain blocked.
-///
+/// Before the @c TimerMgr can be used the server process must call
+/// @c TimerMgr::setIOService to associate the manager with the IO service
+/// that the server is using to its run tasks.
 class TimerMgr : public boost::noncopyable {
 public:
 
@@ -122,16 +63,15 @@ public:
     /// registered timers.
     ~TimerMgr();
 
+    /// @brief Sets IO service to be used by the Timer Manager.
+    ///
+    /// @param io_service Pointer to the new IO service.
+    void setIOService(const asiolink::IOServicePtr& io_service);
+
     /// @name Registering, unregistering and scheduling the timers.
     //@{
 
-    /// @brief Registers new timers in the @c TimerMgr.
-    ///
-    /// This method must not be called while the worker thread is running,
-    /// as it modifies the internal data structure holding registered
-    /// timers, which is also accessed from the worker thread via the
-    /// callback. Inserting new element to this data structure and
-    /// reading it at the same time would yield undefined behavior.
+    /// @brief Registers new timer in the @c TimerMgr.
     ///
     /// @param timer_name Unique name for the timer.
     /// @param callback Pointer to the callback function to be invoked
@@ -142,7 +82,6 @@ public:
     /// @c asiolink::IntervalTimer::Mode.
     ///
     /// @throw BadValue if the timer name is invalid or duplicate.
-    /// @throw InvalidOperation if the worker thread is running.
     void registerTimer(const std::string& timer_name,
                        const asiolink::IntervalTimer::Callback& callback,
                        const long interval,
@@ -150,15 +89,8 @@ public:
 
     /// @brief Unregisters specified timer.
     ///
-    /// This method cancels the timer if it is setup. It removes the external
-    /// socket from the @c IfaceMgr and closes it. It finally removes the
-    /// timer from the internal collection of timers.
-    ///
-    /// This method must not be called while the worker thread is running,
-    /// as it modifies the internal data structure holding registered
-    /// timers, which is also accessed from the worker thread via the
-    /// callback. Removing element from this data structure and
-    /// reading it at the same time would yield undefined behavior.
+    /// This method cancels the timer if it is setup and removes it from the
+    /// internal collection of timers.
     ///
     /// @param timer_name Name of the timer to be unregistered.
     ///
@@ -197,50 +129,6 @@ public:
 
     //@}
 
-    /// @name Starting and stopping the worker thread.
-    //@{
-
-    /// @brief Starts worker thread
-    ///
-    /// This method has no effect if the thread has already been started.
-    void startThread();
-
-    /// @brief Stops worker thread.
-    ///
-    /// When the thread is being stopped, it is possible that some of the
-    /// timers have elapsed and marked their respective watch sockets
-    /// as "ready", but the sockets haven't been yet cleared in the
-    /// main thread and the installed callbacks haven't been executed.
-    /// It is possible to control whether those pending callbacks should
-    /// be executed or not before the call to @c stopThread ends.
-    /// If the thread is being stopped as a result of the DHCP server
-    /// reconfiguration running pending callback may take significant
-    /// amount of time, e.g. when operations on the lease database are
-    /// involved. If this is a concern, the function parameter should
-    /// be left at its default value. In this case, however, it is
-    /// important to note that callbacks installed on ONE_SHOT timers
-    /// often reschedule the timer. If such callback is not executed
-    /// the timer will have to be setup by the application when the
-    /// thread is started again.
-    ///
-    /// Setting the @c run_pending_callbacks to true will guarantee
-    /// that all callbacks for which the timers have already elapsed
-    /// (and marked their watch sockets as ready) will be executed
-    /// prior to the return from @c stopThread method. However, this
-    /// should be avoided if the longer execution time of the
-    /// @c stopThread function is a concern.
-    ///
-    /// This method has no effect if the thread is not running.
-    ///
-    /// @param run_pending_callbacks Indicates if the pending callbacks
-    /// should be executed (if true).
-    void stopThread(const bool run_pending_callbacks = false);
-
-    //@}
-
-    /// @brief Returns a reference to IO service used by the @c TimerMgr.
-    asiolink::IOService& getIOService() const;
-
 private:
 
     /// @brief Private default constructor.