Browse Source

[master] Merge branch 'trac5317a'

Marcin Siodelski 7 years ago
parent
commit
4139a2f41b
38 changed files with 980 additions and 1564 deletions
  1. 6 24
      src/bin/dhcp4/ctrl_dhcp4_srv.cc
  2. 6 3
      src/bin/dhcp4/dhcp4_srv.cc
  3. 11 1
      src/bin/dhcp4/dhcp4_srv.h
  4. 60 8
      src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
  5. 11 11
      src/bin/dhcp4/tests/kea_controller_unittest.cc
  6. 6 26
      src/bin/dhcp6/ctrl_dhcp6_srv.cc
  7. 7 2
      src/bin/dhcp6/dhcp6_srv.cc
  8. 10 0
      src/bin/dhcp6/dhcp6_srv.h
  9. 57 7
      src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
  10. 9 11
      src/bin/dhcp6/tests/kea_controller_unittest.cc
  11. 2 2
      src/lib/asiolink/interval_timer.cc
  12. 132 0
      src/lib/asiolink/io_acceptor.h
  13. 4 69
      src/lib/asiolink/tcp_acceptor.h
  14. 1 2
      src/lib/asiolink/tests/interval_timer_unittest.cc
  15. 5 0
      src/lib/asiolink/unix_domain_socket.cc
  16. 5 0
      src/lib/asiolink/unix_domain_socket.h
  17. 65 0
      src/lib/asiolink/unix_domain_socket_acceptor.h
  18. 50 0
      src/lib/asiolink/unix_domain_socket_endpoint.h
  19. 0 2
      src/lib/config/Makefile.am
  20. 13 31
      src/lib/config/command-socket.dox
  21. 303 92
      src/lib/config/command_mgr.cc
  22. 34 48
      src/lib/config/command_mgr.h
  23. 0 43
      src/lib/config/command_socket.cc
  24. 0 105
      src/lib/config/command_socket.h
  25. 0 226
      src/lib/config/command_socket_factory.cc
  26. 0 39
      src/lib/config/command_socket_factory.h
  27. 4 0
      src/lib/config/config_messages.mes
  28. 0 1
      src/lib/config/tests/Makefile.am
  29. 64 1
      src/lib/config/tests/command_mgr_unittests.cc
  30. 0 94
      src/lib/config/tests/command_socket_factory_unittests.cc
  31. 0 33
      src/lib/dhcpsrv/dhcpsrv_messages.mes
  32. 22 48
      src/lib/dhcpsrv/libdhcpsrv.dox
  33. 0 6
      src/lib/dhcpsrv/memfile_lease_mgr.cc
  34. 16 13
      src/lib/dhcpsrv/tests/cfg_expiration_unittest.cc
  35. 14 21
      src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
  36. 21 116
      src/lib/dhcpsrv/tests/timer_mgr_unittest.cc
  37. 28 353
      src/lib/dhcpsrv/timer_mgr.cc
  38. 14 126
      src/lib/dhcpsrv/timer_mgr.h

+ 6 - 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,12 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
     }
     server_ = this; // remember this instance for later use in handlers
 
+    // TimerMgr uses IO service to run asynchronous timers.
+    TimerMgr::instance()->setIOService(getIOService());
+
+    // CommandMgr uses IO service to run asynchronous socket operations.
+    CommandMgr::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 +661,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);

+ 60 - 8
src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <config.h>
 
+#include <asiolink/io_service.h>
 #include <cc/command_interpreter.h>
 #include <config/command_mgr.h>
 #include <dhcp/dhcp4.h>
@@ -82,10 +83,20 @@ public:
     ~CtrlChannelDhcpv4SrvTest() {
         LeaseMgrFactory::destroy();
         StatsMgr::instance().removeAll();
+
+        CommandMgr::instance().closeCommandSocket();
+
         server_.reset();
-        reset();
     };
 
+    /// @brief Returns pointer to the server's IO service.
+    ///
+    /// @return Pointer to the server's IO service or null pointer if the server
+    /// hasn't been created.
+    IOServicePtr getIOService() {
+        return (server_ ? server_->getIOService() : IOServicePtr());
+    }
+
     void createUnixChannelServer() {
         ::remove(socket_path_.c_str());
 
@@ -179,15 +190,16 @@ public:
         client.reset(new UnixControlClient());
         ASSERT_TRUE(client);
 
-        // Connect and then call server's receivePacket() so it can
-        // detect the control socket connect and call the  accept handler
+        // Connect to the server. This is expected to trigger server's acceptor
+        // handler when IOService::poll() is run.
         ASSERT_TRUE(client->connectToServer(socket_path_));
-        ASSERT_NO_THROW(server_->receivePacket(0));
+        ASSERT_NO_THROW(getIOService()->poll());
 
-        // Send the command and then call server's receivePacket() so it can
-        // detect the inbound data and call the read handler
+        // Send the command. This will trigger server's handler which receives
+        // data over the unix domain socket. The server will start sending
+        // response to the client.
         ASSERT_TRUE(client->sendCommand(command));
-        ASSERT_NO_THROW(server_->receivePacket(0));
+        ASSERT_NO_THROW(getIOService()->poll());
 
         // Read the response generated by the server. Note that getResponse
         // only fails if there an IO error or no response data was present.
@@ -196,7 +208,8 @@ public:
 
         // Now disconnect and process the close event
         client->disconnectFromServer();
-        ASSERT_NO_THROW(server_->receivePacket(0));
+
+        ASSERT_NO_THROW(getIOService()->poll());
     }
 
     /// @brief Checks response for list-commands
@@ -1056,4 +1069,43 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configReloadValid) {
     ::remove("test8.json");
 }
 
+/// Verify that concurrent connections over the control channel can be
+///  established.
+/// @todo Future Kea 1.3 tickets will modify the behavior of the CommandMgr
+/// such that the server will be able to send response in multiple chunks.
+/// This test will need to be extended. For now, the receive and write
+/// operations are atomic and there is no conflict between concurrent
+/// connections.
+TEST_F(CtrlChannelDhcpv4SrvTest, concurrentConnections) {
+    createUnixChannelServer();
+
+    boost::scoped_ptr<UnixControlClient> client1(new UnixControlClient());
+    ASSERT_TRUE(client1);
+
+    boost::scoped_ptr<UnixControlClient> client2(new UnixControlClient());
+    ASSERT_TRUE(client2);
+
+    // Client 1 connects.
+    ASSERT_TRUE(client1->connectToServer(socket_path_));
+    ASSERT_NO_THROW(getIOService()->poll());
+
+    // Client 2 connects.
+    ASSERT_TRUE(client2->connectToServer(socket_path_));
+    ASSERT_NO_THROW(getIOService()->poll());
+
+    // Send the command while another client is connected.
+    ASSERT_TRUE(client2->sendCommand("{ \"command\": \"list-commands\" }"));
+    ASSERT_NO_THROW(getIOService()->poll());
+
+    std::string response;
+    // The server should respond ok.
+    ASSERT_TRUE(client2->getResponse(response));
+    EXPECT_TRUE(response.find("\"result\": 0") != std::string::npos);
+
+    // Disconnect the servers.
+    client1->disconnectFromServer();
+    client2->disconnectFromServer();
+    ASSERT_NO_THROW(getIOService()->poll());
+}
+
 } // End of anonymous namespace

+ 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.
 

+ 6 - 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,12 @@ ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
     }
     server_ = this; // remember this instance for use in callback
 
+    // TimerMgr uses IO service to run asynchronous timers.
+    TimerMgr::instance()->setIOService(getIOService());
+
+    // CommandMgr uses IO service to run asynchronous socket operations.
+    CommandMgr::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 +682,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);

+ 57 - 7
src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc

@@ -114,6 +114,14 @@ public:
         reset();
     };
 
+    /// @brief Returns pointer to the server's IO service.
+    ///
+    /// @return Pointer to the server's IO service or null pointer if the server
+    /// hasn't been created.
+    IOServicePtr getIOService() {
+        return (server_ ? server_->getIOService() : IOServicePtr());
+    }
+
     void createUnixChannelServer() {
         static_cast<void>(::remove(socket_path_.c_str()));
 
@@ -192,15 +200,16 @@ public:
         client.reset(new UnixControlClient());
         ASSERT_TRUE(client);
 
-        // Connect and then call server's receivePacket() so it can
-        // detect the control socket connect and call the  accept handler
+        // Connect to the server. This is expected to trigger server's acceptor
+        // handler when IOService::poll() is run.
         ASSERT_TRUE(client->connectToServer(socket_path_));
-        ASSERT_NO_THROW(server_->receivePacket(0));
+        ASSERT_NO_THROW(getIOService()->poll());
 
-        // Send the command and then call server's receivePacket() so it can
-        // detect the inbound data and call the read handler
+        // Send the command. This will trigger server's handler which receives
+        // data over the unix domain socket. The server will start sending
+        // response to the client.
         ASSERT_TRUE(client->sendCommand(command));
-        ASSERT_NO_THROW(server_->receivePacket(0));
+        ASSERT_NO_THROW(getIOService()->poll());
 
         // Read the response generated by the server. Note that getResponse
         // only fails if there an IO error or no response data was present.
@@ -209,7 +218,8 @@ public:
 
         // Now disconnect and process the close event
         client->disconnectFromServer();
-        ASSERT_NO_THROW(server_->receivePacket(0));
+
+        ASSERT_NO_THROW(getIOService()->poll());
     }
 
     /// @brief Checks response for list-commands
@@ -1081,4 +1091,44 @@ TEST_F(CtrlChannelDhcpv6SrvTest, configReloadValid) {
     ::remove("test8.json");
 }
 
+/// Verify that concurrent connections over the control channel can be
+///  established.
+/// @todo Future Kea 1.3 tickets will modify the behavior of the CommandMgr
+/// such that the server will be able to send response in multiple chunks.
+/// This test will need to be extended. For now, the receive and write
+/// operations are atomic and there is no conflict between concurrent
+/// connections.
+TEST_F(CtrlChannelDhcpv6SrvTest, concurrentConnections) {
+    createUnixChannelServer();
+
+    boost::scoped_ptr<UnixControlClient> client1(new UnixControlClient());
+    ASSERT_TRUE(client1);
+
+    boost::scoped_ptr<UnixControlClient> client2(new UnixControlClient());
+    ASSERT_TRUE(client2);
+
+    // Client 1 connects.
+    ASSERT_TRUE(client1->connectToServer(socket_path_));
+    ASSERT_NO_THROW(getIOService()->poll());
+
+    // Client 2 connects.
+    ASSERT_TRUE(client2->connectToServer(socket_path_));
+    ASSERT_NO_THROW(getIOService()->poll());
+
+    // Send the command while another client is connected.
+    ASSERT_TRUE(client2->sendCommand("{ \"command\": \"list-commands\" }"));
+    ASSERT_NO_THROW(getIOService()->poll());
+
+    std::string response;
+    // The server should respond ok.
+    ASSERT_TRUE(client2->getResponse(response));
+    EXPECT_TRUE(response.find("\"result\": 0") != std::string::npos);
+
+    // Disconnect the servers.
+    client1->disconnectFromServer();
+    client2->disconnectFromServer();
+    ASSERT_NO_THROW(getIOService()->poll());
+}
+
+
 } // End of anonymous namespace

+ 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");
     }

+ 132 - 0
src/lib/asiolink/io_acceptor.h

@@ -0,0 +1,132 @@
+// Copyright (C) 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
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef IO_ACCEPTOR_H
+#define IO_ACCEPTOR_H
+
+#ifndef BOOST_ASIO_HPP
+#error "asio.hpp must be included before including this, see asiolink.h as to why"
+#endif
+
+#include <asiolink/io_service.h>
+#include <asiolink/io_socket.h>
+
+namespace isc {
+namespace asiolink {
+
+/// @brief Base class for acceptor services in Kea.
+///
+/// This is a wrapper class for ASIO acceptor service. Classes implementing
+/// services for specific protocol types should derive from this class.
+///
+/// Acceptor is an IO object which accepts incoming connections into a socket
+/// object. This socket is then used for data transmission from the client
+/// to server and back. The acceptor is continued to be used to accept new
+/// connections while the accepted connection is active.
+///
+/// @tparam ProtocolType ASIO protocol type, e.g. stream_protocol
+/// @tparam CallbackType Callback function type which should have the following
+/// signature: @c void(const boost::system::error_code&).
+template<typename ProtocolType, typename CallbackType>
+class IOAcceptor : public IOSocket {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param io_service Reference to the IO service.
+    explicit IOAcceptor(IOService& io_service)
+        : IOSocket(),
+          acceptor_(new typename ProtocolType::acceptor(io_service.get_io_service())) {
+    }
+
+    /// @brief Destructor.
+    virtual ~IOAcceptor() { }
+
+    /// @brief Returns file descriptor of the underlying socket.
+    virtual int getNative() const {
+        return (acceptor_->native());
+    }
+
+    /// @brief Opens acceptor socket given the endpoint.
+    ///
+    /// @param endpoint Reference to the endpoint object defining local
+    /// acceptor endpoint.
+    ///
+    /// @tparam EndpointType Endpoint type.
+    template<typename EndpointType>
+    void open(const EndpointType& endpoint) {
+        acceptor_->open(endpoint.getASIOEndpoint().protocol());
+    }
+
+    /// @brief Binds socket to an endpoint.
+    ///
+    /// @param endpoint Reference to the endpoint object defining local
+    /// acceptor endpoint.
+    ///
+    /// @tparam EndpointType Endpoint type.
+    template<typename EndpointType>
+    void bind(const EndpointType& endpoint) {
+        acceptor_->bind(endpoint.getASIOEndpoint());
+    }
+
+    /// @brief Sets socket option.
+    ///
+    /// @param socket_option Reference to the object encapsulating an option to
+    /// be set for the socket.
+    /// @tparam SettableSocketOption Type of the object encapsulating socket option
+    /// being set.
+    template<typename SettableSocketOption>
+    void setOption(const SettableSocketOption& socket_option) {
+        acceptor_->set_option(socket_option);
+    }
+
+    /// @brief Starts listening new connections.
+    void listen() {
+        acceptor_->listen();
+    }
+
+    /// @brief Checks if the acceptor is open.
+    ///
+    /// @return true if acceptor is open.
+    bool isOpen() const {
+        return (acceptor_->is_open());
+    }
+
+    /// @brief Closes the acceptor.
+    void close() const {
+        acceptor_->close();
+    }
+
+protected:
+
+    /// @brief Asynchronously accept new connection.
+    ///
+    /// This method accepts new connection into the specified socket. When the
+    /// new connection arrives or an error occurs the specified callback
+    /// function is invoked.
+    ///
+    /// @param socket Socket into which connection should be accepted.
+    /// @param callback Callback function to be invoked when the new connection
+    /// arrives.
+    /// @tparam SocketType Socket type, e.g. @ref UnixDomainSocket. It must
+    /// implement @c getASIOSocket method.
+    template<typename SocketType>
+    void asyncAcceptInternal(const SocketType& socket,
+                             const CallbackType& callback) {
+        acceptor_->async_accept(socket.getASIOSocket(), callback);
+    }
+
+
+    /// @brief Underlying ASIO acceptor implementation.
+    boost::shared_ptr<typename ProtocolType::acceptor> acceptor_;
+
+};
+
+
+} // end of namespace asiolink
+} // end of isc
+
+#endif // IO_ACCEPTOR_H

+ 4 - 69
src/lib/asiolink/tcp_acceptor.h

@@ -11,6 +11,7 @@
 #error "asio.hpp must be included before including this, see asiolink.h as to why"
 #endif
 
+#include <asiolink/io_acceptor.h>
 #include <asiolink/io_service.h>
 #include <asiolink/io_socket.h>
 #include <asiolink/tcp_endpoint.h>
@@ -28,23 +29,14 @@ namespace asiolink {
 ///
 /// @tparam C Acceptor callback type.
 template<typename C>
-class TCPAcceptor : public IOSocket {
+class TCPAcceptor : public IOAcceptor<boost::asio::ip::tcp, C> {
 public:
 
     /// @brief Constructor.
     ///
     /// @param io_service IO service.
     explicit TCPAcceptor(IOService& io_service)
-        : IOSocket(),
-          acceptor_(new boost::asio::ip::tcp::acceptor(io_service.get_io_service())) {
-    }
-
-    /// @brief Destructor.
-    virtual ~TCPAcceptor() { }
-
-    /// @brief Returns file descriptor of the underlying socket.
-    virtual int getNative() const final {
-        return (acceptor_->native());
+        : IOAcceptor<boost::asio::ip::tcp, C>(io_service) {
     }
 
     /// @brief Returns protocol of the socket.
@@ -54,45 +46,6 @@ public:
         return (IPPROTO_TCP);
     }
 
-    /// @brief Opens acceptor socket given the endpoint.
-    ///
-    /// @param endpoint Reference to the endpoint object which specifies the
-    /// address and port on which the acceptor service will run.
-    void open(const TCPEndpoint& endpoint) {
-        acceptor_->open(endpoint.getASIOEndpoint().protocol());
-    }
-
-    /// @brief Sets socket option.
-    ///
-    /// Typically, this method is used to set SO_REUSEADDR option on the socket:
-    /// @code
-    /// IOService io_service;
-    /// TCPAcceptor<Callback> acceptor(io_service);
-    /// acceptor.setOption(TCPAcceptor::ReuseAddress(true))
-    /// @endcode
-    ///
-    /// @param socket_option Reference to the object encapsulating an option to
-    /// be set for the socket.
-    /// @tparam SettableSocketOption Type of the object encapsulating socket option
-    /// being set.
-    template<typename SettableSocketOption>
-    void setOption(const SettableSocketOption& socket_option) {
-        acceptor_->set_option(socket_option);
-    }
-
-    /// @brief Binds socket to an endpoint.
-    ///
-    /// @param endpoint Reference to an endpoint to which the socket is to
-    /// be bound.
-    void bind(const TCPEndpoint& endpoint) {
-        acceptor_->bind(endpoint.getASIOEndpoint());
-    }
-
-    /// @brief Starts listening for the new connections.
-    void listen() {
-        acceptor_->listen();
-    }
-
     /// @brief Asynchronously accept new connection.
     ///
     /// This method accepts new connection into the specified socket. When the
@@ -105,26 +58,8 @@ public:
     /// @tparam SocketCallback Type of the callback for the @ref TCPSocket.
     template<typename SocketCallback>
     void asyncAccept(const TCPSocket<SocketCallback>& socket, C& callback) {
-        acceptor_->async_accept(socket.getASIOSocket(), callback);
-    }
-
-    /// @brief Checks if the acceptor is open.
-    ///
-    /// @return true if acceptor is open.
-    bool isOpen() const {
-        return (acceptor_->is_open());
+        IOAcceptor<boost::asio::ip::tcp, C>::asyncAcceptInternal(socket, callback);
     }
-
-    /// @brief Closes the acceptor.
-    void close() const {
-        acceptor_->close();
-    }
-
-private:
-
-    /// @brief Underlying ASIO acceptor implementation.
-    boost::shared_ptr<boost::asio::ip::tcp::acceptor> acceptor_;
-
 };
 
 

+ 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);
 }
 

+ 5 - 0
src/lib/asiolink/unix_domain_socket.cc

@@ -317,5 +317,10 @@ UnixDomainSocket::close() {
     impl_->close();
 }
 
+boost::asio::local::stream_protocol::socket&
+UnixDomainSocket::getASIOSocket() const {
+    return (impl_->socket_);
+}
+
 } // end of namespace asiolink
 } // end of namespace isc

+ 5 - 0
src/lib/asiolink/unix_domain_socket.h

@@ -107,6 +107,11 @@ public:
     /// @brief Closes the socket.
     void close();
 
+    /// @brief Returns reference to the underlying ASIO socket.
+    ///
+    /// @return Reference to underlying ASIO socket.
+    virtual boost::asio::local::stream_protocol::socket& getASIOSocket() const;
+
 private:
 
     /// @brief Pointer to the implementation of this class.

+ 65 - 0
src/lib/asiolink/unix_domain_socket_acceptor.h

@@ -0,0 +1,65 @@
+// Copyright (C) 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
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef UNIX_DOMAIN_SOCKET_ACCEPTOR_H
+#define UNIX_DOMAIN_SOCKET_ACCEPTOR_H
+
+#ifndef BOOST_ASIO_HPP
+#error "asio.hpp must be included before including this, see asiolink.h as to why"
+#endif
+
+#include <asiolink/io_acceptor.h>
+#include <asiolink/unix_domain_socket.h>
+#include <functional>
+
+namespace isc {
+namespace asiolink {
+
+/// @brief Implements acceptor service for @ref UnixDomainSocket.
+///
+/// This class is used to accept new incoming connections over unix domain
+/// sockets.
+class UnixDomainSocketAcceptor : public IOAcceptor<boost::asio::local::stream_protocol,
+                                 std::function<void(const boost::system::error_code&)> > {
+public:
+
+    /// @brief Callback type used in call to @ref UnixDomainSocketAcceptor::asyncAccept.
+    typedef std::function<void(const boost::system::error_code&)> AcceptHandler;
+
+    /// @brief Constructor.
+    ///
+    /// @param io_service Reference to the IO service.
+    explicit UnixDomainSocketAcceptor(IOService& io_service)
+        : IOAcceptor<boost::asio::local::stream_protocol,
+                     std::function<void(const boost::system::error_code&)> >(io_service) {
+    }
+
+    /// @brief Returns the transport protocol of the socket.
+    ///
+    /// @return AF_LOCAL.
+    virtual int getProtocol() const final {
+        return (AF_LOCAL);
+    }
+
+    /// @brief Asynchronously accept new connection.
+    ///
+    /// This method accepts new connection into the specified socket. When the
+    /// new connection arrives or an error occurs the specified callback function
+    /// is invoked.
+    ///
+    /// @param socket Socket into which connection should be accepted.
+    /// @param callback Callback function to be invoked when the new connection
+    /// arrives.
+    /// @tparam SocketType
+    void asyncAccept(const UnixDomainSocket& socket, const AcceptHandler& callback) {
+        asyncAcceptInternal(socket, callback);
+    }
+};
+
+} // end of namespace isc::asiolink
+} // end of namespace isc
+
+#endif // UNIX_DOMAIN_SOCKET_ACCEPTOR_H

+ 50 - 0
src/lib/asiolink/unix_domain_socket_endpoint.h

@@ -0,0 +1,50 @@
+// Copyright (C) 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
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+#ifndef UNIX_DOMAIN_SOCKET_ENDPOINT_H
+#define UNIX_DOMAIN_SOCKET_ENDPOINT_H
+
+#ifndef BOOST_ASIO_HPP
+#error "asio.hpp must be included before including this, see asiolink.h as to why"
+#endif
+
+#include <string>
+
+namespace isc {
+namespace asiolink {
+
+/// @brief Endpoint for @ref UnixDomainSocket.
+///
+/// This is a simple class encapsulating ASIO unix domain socket endpoint.
+/// It is used to represent endpoints taking part in communication via
+/// unix domain sockets.
+class UnixDomainSocketEndpoint {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// @param endpoint_path Path to the socket descriptor.
+    explicit UnixDomainSocketEndpoint(const std::string& endpoint_path)
+        : endpoint_(endpoint_path) {
+    }
+
+    /// @brief Returns underlying ASIO endpoint.
+    const boost::asio::local::stream_protocol::endpoint&
+    getASIOEndpoint() const {
+        return (endpoint_);
+    }
+
+private:
+
+    /// @brief Underlying ASIO endpoint.
+    boost::asio::local::stream_protocol::endpoint endpoint_;
+
+};
+
+} // end of namespace isc::asiolink
+} // end of namespace isc
+
+#endif // UNIX_DOMAIN_SOCKET_ENDPOINT_H

+ 0 - 2
src/lib/config/Makefile.am

@@ -18,8 +18,6 @@ libkea_cfgclient_la_SOURCES += module_spec.h module_spec.cc
 libkea_cfgclient_la_SOURCES += base_command_mgr.cc base_command_mgr.h
 libkea_cfgclient_la_SOURCES += client_connection.cc client_connection.h
 libkea_cfgclient_la_SOURCES += command_mgr.cc command_mgr.h
-libkea_cfgclient_la_SOURCES += command_socket.cc command_socket.h
-libkea_cfgclient_la_SOURCES += command_socket_factory.cc command_socket_factory.h
 libkea_cfgclient_la_SOURCES += config_log.h config_log.cc
 libkea_cfgclient_la_SOURCES += hooked_command_mgr.cc hooked_command_mgr.h
 

+ 13 - 31
src/lib/config/command-socket.dox

@@ -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
@@ -154,39 +154,21 @@ or HTTPS connection):
 - @ref isc::config::CommandMgr::openCommandSocket that passes structure defined
   in the configuration file. Currently only two parameters are supported: socket-type
   (which must contain value 'unix') and socket-name (which contains unix path for
-  the named socket to be created). This method calls @ref
-  isc::config::CommandSocketFactory::create method, which parses the parameters
-  and instantiates one object from a class derived from @ref isc::config::CommandSocket.
-  Again, currently only UNIX type is supported, but the factory
-  class is expected to be extended to cover additional types.
+  the named socket to be created).
 - @ref isc::config::CommandMgr::closeCommandSocket() - it is used to close the
-  socket. It calls close method on the @ref isc::config::CommandSocket object, if
-  one exists. In particular, for UNIX socket, it also deletes the file after socket
-  was closed.
+  socket.
 
 @section ctrlSocketConnections Accepting connections
 
-Control Channel is connection oriented communication. In that sense it is
-different than all other communications supported so far in Kea. To facilitate
-connections, several mechanisms were implemented. Intially a single UNIX socket
-it opened (see @c isc::config::UnixCommandSocket in
-src/lib/config/command_socket_factory.cc). Its @ref
-isc::config::UnixCommandSocket::receiveHandler callback method is
-installed in @ref isc::dhcp::IfaceMgr to process incoming connections. When the
-select call in @ref isc::dhcp::IfaceMgr::receive4 indicates that there is some data to be
-processed, this callback calls accept, which creates a new socket for handling
-this particular incoming connection. Once the socket descriptor is known, a new
-instance of @ref isc::config::ConnectionSocket is created to represent that
-socket (and the whole ongoing connection). It installs another callback
-(@ref isc::config::ConnectionSocket::receiveHandler that calls
-(@ref isc::config::CommandMgr::commandReader) that will process incoming
-data or will close the socket when necessary. CommandReader reads data from
-incoming socket and attempts to parse it as JSON structures. If successful,
-it calls isc::config::CommandMgr::processCommand(), serializes the structure
-returned and attempts to send it back.
-
-@todo Currently commands and responses up to 64KB are supported. It was deemed
-sufficient for the current needs, but in the future we may need to extend
-it to handle bigger structures.
+The @ref isc::config::CommandMgr is implemented using boost ASIO and uses
+asynchronous calls to accept new connections and receive commands from the
+controlling clients. ASIO uses IO service object to run asynchronous calls.
+Thus, before the server can use the @ref isc::dhcp::CommandMgr it must
+provide it with a common instance of the @ref isc::asiolink::IOService
+object using @ref isc::dhcp::CommandMgr::setIOService. The server's
+main loop must contain calls to @ref isc::asiolink::IOService::run or
+@ref isc::asiolink::IOService::poll or their variants to invoke Command
+Manager's handlers as required for processing control requests.
+
 
 */

+ 303 - 92
src/lib/config/command_mgr.cc

@@ -4,142 +4,205 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+#include <asiolink/asio_wrapper.h>
+#include <asiolink/io_service.h>
+#include <asiolink/unix_domain_socket.h>
+#include <asiolink/unix_domain_socket_acceptor.h>
+#include <asiolink/unix_domain_socket_endpoint.h>
 #include <config/command_mgr.h>
-#include <config/command_socket_factory.h>
 #include <cc/data.h>
 #include <cc/command_interpreter.h>
 #include <dhcp/iface_mgr.h>
 #include <config/config_log.h>
 #include <boost/bind.hpp>
+#include <boost/enable_shared_from_this.hpp>
+#include <array>
 #include <unistd.h>
 
+using namespace isc;
+using namespace isc::asiolink;
+using namespace isc::config;
 using namespace isc::data;
 
-namespace isc {
-namespace config {
-
-CommandMgr::CommandMgr()
-    : HookedCommandMgr() {
-}
-
-CommandSocketPtr
-CommandMgr::openCommandSocket(const isc::data::ConstElementPtr& socket_info) {
-    if (socket_) {
-        isc_throw(SocketError, "There is already a control socket open");
+namespace {
+
+class ConnectionPool;
+
+/// @brief Represents a single connection over control socket.
+///
+/// An instance of this object is created when the @c CommandMgr acceptor
+/// receives new connection from a controlling client.
+class Connection : public boost::enable_shared_from_this<Connection> {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// This constructor registers a socket of this connection in the Interface
+    /// Manager to cause the blocking call to @c select() to return as soon as
+    /// a transmission over the control socket is received.
+    Connection(const boost::shared_ptr<UnixDomainSocket>& socket,
+               ConnectionPool& connection_pool)
+        : socket_(socket), connection_pool_(connection_pool),
+          response_in_progress_(false) {
+        // Callback value of 0 is used to indicate that callback function is
+        // not installed.
+        isc::dhcp::IfaceMgr::instance().addExternalSocket(socket_->getNative(), 0);
     }
 
-    socket_ = CommandSocketFactory::create(socket_info);
+    /// @brief Start asynchronous read over the unix domain socket.
+    ///
+    /// This method doesn't block. Once the transmission is received over the
+    /// socket, the @c Connection::receiveHandler callback is invoked to
+    /// process received data.
+    void start() {
+        socket_->asyncReceive(&buf_[0], sizeof(buf_),
+                              boost::bind(&Connection::receiveHandler,
+                                          shared_from_this(), _1, _2));
 
-    return (socket_);
-}
 
-void CommandMgr::closeCommandSocket() {
-    // First, let's close the socket for incoming new connections.
-    if (socket_) {
-        socket_->close();
-        socket_.reset();
     }
 
-    // Now let's close all existing connections that we may have.
-    for (std::list<CommandSocketPtr>::iterator conn = connections_.begin();
-         conn != connections_.end(); ++conn) {
-        (*conn)->close();
+    /// @brief Close current connection.
+    ///
+    /// Connection is not closed if the invocation of this method is a result of
+    /// server reconfiguration. The connection will be closed once a response is
+    /// sent to the client. Closing a socket during processing a request would
+    /// cause the server to not send a response to the client.
+    void stop() {
+        if (!response_in_progress_) {
+            LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_CLOSED)
+                .arg(socket_->getNative());
+
+            isc::dhcp::IfaceMgr::instance().deleteExternalSocket(socket_->getNative());
+            socket_->close();
+        }
     }
-    connections_.clear();
-}
 
+    /// @brief Handler invoked when the data is received over the control
+    /// socket.
+    ///
+    /// @param ec Error code.
+    /// @param bytes_transferred Number of bytes received.
+    void receiveHandler(const boost::system::error_code& ec,
+                        size_t bytes_transferred);
 
-void CommandMgr::addConnection(const CommandSocketPtr& conn) {
-    connections_.push_back(conn);
-}
+private:
+
+    /// @brief Pointer to the socket used for transmission.
+    boost::shared_ptr<UnixDomainSocket> socket_;
+
+    /// @brief Buffer used for received data.
+    std::array<char, 65535> buf_;
 
-bool CommandMgr::closeConnection(int fd) {
+    /// @brief Reference to the pool of connections.
+    ConnectionPool& connection_pool_;
 
-    // Let's iterate over all currently registered connections.
-    for (std::list<CommandSocketPtr>::iterator conn = connections_.begin();
-         conn != connections_.end(); ++conn) {
+    /// @brief Boolean flag indicating if the request to stop connection is a
+    /// result of server reconfiguration.
+    bool response_in_progress_;
 
-        // If found, close it.
-        if ((*conn)->getFD() == fd) {
-            (*conn)->close();
-            connections_.erase(conn);
-            return (true);
+};
+
+/// @brief Pointer to the @c Connection.
+typedef boost::shared_ptr<Connection> ConnectionPtr;
+
+/// @brief Holds all open connections.
+class ConnectionPool {
+public:
+
+    /// @brief Starts new connection.
+    ///
+    /// @param connection Pointer to the new connection object.
+    void start(const ConnectionPtr& connection) {
+        connection->start();
+        connections_.insert(connection);
+    }
+
+    /// @brief Stops running connection.
+    ///
+    /// @param connection Pointer to the new connection object.
+    void stop(const ConnectionPtr& connection) {
+        connection->stop();
+        connections_.erase(connection);
+    }
+
+    /// @brief Stops all connections which are allowed to stop.
+    void stopAll() {
+        for (auto conn = connections_.begin(); conn != connections_.end();
+             ++conn) {
+            (*conn)->stop();
         }
+        connections_.clear();
     }
 
-    return (false);
-}
+    size_t getConnectionsNum() const {
+        return (connections_.size());
+    }
 
-CommandMgr&
-CommandMgr::instance() {
-    static CommandMgr cmd_mgr;
-    return (cmd_mgr);
-}
+private:
 
-void
-CommandMgr::commandReader(int sockfd) {
+    /// @brief Pool of connections.
+    std::set<ConnectionPtr> connections_;
 
-    /// @todo: We do not handle commands that are larger than 64K.
+};
 
-    // We should not expect commands bigger than 64K.
-    char buf[65536];
-    memset(buf, 0, sizeof(buf));
-    ConstElementPtr cmd, rsp;
 
-    // Read incoming data.
-    int rval = read(sockfd, buf, sizeof(buf));
-    if (rval < 0) {
-        // Read failed
-        LOG_ERROR(command_logger, COMMAND_SOCKET_READ_FAIL).arg(rval).arg(sockfd);
+void
+Connection::receiveHandler(const boost::system::error_code& ec,
+                           size_t bytes_transferred) {
+    if (ec) {
+        if (ec.value() == boost::asio::error::eof) {
+            // Foreign host has closed the connection. We should remove it from the
+            // connection pool.
+            LOG_INFO(command_logger, COMMAND_SOCKET_CLOSED_BY_FOREIGN_HOST)
+                .arg(socket_->getNative());
+            connection_pool_.stop(shared_from_this());
+
+        } else if (ec.value() != boost::asio::error::operation_aborted) {
+            LOG_ERROR(command_logger, COMMAND_SOCKET_READ_FAIL)
+                .arg(ec.value()).arg(socket_->getNative());
+        }
 
         /// @todo: Should we close the connection, similar to what is already
-        /// being done for rval == 0?
+        /// being done for bytes_transferred == 0.
         return;
-    } else if (rval == 0) {
-
-        // Remove it from the active connections list.
-        instance().closeConnection(sockfd);
 
+    } else if (bytes_transferred == 0) {
+        // Nothing received. Close the connection.
+        connection_pool_.stop(shared_from_this());
         return;
     }
 
-    // Duplicate the connection's socket in the event, the command causes the
-    // channel to close (like a reconfig).  This permits us to always have
-    // a socket on which to respond. If for some reason  we can't fall back
-    // to the connection socket.
-    int rsp_fd = dup(sockfd);
-    if (rsp_fd < 0 ) {
-        // Highly unlikely
-        const char* errmsg = strerror(errno);
-        LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_DUP_WARN)
-                  .arg(errmsg);
-        rsp_fd = sockfd;
-    }
+    LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ)
+        .arg(bytes_transferred).arg(socket_->getNative());
 
-    LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_READ).arg(rval).arg(sockfd);
+    ConstElementPtr cmd, rsp;
 
-    // Ok, we received something. Let's see if we can make any sense of it.
     try {
 
         // Try to interpret it as JSON.
-        std::string sbuf(buf, static_cast<size_t>(rval));
+        std::string sbuf(&buf_[0], bytes_transferred);
         cmd = Element::fromJSON(sbuf, true);
 
+        response_in_progress_ = true;
+
         // If successful, then process it as a command.
         rsp = CommandMgr::instance().processCommand(cmd);
+
+        response_in_progress_ = false;
+
+
     } catch (const Exception& ex) {
         LOG_WARN(command_logger, COMMAND_PROCESS_ERROR1).arg(ex.what());
         rsp = createAnswer(CONTROL_RESULT_ERROR, std::string(ex.what()));
     }
 
+    // No response generated. Connection will be closed.
     if (!rsp) {
         LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR);
-        // Only close the duped socket if it's different (should be)
-        if (rsp_fd != sockfd) {
-            close(rsp_fd);
-        }
+        rsp = createAnswer(CONTROL_RESULT_ERROR,
+                           "internal server error: no response generated");
 
-        return;
     }
 
     // Let's convert JSON response to text. Note that at this stage
@@ -154,24 +217,172 @@ CommandMgr::commandReader(int sockfd) {
         len = 65535;
     }
 
-    // Send the data back over socket.
-    rval = write(rsp_fd, txt.c_str(), len);
-    int saverr = errno;
-
-    LOG_DEBUG(command_logger, DBG_COMMAND, COMMAND_SOCKET_WRITE).arg(len).arg(sockfd);
+    try {
+        // Send the data back over socket.
+        socket_->write(txt.c_str(), len);
 
-    if (rval < 0) {
+    } catch (const std::exception& ex) {
         // Response transmission failed. Since the response failed, it doesn't
         // make sense to send any status codes. Let's log it and be done with
         // it.
         LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
-                  .arg(len).arg(sockfd).arg(strerror(saverr));
+            .arg(len).arg(socket_->getNative()).arg(ex.what());
     }
 
-    // Only close the duped socket if it's different (should be)
-    if (rsp_fd != sockfd) {
-        close(rsp_fd);
+    connection_pool_.stop(shared_from_this());
+}
+
+}
+
+namespace isc {
+namespace config {
+
+/// @brief Implementation of the @c CommandMgr.
+class CommandMgrImpl {
+public:
+
+    /// @brief Constructor.
+    CommandMgrImpl()
+        : io_service_(), acceptor_(), socket_(), socket_name_(),
+          connection_pool_() {
     }
+
+    /// @brief Opens acceptor service allowing the control clients to connect.
+    ///
+    /// @param socket_info Configuration information for the control socket.
+    /// @throw BadSocketInfo When socket configuration is invalid.
+    /// @throw SocketError When socket operation fails.
+    void openCommandSocket(const isc::data::ConstElementPtr& socket_info);
+
+    /// @brief Asynchronously accepts next connection.
+    void doAccept();
+
+    /// @brief Pointer to the IO service used by the server process for running
+    /// asynchronous tasks.
+    IOServicePtr io_service_;
+
+    /// @brief Pointer to the acceptor service.
+    boost::shared_ptr<UnixDomainSocketAcceptor> acceptor_;
+
+    /// @brief Pointer to the socket into which the new connection is accepted.
+    boost::shared_ptr<UnixDomainSocket> socket_;
+
+    /// @brief Path to the unix domain socket descriptor.
+    ///
+    /// This is used to remove the socket file once the connection terminates.
+    std::string socket_name_;
+
+    /// @brief Pool of connections.
+    ConnectionPool connection_pool_;
+};
+
+void
+CommandMgrImpl::openCommandSocket(const isc::data::ConstElementPtr& socket_info) {
+    socket_name_.clear();
+
+    if(!socket_info) {
+        isc_throw(BadSocketInfo, "Missing socket_info parameters, can't create socket.");
+    }
+
+    ConstElementPtr type = socket_info->get("socket-type");
+    if (!type) {
+        isc_throw(BadSocketInfo, "Mandatory 'socket-type' parameter missing");
+    }
+
+    // Only supporting unix sockets right now.
+    if (type->stringValue() != "unix") {
+        isc_throw(BadSocketInfo, "Invalid 'socket-type' parameter value "
+                  << type->stringValue());
+    }
+
+    // UNIX socket is requested. It takes one parameter: socket-name that
+    // specifies UNIX path of the socket.
+    ConstElementPtr name = socket_info->get("socket-name");
+    if (!name) {
+        isc_throw(BadSocketInfo, "Mandatory 'socket-name' parameter missing");
+    }
+
+    if (name->getType() != Element::string) {
+        isc_throw(BadSocketInfo, "'socket-name' parameter expected to be a string");
+    }
+
+    socket_name_ = name->stringValue();
+
+    try {
+        // Start asynchronous acceptor service.
+        acceptor_.reset(new UnixDomainSocketAcceptor(*io_service_));
+        UnixDomainSocketEndpoint endpoint(socket_name_);
+        acceptor_->open(endpoint);
+        acceptor_->bind(endpoint);
+        acceptor_->listen();
+
+        // Install this socket in Interface Manager.
+        isc::dhcp::IfaceMgr::instance().addExternalSocket(acceptor_->getNative(), 0);
+
+        doAccept();
+
+    } catch (const std::exception& ex) {
+        isc_throw(SocketError, ex.what());
+    }
+}
+
+void
+CommandMgrImpl::doAccept() {
+    // Create a socket into which the acceptor will accept new connection.
+    socket_.reset(new UnixDomainSocket(*io_service_));
+    acceptor_->asyncAccept(*socket_, [this](const boost::system::error_code& ec) {
+        if (!ec) {
+            // New connection is arriving. Start asynchronous transmission.
+            ConnectionPtr connection(new Connection(socket_, connection_pool_));
+            connection_pool_.start(connection);
+        }
+
+        // Unless we're stopping the service, start accepting connections again.
+        if (ec.value() != boost::asio::error::operation_aborted) {
+            doAccept();
+        }
+    });
+}
+
+CommandMgr::CommandMgr()
+    : HookedCommandMgr(), impl_(new CommandMgrImpl()) {
+}
+
+void
+CommandMgr::openCommandSocket(const isc::data::ConstElementPtr& socket_info) {
+    impl_->openCommandSocket(socket_info);
+}
+
+void CommandMgr::closeCommandSocket() {
+    // Close acceptor if the acceptor is open.
+    if (impl_->acceptor_ && impl_->acceptor_->isOpen()) {
+        isc::dhcp::IfaceMgr::instance().deleteExternalSocket(impl_->acceptor_->getNative());
+        impl_->acceptor_->close();
+        static_cast<void>(::remove(impl_->socket_name_.c_str()));
+    }
+
+    // Stop all connections which can be closed. The only connection that won't
+    // be closed is the one over which we have received a request to reconfigure
+    // the server. This connection will be held until the CommandMgr responds to
+    // such request.
+    impl_->connection_pool_.stopAll();
+}
+
+int
+CommandMgr::getControlSocketFD() {
+    return (impl_->acceptor_ ? impl_->acceptor_->getNative() : -1);
+}
+
+
+CommandMgr&
+CommandMgr::instance() {
+    static CommandMgr cmd_mgr;
+    return (cmd_mgr);
+}
+
+void
+CommandMgr::setIOService(const IOServicePtr& io_service) {
+    impl_->io_service_ = io_service;
 }
 
 }; // end of isc::config

+ 34 - 48
src/lib/config/command_mgr.h

@@ -7,15 +7,33 @@
 #ifndef COMMAND_MGR_H
 #define COMMAND_MGR_H
 
+#include <asiolink/io_service.h>
 #include <cc/data.h>
 #include <config/hooked_command_mgr.h>
-#include <config/command_socket.h>
+#include <exceptions/exceptions.h>
 #include <boost/noncopyable.hpp>
-#include <list>
+#include <boost/shared_ptr.hpp>
 
 namespace isc {
 namespace config {
 
+/// @brief An exception indicating that specified socket parameters are invalid
+class BadSocketInfo : public Exception {
+public:
+    BadSocketInfo(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+/// @brief An exception indicating a problem with socket operation
+class SocketError : public Exception {
+public:
+    SocketError(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+
+class CommandMgrImpl;
+
 /// @brief Commands Manager implementation for the Kea servers.
 ///
 /// This class extends @ref BaseCommandMgr with the ability to receive and
@@ -29,72 +47,40 @@ public:
     /// @return the only existing instance of the manager
     static CommandMgr& instance();
 
+    /// @brief Sets IO service to be used by the command manager.
+    ///
+    /// The server should use this method to provide the Command Manager with the
+    /// common IO service used by the server.
+    /// @param io_service Pointer to the IO service.
+    void setIOService(const asiolink::IOServicePtr& io_service);
+
     /// @brief Opens control socket with parameters specified in socket_info
     ///
     /// Currently supported types are:
     /// - unix (required parameters: socket-type: unix, socket-name:/unix/path)
     ///
-    /// This method will close previously open command socket (if exists).
-    ///
-    /// @throw CommandSocketError if socket creation fails.
-    /// @throw SocketError if command socket is already open.
+    /// @throw BadSocketInfo When socket configuration is invalid.
+    /// @throw SocketError When socket operation fails.
     ///
-    /// @param socket_info describes control socket parameters
-    /// @return object representing a socket
-    CommandSocketPtr
+    /// @param socket_info Configuration information for the control socket.
+    void
     openCommandSocket(const isc::data::ConstElementPtr& socket_info);
 
     /// @brief Shuts down any open control sockets
     void closeCommandSocket();
 
-    /// @brief Reads data from a socket, parses as JSON command and processes it
-    ///
-    /// This method is used to handle traffic on connected socket. This callback
-    /// is installed by the @c isc::config::UnixCommandSocket::receiveHandler
-    /// (located in the src/lib/config/command_socket_factory.cc)
-    /// once the incoming connection is accepted. If end-of-file is detected, this
-    /// method will close the socket and will uninstall itself from
-    /// @ref isc::dhcp::IfaceMgr.
-    ///
-    /// @param sockfd socket descriptor of a connected socket
-    static void commandReader(int sockfd);
-
-    /// @brief Adds an information about opened connection socket
-    ///
-    /// @param conn Connection socket to be stored
-    void addConnection(const CommandSocketPtr& conn);
-
-    /// @brief Closes connection with a specific socket descriptor
-    ///
-    /// @param fd socket descriptor
-    /// @return true if closed successfully, false if not found
-    bool closeConnection(int fd);
-
     /// @brief Returns control socket descriptor
     ///
     /// This method should be used only in tests.
-    int getControlSocketFD() const {
-        return (socket_->getFD());
-    }
+    int getControlSocketFD();
 
 private:
 
     /// @brief Private constructor
-    ///
-    /// Registers internal 'list-commands' command.
     CommandMgr();
 
-    /// @brief Control socket structure
-    ///
-    /// This is the socket that accepts incoming connections. There can be at
-    /// most one (if command channel is configured).
-    CommandSocketPtr socket_;
-
-    /// @brief Sockets for open connections
-    ///
-    /// These are the sockets that are dedicated to handle a specific connection.
-    /// Their number is equal to number of current control connections.
-    std::list<CommandSocketPtr> connections_;
+    /// @brief Pointer to the implementation of the @ref CommandMgr.
+    boost::shared_ptr<CommandMgrImpl> impl_;
 };
 
 }; // end of isc::config namespace

+ 0 - 43
src/lib/config/command_socket.cc

@@ -1,43 +0,0 @@
-// Copyright (C) 2015 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
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#include <config/command_socket.h>
-#include <config/command_mgr.h>
-#include <config/config_log.h>
-#include <dhcp/iface_mgr.h>
-#include <boost/bind.hpp>
-#include <unistd.h>
-
-namespace isc {
-namespace config {
-
-ConnectionSocket::ConnectionSocket(int sockfd) {
-    sockfd_ = sockfd;
-
-    // Install commandReader callback. When there's any data incoming on this
-    // socket, commandReader will be called and process it. It may also
-    // eventually close this socket.
-    isc::dhcp::IfaceMgr::instance().addExternalSocket(sockfd,
-        boost::bind(&ConnectionSocket::receiveHandler, this));
-    }
-
-void ConnectionSocket::close() {
-    LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_CLOSED).arg(sockfd_);
-
-    // Unregister this callback
-    isc::dhcp::IfaceMgr::instance().deleteExternalSocket(sockfd_);
-
-    // We're closing a connection, not the whole socket. It's ok to just
-    // close the connection and don't delete anything.
-    ::close(sockfd_);
-}
-
-void ConnectionSocket::receiveHandler() {
-    CommandMgr::instance().commandReader(sockfd_);
-}
-
-};
-};

+ 0 - 105
src/lib/config/command_socket.h

@@ -1,105 +0,0 @@
-// Copyright (C) 2015 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
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#ifndef COMMAND_SOCKET_H
-#define COMMAND_SOCKET_H
-
-#include <cc/data.h>
-#include <unistd.h>
-
-namespace isc {
-namespace config {
-
-/// @brief An exception indicating that specified socket parameters are invalid
-class BadSocketInfo : public Exception {
-public:
-    BadSocketInfo(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) { };
-};
-
-/// @brief An exception indicating a problem with socket operation
-class SocketError : public Exception {
-public:
-    SocketError(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) { };
-};
-
-/// @brief Abstract base class that represents an open command socket
-///
-/// Derived classes are expected to handle specific socket types (e.g. UNIX
-/// or https).
-///
-/// For derived classes, see @ref UnixCommandSocket for a socket that
-/// accepts connections over UNIX socket and @ref ConnectionSocket that
-/// handles established connections (currently over UNIX sockets, but
-/// should be generic).
-class CommandSocket {
-public:
-    /// @brief Method used to handle incoming data
-    ///
-    /// This may be registered in @ref isc::dhcp::IfaceMgr
-    virtual void receiveHandler() = 0;
-
-    /// @brief General method for closing socket.
-    ///
-    /// This is the default implementation that simply closes
-    /// the socket. Derived classes may do additional steps
-    /// to terminate the connection.
-    virtual void close() {
-        ::close(sockfd_);
-    }
-
-    /// @brief Virtual destructor.
-    virtual ~CommandSocket() {
-        close();
-    }
-
-    /// @brief Returns socket descriptor.
-    int getFD() const {
-        return (sockfd_);
-    }
-
-protected:
-    /// Stores socket descriptor.
-    int sockfd_;
-};
-
-/// Pointer to a command socket object
-typedef boost::shared_ptr<CommandSocket> CommandSocketPtr;
-
-/// @brief This class represents a streaming socket for handling connections
-///
-/// Initially a socket (e.g. UNIX) is opened (represented by other classes, e.g.
-/// @ref UnixCommandSocket). Once incoming connection is detected, that class
-/// calls accept(), which returns a new socket dedicated to handling that
-/// specific connection. That socket is represented by this class.
-class ConnectionSocket : public CommandSocket {
-public:
-    /// @brief Default constructor
-    ///
-    /// This constructor is used in methods that call accept on existing
-    /// sockets. accept() returns a socket descriptor. Hence only one
-    /// parameter here.
-    ///
-    /// @param sockfd socket descriptor
-    ConnectionSocket(int sockfd);
-
-    /// @brief Method used to handle incoming data
-    ///
-    /// This method calls isc::config::CommandMgr::commandReader method.
-    virtual void receiveHandler();
-
-    /// @brief Closes socket.
-    ///
-    /// This method closes the socket, prints appropriate log message and
-    /// unregisters callback from @ref isc::dhcp::IfaceMgr.
-    virtual void close();
-};
-
-};
-};
-
-#endif

+ 0 - 226
src/lib/config/command_socket_factory.cc

@@ -1,226 +0,0 @@
-// Copyright (C) 2015 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
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#include <config/command_socket_factory.h>
-#include <config/config_log.h>
-#include <config/command_mgr.h>
-#include <dhcp/iface_mgr.h>
-#include <boost/bind.hpp>
-#include <sys/socket.h>
-#include <sys/un.h>
-#include <string.h>
-#include <errno.h>
-#include <cstdio>
-#include <fcntl.h>
-
-using namespace isc::data;
-
-namespace isc {
-namespace config {
-
-/// @brief Wrapper for UNIX stream sockets
-///
-/// There are two UNIX socket types: datagram-based (equivalent of UDP) and
-/// stream-based (equivalent of TCP). This class represents stream-based
-/// sockets. It opens up a unix-socket and waits for incoming connections.
-/// Once incoming connection is detected, accept() system call is called
-/// and a new socket for that particular connection is returned. A new
-/// object of @ref ConnectionSocket is created.
-class UnixCommandSocket : public CommandSocket {
-public:
-    /// @brief Default constructor
-    ///
-    /// Opens specified UNIX socket.
-    ///
-    /// @param filename socket filename
-    UnixCommandSocket(const std::string& filename)
-        : filename_(filename) {
-
-        // Create the socket and set it up.
-        sockfd_ = createUnixSocket(filename_);
-
-        // Install this socket in Interface Manager.
-        isc::dhcp::IfaceMgr::instance().addExternalSocket(sockfd_,
-            boost::bind(&UnixCommandSocket::receiveHandler, this));
-    }
-
-private:
-
-    /// @brief Auxiliary method for creating a UNIX socket
-    ///
-    /// @param file_name specifies socket file path
-    /// @return socket file descriptor
-    int createUnixSocket(const std::string& file_name) {
-
-        struct sockaddr_un addr;
-
-        // string.size() returns number of bytes (without trailing zero)
-        // we need 1 extra byte for terminating 0.
-        if (file_name.size() > sizeof(addr.sun_path) - 1) {
-            isc_throw(SocketError, "Failed to open socket: path specified ("
-                      << file_name << ") is longer (" << file_name.size()
-                      << " bytes) than allowed "
-                      << (sizeof(addr.sun_path) - 1) << " bytes.");
-        }
-
-        int fd = socket(AF_UNIX, SOCK_STREAM, 0);
-        if (fd == -1) {
-            isc_throw(isc::config::SocketError, "Failed to create AF_UNIX socket:"
-                      << strerror(errno));
-        }
-
-        // Let's remove the old file. We don't care about any possible
-        // errors here. The file should not be there if the file was
-        // shut down properly.
-        static_cast<void>(remove(file_name.c_str()));
-
-        // Set this socket to be closed-on-exec.
-        if (fcntl(fd, F_SETFD, FD_CLOEXEC) != 0) {
-            const char* errmsg = strerror(errno);
-            ::close(fd);
-            isc_throw(SocketError, "Failed to set close-on-exec on unix socket\
- "
-                      << fd << ": " << errmsg);
-        }
-
-        // Set this socket to be non-blocking one.
-        if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) {
-            const char* errmsg = strerror(errno);
-            ::close(fd);
-            isc_throw(SocketError, "Failed to set non-block mode on unix socket "
-                      << fd << ": " << errmsg);
-        }
-
-        // Now bind the socket to the specified path.
-        memset(&addr, 0, sizeof(addr));
-        addr.sun_family = AF_UNIX;
-        strncpy(addr.sun_path, file_name.c_str(), sizeof(addr.sun_path) - 1);
-        if (bind(fd, (struct sockaddr*)&addr, sizeof(addr))) {
-            const char* errmsg = strerror(errno);
-            ::close(fd);
-            static_cast<void>(remove(file_name.c_str()));
-            isc_throw(isc::config::SocketError, "Failed to bind socket " << fd
-                      << " to " << file_name << ": " << errmsg);
-        }
-
-        // One means that we allow at most 1 awaiting connections.
-        // Any additional attempts will get ECONNREFUSED error.
-        // That means that at any given time, there may be at most one controlling
-        // connection.
-        /// @todo: Make the number of parallel connections configurable.
-        int status = listen(fd, 1);
-        if (status < 0) {
-            const char* errmsg = strerror(errno);
-            ::close(fd);
-            static_cast<void>(remove(file_name.c_str()));
-            isc_throw(isc::config::SocketError, "Failed to listen on socket fd="
-                      << fd << ", filename=" << file_name << ": " << errmsg);
-        }
-
-        // Woohoo! Socket opened, let's log it!
-        LOG_INFO(command_logger, COMMAND_SOCKET_UNIX_OPEN).arg(fd).arg(file_name);
-
-        return (fd);
-    }
-
-    /// @public
-
-    /// @brief Connection acceptor, a callback used to accept incoming connections.
-    ///
-    /// This callback is used on a control socket. Once called, it will accept
-    /// incoming connection, create a new socket for it and create an instance
-    /// of ConnectionSocket, which will take care of the rest (i.e. install
-    /// appropriate callback for that new socket in @ref isc::dhcp::IfaceMgr).
-    void receiveHandler() {
-
-        // This method is specific to receiving data over UNIX socket, so using
-        // sockaddr_un instead of sockaddr_storage here is ok.
-        struct sockaddr_un client_addr;
-        socklen_t client_addr_len;
-        client_addr_len = sizeof(client_addr);
-
-        // Accept incoming connection. This will create a separate socket for
-        // handling this specific connection.
-        int fd2 = accept(sockfd_, reinterpret_cast<struct sockaddr*>(&client_addr),
-                         &client_addr_len);
-        if (fd2 == -1) {
-            LOG_ERROR(command_logger, COMMAND_SOCKET_ACCEPT_FAIL)
-                .arg(sockfd_).arg(strerror(errno));
-            return;
-        }
-
-        // And now create an object that represents that new connection.
-        CommandSocketPtr conn(new ConnectionSocket(fd2));
-
-        // Not sure if this is really needed, but let's set it to non-blocking
-        // mode.
-        if (fcntl(fd2, F_SETFL, O_NONBLOCK) != 0) {
-            // Failed to set socket to non-blocking mode.
-            LOG_ERROR(command_logger, COMMAND_SOCKET_FAIL_NONBLOCK)
-                .arg(fd2).arg(sockfd_).arg(strerror(errno));
-
-            conn.reset();
-            return;
-        }
-
-        // Remember this socket descriptor. It will be needed when we shut down
-        // the server.
-        CommandMgr::instance().addConnection(conn);
-
-        LOG_INFO(command_logger, COMMAND_SOCKET_CONNECTION_OPENED).arg(fd2)
-            .arg(sockfd_);
-    }
-
-    /// @private
-
-    // This method is called when we shutdown the connection.
-    void close() {
-        LOG_INFO(command_logger, COMMAND_SOCKET_UNIX_CLOSE).arg(sockfd_)
-            .arg(filename_);
-
-        isc::dhcp::IfaceMgr::instance().deleteExternalSocket(sockfd_);
-
-        // Close should always succeed. We don't care if we're able to delete
-        // the socket or not.
-        ::close(sockfd_);
-        static_cast<void>(remove(filename_.c_str()));
-    }
-
-    /// @brief UNIX filename representing this socket
-    std::string filename_;
-};
-
-CommandSocketPtr
-CommandSocketFactory::create(const isc::data::ConstElementPtr& socket_info) {
-    if(!socket_info) {
-        isc_throw(BadSocketInfo, "Missing socket_info parameters, can't create socket.");
-    }
-
-    ConstElementPtr type = socket_info->get("socket-type");
-    if (!type) {
-        isc_throw(BadSocketInfo, "Mandatory 'socket-type' parameter missing");
-    }
-
-    if (type->stringValue() == "unix") {
-        // UNIX socket is requested. It takes one parameter: socket-name that
-        // specifies UNIX path of the socket.
-        ConstElementPtr name = socket_info->get("socket-name");
-        if (!name) {
-            isc_throw(BadSocketInfo, "Mandatory 'socket-name' parameter missing");
-        }
-        if (name->getType() != Element::string) {
-            isc_throw(BadSocketInfo, "'socket-name' parameter expected to be a string");
-        }
-
-        return (CommandSocketPtr(new UnixCommandSocket(name->stringValue())));
-    } else {
-        isc_throw(BadSocketInfo, "Specified socket type ('" + type->stringValue()
-                  + "') is not supported.");
-    }
-}
-
-};
-};

+ 0 - 39
src/lib/config/command_socket_factory.h

@@ -1,39 +0,0 @@
-// Copyright (C) 2015 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
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#ifndef COMMAND_SOCKET_FACTORY_H
-#define COMMAND_SOCKET_FACTORY_H
-
-#include <cc/data.h>
-#include <config/command_socket.h>
-
-namespace isc {
-namespace config {
-
-/// A factory class for opening command socket
-///
-/// This class provides an interface for opening command socket.
-class CommandSocketFactory {
-public:
-
-    /// @brief Creates a socket specified by socket_info structure
-    ///
-    ///
-    /// Currently supported types are:
-    /// - unix
-    ///
-    /// See @ref CommandMgr::openCommandSocket for detailed description.
-    /// @throw CommandSocketError
-    ///
-    /// @param socket_info structure that describes the socket
-    /// @return socket descriptor
-    static CommandSocketPtr create(const isc::data::ConstElementPtr& socket_info);
-};
-
-};
-};
-
-#endif

+ 4 - 0
src/lib/config/config_messages.mes

@@ -55,6 +55,10 @@ This error indicates that the server detected incoming connection and executed
 accept system call on said socket, but this call returned an error. Additional
 information may be provided by the system as second parameter.
 
+% COMMAND_SOCKET_CLOSED_BY_FOREIGN_HOST Closed command socket %1 by foreign host
+This is an information message indicating that the command connection has been
+closed by a command control client.
+
 % COMMAND_SOCKET_CONNECTION_CLOSED Closed socket %1 for existing command connection
 This is an informational message that the socket created for handling
 client's connection is closed. This usually means that the client disconnected,

+ 0 - 1
src/lib/config/tests/Makefile.am

@@ -20,7 +20,6 @@ if HAVE_GTEST
 TESTS += run_unittests
 run_unittests_SOURCES = module_spec_unittests.cc
 run_unittests_SOURCES += client_connection_unittests.cc
-run_unittests_SOURCES += command_socket_factory_unittests.cc
 run_unittests_SOURCES += config_data_unittests.cc run_unittests.cc
 run_unittests_SOURCES += command_mgr_unittests.cc
 

+ 64 - 1
src/lib/config/tests/command_mgr_unittests.cc

@@ -6,6 +6,7 @@
 
 #include <gtest/gtest.h>
 
+#include <asiolink/io_service.h>
 #include <config/base_command_mgr.h>
 #include <config/command_mgr.h>
 #include <config/hooked_command_mgr.h>
@@ -16,6 +17,7 @@
 #include <string>
 #include <vector>
 
+using namespace isc::asiolink;
 using namespace isc::config;
 using namespace isc::data;
 using namespace isc::hooks;
@@ -26,7 +28,11 @@ class CommandMgrTest : public ::testing::Test {
 public:
 
     /// Default constructor
-    CommandMgrTest() {
+    CommandMgrTest()
+        : io_service_(new IOService()) {
+
+        CommandMgr::instance().setIOService(io_service_);
+
         handler_name = "";
         handler_params = ElementPtr();
         handler_called = false;
@@ -46,6 +52,20 @@ public:
                 "control_command_receive");
     }
 
+    /// @brief Returns socket path (using either hardcoded path or env variable)
+    /// @return path to the unix socket
+    std::string getSocketPath() {
+
+        std::string socket_path;
+        const char* env = getenv("KEA_SOCKET_TEST_DIR");
+        if (env) {
+            socket_path = std::string(env) + "/test-socket";
+        } else {
+            socket_path = std::string(TEST_DATA_BUILDDIR) + "/test-socket";
+        }
+        return (socket_path);
+    }
+
     /// @brief Resets indicators related to callout invocation.
     static void resetCalloutIndicators() {
         callout_name = "";
@@ -162,6 +182,9 @@ public:
         return (0);
     }
 
+    /// @brief IO service used by these tests.
+    IOServicePtr io_service_;
+
     /// @brief Name of the command (used in my_handler)
     static std::string handler_name;
 
@@ -490,3 +513,43 @@ TEST_F(CommandMgrTest, modifyCommandArgsInHook) {
     EXPECT_EQ("response", callout_argument_names[1]);
 
 }
+
+// This test verifies that a Unix socket can be opened properly and that input
+// parameters (socket-type and socket-name) are verified.
+TEST_F(CommandMgrTest, unixCreate) {
+    // Null pointer is obviously a bad idea.
+    EXPECT_THROW(CommandMgr::instance().openCommandSocket(ConstElementPtr()),
+                 isc::config::BadSocketInfo);
+
+    // So is passing no parameters.
+    ElementPtr socket_info = Element::createMap();
+    EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info),
+                 isc::config::BadSocketInfo);
+
+    // We don't support ipx sockets
+    socket_info->set("socket-type", Element::create("ipx"));
+    EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info),
+                 isc::config::BadSocketInfo);
+
+    socket_info->set("socket-type", Element::create("unix"));
+    EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info),
+                 isc::config::BadSocketInfo);
+
+    socket_info->set("socket-name", Element::create(getSocketPath()));
+    EXPECT_NO_THROW(CommandMgr::instance().openCommandSocket(socket_info));
+    EXPECT_GE(CommandMgr::instance().getControlSocketFD(), 0);
+
+    // It should be possible to close the socket.
+    EXPECT_NO_THROW(CommandMgr::instance().closeCommandSocket());
+}
+
+// This test checks that when unix path is too long, the socket cannot be opened.
+TEST_F(CommandMgrTest, unixCreateTooLong) {
+    ElementPtr socket_info = Element::fromJSON("{ \"socket-type\": \"unix\","
+        "\"socket-name\": \"/tmp/toolongtoolongtoolongtoolongtoolongtoolong"
+        "toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong"
+        "\" }");
+
+    EXPECT_THROW(CommandMgr::instance().openCommandSocket(socket_info),
+                 SocketError);
+}

+ 0 - 94
src/lib/config/tests/command_socket_factory_unittests.cc

@@ -1,94 +0,0 @@
-// Copyright (C) 2015 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
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#include <gtest/gtest.h>
-
-#include <cc/data.h>
-#include <config/command_mgr.h>
-#include <config/command_socket.h>
-#include <config/command_socket_factory.h>
-#include <cstdio>
-#include <cstdlib>
-
-using namespace isc::config;
-using namespace isc::data;
-
-// Test class for Command Manager
-class CommandSocketFactoryTest : public ::testing::Test {
-public:
-
-    /// Default constructor
-    CommandSocketFactoryTest()
-        :SOCKET_NAME(getSocketPath()) {
-
-        // Remove any stale socket files
-        static_cast<void>(remove(SOCKET_NAME.c_str()));
-    }
-
-    /// Default destructor
-    ~CommandSocketFactoryTest() {
-
-        // Remove any stale socket files
-        static_cast<void>(remove(SOCKET_NAME.c_str()));
-    }
-
-    /// @brief Returns socket path (using either hardcoded path or env variable)
-    /// @return path to the unix socket
-    std::string getSocketPath() {
-
-        std::string socket_path;
-        const char* env = getenv("KEA_SOCKET_TEST_DIR");
-        if (env) {
-            socket_path = std::string(env) + "/test-socket";
-        } else {
-            socket_path = std::string(TEST_DATA_BUILDDIR) + "/test-socket";
-        }
-        return (socket_path);
-    }
-
-    std::string SOCKET_NAME;
-};
-
-// This test verifies that a Unix socket can be opened properly and that input
-// parameters (socket-type and socket-name) are verified.
-TEST_F(CommandSocketFactoryTest, unixCreate) {
-    // Null pointer is obviously a bad idea.
-    EXPECT_THROW(CommandSocketFactory::create(ConstElementPtr()),
-                 isc::config::BadSocketInfo);
-
-    // So is passing no parameters.
-    ElementPtr socket_info = Element::createMap();
-    EXPECT_THROW(CommandSocketFactory::create(socket_info),
-                 isc::config::BadSocketInfo);
-
-    // We don't support ipx sockets
-    socket_info->set("socket-type", Element::create("ipx"));
-    EXPECT_THROW(CommandSocketFactory::create(socket_info),
-                 isc::config::BadSocketInfo);
-
-    socket_info->set("socket-type", Element::create("unix"));
-    EXPECT_THROW(CommandSocketFactory::create(socket_info),
-                 isc::config::BadSocketInfo);
-
-    socket_info->set("socket-name", Element::create(SOCKET_NAME));
-    CommandSocketPtr sock;
-    EXPECT_NO_THROW(sock = CommandSocketFactory::create(socket_info));
-    ASSERT_TRUE(sock);
-    EXPECT_NE(-1, sock->getFD());
-
-    // It should be possible to close the socket.
-    EXPECT_NO_THROW(sock->close());
-}
-
-// This test checks that when unix path is too long, the socket cannot be opened.
-TEST_F(CommandSocketFactoryTest, unixCreateTooLong) {
-    ElementPtr socket_info = Element::fromJSON("{ \"socket-type\": \"unix\","
-        "\"socket-name\": \"/tmp/toolongtoolongtoolongtoolongtoolongtoolong"
-        "toolongtoolongtoolongtoolongtoolongtoolongtoolongtoolongtoolong"
-        "\" }");
-
-    EXPECT_THROW(CommandSocketFactory::create(socket_info), SocketError);
-}

+ 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());
 }

+ 21 - 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,14 @@ 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));
+}
+
+// This test verifies that IO service specified for the TimerMgr
+// must not be null.
+TEST_F(TimerMgrTest, setIOService) {
+    EXPECT_THROW(timer_mgr_->setIOService(IOServicePtr()),
+                 BadValue);
 }
 
 } // end of anonymous namespace

+ 28 - 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,32 @@ 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) {
+    if (!io_service) {
+        isc_throw(BadValue, "IO service object must not be null for TimerMgr");
+    }
+    io_service_ = io_service;
 }
 
 void
@@ -327,29 +183,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 +197,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 +211,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 +270,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 +277,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 +299,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 +316,6 @@ TimerMgr::TimerMgr()
 }
 
 TimerMgr::~TimerMgr() {
-    stopThread();
     unregisterTimers();
     delete impl_;
 }
@@ -681,31 +379,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.
 ///
-/// 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.