[master] Merge branch 'trac3971'

src/bin/dhcp4/ctrl_dhcp4_srv.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -123,8 +123,32 @@ 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);
 
+    // Start worker thread if there are any timers installed. Note that
+    // we also start worker thread when the reconfiguration failed, because
+    // in that case we continue using an old configuration and the server
+    // should still run installed timers.
+    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()));
+        }
+    }
+
     // Check that configuration was successful. If not, do not reopen sockets
     // and don't bother with DDNS stuff.
     try {
@@ -157,11 +181,12 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) {
     CfgMgr::instance().getStagingCfg()->getCfgIface()->
         openSockets(AF_INET, srv->getPort(), getInstance()->useBroadcast());
 
+
     return (answer);
 }
 
 ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t port /*= DHCP4_SERVER_PORT*/)
-    :Dhcpv4Srv(port) {
+    : Dhcpv4Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) {
     if (getInstance()) {
         isc_throw(InvalidOperation,
                   "There is another Dhcpv4Srv instance already.");
@@ -201,19 +226,29 @@ void ControlledDhcpv4Srv::shutdown() {
 }
 
 ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
-    cleanup();
-
-    // Close the command socket (if it exists).
-    CommandMgr::instance().closeCommandSocket();
-
-    // Deregister any registered commands
-    CommandMgr::instance().deregisterCommand("shutdown");
-    CommandMgr::instance().deregisterCommand("statistic-get");
-    CommandMgr::instance().deregisterCommand("statistic-reset");
-    CommandMgr::instance().deregisterCommand("statistic-remove");
-    CommandMgr::instance().deregisterCommand("statistic-get-all");
-    CommandMgr::instance().deregisterCommand("statistic-reset-all");
-    CommandMgr::instance().deregisterCommand("statistic-remove-all");
+    try {
+        cleanup();
+
+        // Stop worker thread running timers, if it is running.
+        timer_mgr_->stopThread();
+
+        // Close the command socket (if it exists).
+        CommandMgr::instance().closeCommandSocket();
+
+        // Deregister any registered commands
+        CommandMgr::instance().deregisterCommand("shutdown");
+        CommandMgr::instance().deregisterCommand("statistic-get");
+        CommandMgr::instance().deregisterCommand("statistic-reset");
+        CommandMgr::instance().deregisterCommand("statistic-remove");
+        CommandMgr::instance().deregisterCommand("statistic-get-all");
+        CommandMgr::instance().deregisterCommand("statistic-reset-all");
+        CommandMgr::instance().deregisterCommand("statistic-remove-all");
+
+    } catch (...) {
+        // Don't want to throw exceptions from the destructor. The server
+        // is shutting down anyway.
+        ;
+    }
 
     server_ = NULL; // forget this instance. Noone should call any handlers at
                     // this stage.

src/bin/dhcp4/ctrl_dhcp4_srv.h

@@ -18,6 +18,7 @@
 #include <asiolink/asiolink.h>
 #include <cc/data.h>
 #include <cc/command_interpreter.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <dhcp4/dhcp4_srv.h>
 
 namespace isc {
@@ -102,13 +103,7 @@ public:
     }
 
 
-protected:
-    /// @brief Static pointer to the sole instance of the DHCP server.
-    ///
-    /// This is required for config and command handlers to gain access to
-    /// the server
-    static ControlledDhcpv4Srv* server_;
-
+private:
     /// @brief Callback that will be called from iface_mgr when data
     /// is received over control socket.
     ///
@@ -117,9 +112,6 @@ protected:
     /// (that was sent from some yet unspecified sender).
     static void sessionReader(void);
 
-    /// @brief IOService object, used for all ASIO operations.
-    isc::asiolink::IOService io_service_;
-
     /// @brief Handler for processing 'shutdown' command
     ///
     /// This handler processes shutdown command, which initializes shutdown
@@ -157,6 +149,21 @@ protected:
     isc::data::ConstElementPtr
     commandConfigReloadHandler(const std::string& command,
                                isc::data::ConstElementPtr args);
+
+    /// @brief Static pointer to the sole instance of the DHCP server.
+    ///
+    /// This is required for config and command handlers to gain access to
+    /// the server
+    static ControlledDhcpv4Srv* server_;
+
+    /// @brief IOService object, used for all ASIO operations.
+    isc::asiolink::IOService io_service_;
+
+    /// @brief Instance of the @c TimerMgr.
+    ///
+    /// Shared pointer to the instance of timer @c TimerMgr is held here to
+    /// make sure that the @c TimerMgr outlives instance of this class.
+    TimerMgrPtr timer_mgr_;
 };
 
 }; // namespace isc::dhcp

src/bin/dhcp4/dhcp4_messages.mes

@@ -312,13 +312,6 @@ be sent to the client in the DHCPACK message. The first argument
 contains the client and the transaction identification information. The
 second argument contains the allocated IPv4 address.
 
-% DHCP4_LEASE_DATABASE_TIMERS_EXEC_FAIL failed to execute timer-based functions for lease database: %1
-A warning message executed when a server process is unable to execute
-the periodic actions for the lease database. An example of the periodic
-action is a Lease File Cleanup. One of the reasons for the failure is
-a misconfiguration of the lease database, whereby the lease database
-hasn't been selected.
-
 % DHCP4_NAME_GEN_UPDATE_FAIL %1: failed to update the lease after generating name %2 for a client: %3
 This message indicates the failure when trying to update the lease and/or
 options in the server's response with the hostname generated by the server

src/bin/dhcp4/dhcp4_srv.cc

@@ -365,16 +365,7 @@ Dhcpv4Srv::run() {
         Pkt4Ptr rsp;
 
         try {
-            // The lease database backend may install some timers for which
-            // the handlers need to be executed periodically. Retrieve the
-            // maximum interval at which the handlers must be executed from
-            // the lease manager.
-            uint32_t timeout = LeaseMgrFactory::instance().getIOServiceExecInterval();
-            // If the returned value is zero it means that there are no
-            // timers installed, so use a default value.
-            if (timeout == 0) {
-                timeout = 1000;
-            }
+            uint32_t timeout = 1000;
             LOG_DEBUG(packet4_logger, DBG_DHCP4_DETAIL, DHCP4_BUFFER_WAIT).arg(timeout);
             query = receivePacket(timeout);
 
@@ -429,15 +420,6 @@ Dhcpv4Srv::run() {
                 .arg(e.what());
         }
 
-        // Execute ready timers for the lease database, e.g. Lease File Cleanup.
-        try {
-            LeaseMgrFactory::instance().getIOService()->poll();
-
-        } catch (const std::exception& ex) {
-            LOG_WARN(dhcp4_logger, DHCP4_LEASE_DATABASE_TIMERS_EXEC_FAIL)
-                .arg(ex.what());
-        }
-
         // Timeout may be reached or signal received, which breaks select()
         // with no reception occurred. No need to log anything here because
         // we have logged right after the call to receivePacket().

src/bin/dhcp4/json_config_parser.cc

@@ -27,6 +27,7 @@
 #include <dhcpsrv/parsers/host_reservation_parser.h>
 #include <dhcpsrv/parsers/host_reservations_list_parser.h>
 #include <dhcpsrv/parsers/ifaces_config_parser.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <config/command_mgr.h>
 #include <util/encode/hex.h>
 #include <util/strutil.h>
@@ -461,6 +462,9 @@ configureDhcp4Server(Dhcpv4Srv&, isc::data::ConstElementPtr config_set) {
     // so newly recreated configuration starts with first subnet-id equal 1.
     Subnet::resetSubnetID();
 
+    // Remove any existing timers.
+    TimerMgr::instance()->unregisterTimers();
+
     // Some of the values specified in the configuration depend on
     // other values. Typically, the values in the subnet4 structure
     // depend on the global values. Also, option values configuration

src/bin/dhcp4/tests/dhcp4_process_tests.sh.in

@@ -16,6 +16,10 @@
 CFG_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test_config.json
 # Path to the Kea log file.
 LOG_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test.log
+# Path to the Kea lease file.
+LEASE_FILE=@abs_top_builddir@/src/bin/dhcp4/tests/test_leases.csv
+# Path to the Kea LFC application
+export KEA_LFC_EXECUTABLE=@abs_top_builddir@/src/bin/lfc/kea-lfc
 # Expected version
 EXPECTED_VERSION="@PACKAGE_VERSION@"
 # Kea configuration to be stored in the configuration file.
@@ -31,7 +35,9 @@ CONFIG="{
         \"lease-database\":
         {
             \"type\": \"memfile\",
-            \"persist\": false
+            \"name\": \"$LEASE_FILE\",
+            \"persist\": false,
+            \"lfc-interval\": 0
         },
         \"subnet4\": [
         {
@@ -276,9 +282,109 @@ shutdown_test() {
     test_finish 0
 }
 
+# This test verifies that DHCPv4 can be configured to run lease file cleanup
+# periodially.
+lfc_timer_test() {
+    # Log the start of the test and print test name.
+    test_start "dhcpv4_srv.lfc_timer_test"
+    # Remove dangling Kea instances and remove log files.
+    cleanup
+    # Create a configuration with the LFC enabled, by replacing the section
+    # with the lfc-interval and persist parameters.
+    LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 0/\"lfc-interval\": 1/g' \
+                        | sed -e 's/\"persist\": false/\"persist\": true/g')
+    # Create new configuration file.
+    create_config "${LFC_CONFIG}"
+    # Instruct Kea to log to the specific file.
+    set_logger
+    # Start Kea.
+    start_kea ${bin_path}/${bin}
+    # Wait up to 20s for Kea to start.
+    wait_for_kea 20
+    if [ ${_WAIT_FOR_KEA} -eq 0 ]; then
+        printf "ERROR: timeout waiting for Kea to start.\n"
+        clean_exit 1
+    fi
+
+    # Check if it is still running. It could have terminated (e.g. as a result
+    # of configuration failure).
+    get_pids ${bin}
+    if [ ${_GET_PIDS_NUM} -ne 1 ]; then
+        printf "ERROR: expected one Kea process to be started. Found %d processes\
+ started.\n" ${_GET_PIDS_NUM}
+        clean_exit 1
+    fi
+
+    # Check if Kea emits the log message indicating that LFC is started.
+    wait_for_message 10 "DHCPSRV_MEMFILE_LFC_EXECUTE" 1
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: Server did not execute LFC.\n"
+        clean_exit 1
+    fi
+
+    # Give it a short time to run.
+    sleep 1
+
+    # Modify the interval.
+    LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 1/\"lfc-interval\": 2/g')
+    # Create new configuration file.
+    create_config "${LFC_CONFIG}"
+
+    # Reconfigure the server with SIGHUP.
+    send_signal 1 ${bin}
+
+    # There should be two occurrences of the DHCP4_CONFIG_COMPLETE messages.
+    # Wait for it up to 10s.
+    wait_for_message 10 "DHCP4_CONFIG_COMPLETE" 2
+
+    # After receiving SIGHUP the server should get reconfigured and the
+    # reconfiguration should be noted in the log file. We should now
+    # have two configurations logged in the log file.
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: server hasn't been reconfigured.\n"
+        clean_exit 1
+    else
+        printf "Server successfully reconfigured.\n"
+    fi
+
+    # Make sure the server is still operational.
+    get_pids ${bin}
+    if [ ${_GET_PIDS_NUM} -ne 1 ]; then
+        printf "ERROR: Kea process was killed when attempting reconfiguration.\n"
+        clean_exit 1
+    fi
+
+    # Wait for the LFC to run the second time.
+    wait_for_message 10 "DHCPSRV_MEMFILE_LFC_EXECUTE" 2
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: Server did not execute LFC.\n"
+        clean_exit 1
+    fi
+
+    # Send signal to Kea SIGTERM
+    send_signal 15 ${bin}
+
+    # Wait up to 10s for the server's graceful shutdown. The graceful shut down
+    # should be recorded in the log file with the appropriate message.
+    wait_for_message 10 "DHCP4_SHUTDOWN" 1
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: Server did not record shutdown in the log.\n"
+        clean_exit 1
+    fi
+
+    # Make sure the server is down.
+    wait_for_server_down 5 ${bin}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
+
+    # All ok. Shut down Kea and exit.
+    test_finish 0
+}
+
 server_pid_file_test "${CONFIG}" DHCP4_ALREADY_RUNNING
 dynamic_reconfiguration_test
 shutdown_test "dhcpv4.sigterm_test" 15
 shutdown_test "dhcpv4.sigint_test" 2
 version_test "dhcpv4.version"
 logger_vars_test "dhcpv4.variables"
+lfc_timer_test

src/bin/dhcp6/ctrl_dhcp6_srv.cc

@@ -118,8 +118,34 @@ 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);
 
+    // Start worker thread if there are any timers installed. Note that
+    // we also start worker thread when the reconfiguration failed, because
+    // in that case we continue using an old configuration and the server
+    // should still run installed timers.
+    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()));
+        }
+    }
+
     // Check that configuration was successful. If not, do not reopen sockets
     // and don't bother with DDNS stuff.
     try {
@@ -156,7 +182,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) {
 }
 
 ControlledDhcpv6Srv::ControlledDhcpv6Srv(uint16_t port)
-    : Dhcpv6Srv(port) {
+    : Dhcpv6Srv(port), io_service_(), timer_mgr_(TimerMgr::instance()) {
     if (server_) {
         isc_throw(InvalidOperation,
                   "There is another Dhcpv6Srv instance already.");
@@ -196,19 +222,29 @@ void ControlledDhcpv6Srv::shutdown() {
 }
 
 ControlledDhcpv6Srv::~ControlledDhcpv6Srv() {
-    cleanup();
-
-   // Close the command socket (if it exists).
-    CommandMgr::instance().closeCommandSocket();
-
-    // Deregister any registered commands
-    CommandMgr::instance().deregisterCommand("shutdown");
-    CommandMgr::instance().deregisterCommand("statistic-get");
-    CommandMgr::instance().deregisterCommand("statistic-reset");
-    CommandMgr::instance().deregisterCommand("statistic-remove");
-    CommandMgr::instance().deregisterCommand("statistic-get-all");
-    CommandMgr::instance().deregisterCommand("statistic-reset-all");
-    CommandMgr::instance().deregisterCommand("statistic-remove-all");
+    try {
+        cleanup();
+
+        // Stop worker thread running timers, if it is running.
+        timer_mgr_->stopThread();
+
+        // Close the command socket (if it exists).
+        CommandMgr::instance().closeCommandSocket();
+
+        // Deregister any registered commands
+        CommandMgr::instance().deregisterCommand("shutdown");
+        CommandMgr::instance().deregisterCommand("statistic-get");
+        CommandMgr::instance().deregisterCommand("statistic-reset");
+        CommandMgr::instance().deregisterCommand("statistic-remove");
+        CommandMgr::instance().deregisterCommand("statistic-get-all");
+        CommandMgr::instance().deregisterCommand("statistic-reset-all");
+        CommandMgr::instance().deregisterCommand("statistic-remove-all");
+
+    } catch (...) {
+        // Don't want to throw exceptions from the destructor. The server
+        // is shutting down anyway.
+        ;
+    }
 
     server_ = NULL; // forget this instance. There should be no callback anymore
                     // at this stage anyway.

src/bin/dhcp6/ctrl_dhcp6_srv.h

@@ -18,6 +18,7 @@
 #include <asiolink/asiolink.h>
 #include <cc/data.h>
 #include <cc/command_interpreter.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <dhcp6/dhcp6_srv.h>
 
 namespace isc {
@@ -101,12 +102,7 @@ public:
         return (server_);
     }
 
-protected:
-    /// @brief Static pointer to the sole instance of the DHCP server.
-    ///
-    /// This is required for config and command handlers to gain access to
-    /// the server. Some of them need to be static methods.
-    static ControlledDhcpv6Srv* server_;
+private:
 
     /// @brief Callback that will be called from iface_mgr when data
     /// is received over control socket.
@@ -116,9 +112,6 @@ protected:
     /// (that was sent from some yet unspecified sender).
     static void sessionReader(void);
 
-    /// @brief IOService object, used for all ASIO operations.
-    isc::asiolink::IOService io_service_;
-
     /// @brief handler for processing 'shutdown' command
     ///
     /// This handler processes shutdown command, which initializes shutdown
@@ -156,6 +149,22 @@ protected:
     isc::data::ConstElementPtr
     commandConfigReloadHandler(const std::string& command,
                                isc::data::ConstElementPtr args);
+
+    /// @brief Static pointer to the sole instance of the DHCP server.
+    ///
+    /// This is required for config and command handlers to gain access to
+    /// the server. Some of them need to be static methods.
+    static ControlledDhcpv6Srv* server_;
+
+    /// @brief IOService object, used for all ASIO operations.
+    isc::asiolink::IOService io_service_;
+
+    /// @brief Instance of the @c TimerMgr.
+    ///
+    /// Shared pointer to the instance of timer @c TimerMgr is held here to
+    /// make sure that the @c TimerMgr outlives instance of this class.
+    TimerMgrPtr timer_mgr_;
+
 };
 
 }; // namespace isc::dhcp

src/bin/dhcp6/dhcp6_messages.mes

@@ -376,13 +376,6 @@ The first argument holds the client and the transaction identification
 information. The second argument holds the IAID. The third argument
 holds the detailed lease information.
 
-% DHCP6_LEASE_DATABASE_TIMERS_EXEC_FAIL failed to execute timer-based functions for lease database: %1
-A warning message executed when a server process is unable to execute
-the periodic actions for the lease database. An example of the periodic
-action is a Lease File Cleanup. One of the reasons for the failure is
-a misconfiguration of the lease database, whereby the lease database
-hasn't been selected.
-
 % DHCP6_LEASE_NA_WITHOUT_DUID %1: address lease for address %2 does not have a DUID
 This error message indicates a database consistency problem. The lease
 database has an entry indicating that the given address is in use,

src/bin/dhcp6/dhcp6_srv.cc

@@ -317,16 +317,7 @@ bool Dhcpv6Srv::run() {
         Pkt6Ptr rsp;
 
         try {
-            // The lease database backend may install some timers for which
-            // the handlers need to be executed periodically. Retrieve the
-            // maximum interval at which the handlers must be executed from
-            // the lease manager.
-            uint32_t timeout = LeaseMgrFactory::instance().getIOServiceExecInterval();
-            // If the returned value is zero it means that there are no
-            // timers installed, so use a default value.
-            if (timeout == 0) {
-                timeout = 1000;
-            }
+            uint32_t timeout = 1000;
             LOG_DEBUG(packet6_logger, DBG_DHCP6_DETAIL, DHCP6_BUFFER_WAIT).arg(timeout);
             query = receivePacket(timeout);
 
@@ -385,15 +376,6 @@ bool Dhcpv6Srv::run() {
                 .arg(e.what());
         }
 
-        // Execute ready timers for the lease database, e.g. Lease File Cleanup.
-        try {
-            LeaseMgrFactory::instance().getIOService()->poll();
-
-        } catch (const std::exception& ex) {
-            LOG_WARN(dhcp6_logger, DHCP6_LEASE_DATABASE_TIMERS_EXEC_FAIL)
-                .arg(ex.what());
-        }
-
         // Timeout may be reached or signal received, which breaks select()
         // with no packet received
         if (!query) {

src/bin/dhcp6/tests/dhcp6_process_tests.sh.in

@@ -16,6 +16,8 @@
 CFG_FILE=@abs_top_builddir@/src/bin/dhcp6/tests/test_config.json
 # Path to the Kea log file.
 LOG_FILE=@abs_top_builddir@/src/bin/dhcp6/tests/test.log
+# Path to the Kea lease file.
+LEASE_FILE=@abs_top_builddir@/src/bin/dhcp6/tests/test_leases.csv
 # Expected version
 EXPECTED_VERSION="@PACKAGE_VERSION@"
 # Kea configuration to be stored in the configuration file.
@@ -31,7 +33,9 @@ CONFIG="{
         \"lease-database\":
         {
             \"type\": \"memfile\",
-            \"persist\": false
+            \"name\": \"$LEASE_FILE\",
+            \"persist\": false,
+            \"lfc-interval\": 0
         },
         \"subnet6\": [
         {
@@ -277,9 +281,110 @@ returned %d."
     test_finish 0
 }
 
+# This test verifies that DHCPv6 can be configured to run lease file cleanup
+# periodially.
+lfc_timer_test() {
+    # Log the start of the test and print test name.
+    test_start "dhcpv6_srv.lfc_timer_test"
+    # Remove dangling Kea instances and remove log files.
+    cleanup
+    # Create a configuration with the LFC enabled, by replacing the section
+    # with the lfc-interval and persist parameters.
+    LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 0/\"lfc-interval\": 1/g' \
+                        | sed -e 's/\"persist\": false/\"persist\": true/g')
+    # Create new configuration file.
+    create_config "${LFC_CONFIG}"
+    # Instruct Kea to log to the specific file.
+    set_logger
+    # Start Kea.
+    start_kea ${bin_path}/${bin}
+    # Wait up to 20s for Kea to start.
+    wait_for_kea 20
+    if [ ${_WAIT_FOR_KEA} -eq 0 ]; then
+        printf "ERROR: timeout waiting for Kea to start.\n"
+        clean_exit 1
+    fi
+
+    # Check if it is still running. It could have terminated (e.g. as a result
+    # of configuration failure).
+    get_pids ${bin}
+    if [ ${_GET_PIDS_NUM} -ne 1 ]; then
+        printf "ERROR: expected one Kea process to be started. Found %d processes\
+ started.\n" ${_GET_PIDS_NUM}
+        clean_exit 1
+    fi
+
+    # Check if Kea emits the log message indicating that LFC is started.
+    wait_for_message 10 "DHCPSRV_MEMFILE_LFC_EXECUTE" 1
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: Server did not execute LFC.\n"
+        clean_exit 1
+    fi
+
+    # Give it a short time to run.
+    sleep 1
+
+    # Modify the interval.
+    LFC_CONFIG=$(printf "${CONFIG}" | sed -e 's/\"lfc-interval\": 1/\"lfc-interval\": 2/g')
+    # Create new configuration file.
+    create_config "${LFC_CONFIG}"
+
+    # Reconfigure the server with SIGHUP.
+    send_signal 1 ${bin}
+
+    # There should be two occurrences of the DHCP4_CONFIG_COMPLETE messages.
+    # Wait for it up to 10s.
+    wait_for_message 10 "DHCP6_CONFIG_COMPLETE" 2
+
+    # After receiving SIGHUP the server should get reconfigured and the
+    # reconfiguration should be noted in the log file. We should now
+    # have two configurations logged in the log file.
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: server hasn't been reconfigured.\n"
+        clean_exit 1
+    else
+        printf "Server successfully reconfigured.\n"
+    fi
+
+    # Make sure the server is still operational.
+    get_pids ${bin}
+    if [ ${_GET_PIDS_NUM} -ne 1 ]; then
+        printf "ERROR: Kea process was killed when attempting reconfiguration.\n"
+        clean_exit 1
+    fi
+
+    # Wait for the LFC to run the second time.
+    wait_for_message 10 "DHCPSRV_MEMFILE_LFC_EXECUTE" 2
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: Server did not execute LFC.\n"
+        clean_exit 1
+    fi
+
+    # Send signal to Kea SIGTERM
+    send_signal 15 ${bin}
+
+    # Wait up to 10s for the server's graceful shutdown. The graceful shut down
+    # should be recorded in the log file with the appropriate message.
+    wait_for_message 10 "DHCP6_SHUTDOWN" 1
+    if [ ${_WAIT_FOR_MESSAGE} -eq 0 ]; then
+        printf "ERROR: Server did not record shutdown in the log.\n"
+        clean_exit 1
+    fi
+
+    # Make sure the server is down.
+    wait_for_server_down 5 ${bin}
+    assert_eq 1 ${_WAIT_FOR_SERVER_DOWN} \
+        "Expected wait_for_server_down return %d, returned %d"
+
+    # All ok. Shut down Kea and exit.
+    test_finish 0
+}
+
 server_pid_file_test "${CONFIG}" DHCP6_ALREADY_RUNNING
 dynamic_reconfiguration_test
 shutdown_test "dhcpv6.sigterm_test" 15
 shutdown_test "dhcpv6.sigint_test" 2
 version_test "dhcpv6.version"
 logger_vars_test "dhcpv6.variables"
+lfc_timer_test
+

src/lib/dhcpsrv/dhcpsrv_messages.mes

@@ -346,6 +346,12 @@ failure persist however, the size of the lease file will increase without bound.
 An informational message issued when the Memfile lease database backend
 starts the periodic Lease File Cleanup.
 
+% DHCPSRV_MEMFILE_LFC_UNREGISTER_TIMER_FAILED failed to unregister timer 'memfile-lfc': %1
+This error message is logged when Memfile backend fails to unregister
+timer used for lease file cleanup scheduling. This is highly unlikely
+and indicates programming error. The message include the reason for this
+error.
+
 % DHCPSRV_MEMFILE_NO_STORAGE running in non-persistent mode, leases will be lost after restart
 A warning message issued when writes of leases to disk have been disabled
 in the configuration. This mode is useful for some kinds of performance

src/lib/dhcpsrv/lease_mgr.h

@@ -118,16 +118,6 @@ public:
 /// be used directly, but rather specialized derived class should be used
 /// instead.
 ///
-/// This class creates an instance of the @c asiolink::IOService in the
-/// constructor. This object is required to execute the
-/// @c asiolink::IntervalTimer for the operations triggered periodically
-/// by the lease database backends which implement @c LeaseMgr interface.
-/// In order to execute the timers installed by the particular backend,
-/// the owner of the backend (e.g. DHCP server) should retrieve the pointer
-/// to the @c asiolink::IOService object by calling @c LeaseMgr::getIOService
-/// and call the appropriate functions, e.g. @c poll_one or @c run_one in a
-/// main loop.
-///
 /// This class throws no exceptions.  However, methods in concrete
 /// implementations of this class may throw exceptions: see the documentation
 /// of those classes for details.
@@ -145,7 +135,7 @@ public:
     /// @param parameters A data structure relating keywords and values
     ///        concerned with the database.
     LeaseMgr(const ParameterMap& parameters)
-        : parameters_(parameters), io_service_(new asiolink::IOService())
+        : parameters_(parameters)
     {}
 
     /// @brief Destructor
@@ -439,31 +429,6 @@ public:
     /// @brief returns value of the parameter
     virtual std::string getParameter(const std::string& name) const;
 
-    /// @brief Returns the interval at which the @c IOService events should
-    /// be released.
-    ///
-    /// The implementations of this class may install the timers which
-    /// periodically trigger event handlers defined for them. Depending
-    /// on the intervals specified for these timers the @c IOService::poll,
-    /// @c IOService::run etc. have to be executed to allow the timers
-    /// for checking whether they have already expired and the handler
-    /// must be executed. Running the @c IOService with a lower interval
-    /// would cause the desynchronization of timers with the clock.
-    ///
-    /// @return A maximum interval in seconds at which the @c IOService
-    /// should be executed. A value of 0 means that no timers are installed
-    /// and that there is no requirement for the @c IOService to be
-    /// executed at any specific interval.
-    virtual uint32_t getIOServiceExecInterval() const {
-        return (0);
-    }
-
-    /// @brief Returns a reference to the @c IOService object used
-    /// by the Lease Manager.
-    const asiolink::IOServicePtr& getIOService() const {
-        return (io_service_);
-    }
-
 private:
     /// @brief list of parameters passed in dbconfig
     ///
@@ -471,10 +436,6 @@ private:
     /// password and other parameters required for DB access. It is not
     /// intended to keep any DHCP-related parameters.
     ParameterMap parameters_;
-
-    /// @brief Pointer to the IO service object used by the derived classes
-    /// to trigger interval timers.
-    asiolink::IOServicePtr io_service_;
 };
 
 }; // end of isc::dhcp namespace

src/lib/dhcpsrv/memfile_lease_mgr.cc

@@ -17,6 +17,7 @@
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <dhcpsrv/lease_file_loader.h>
 #include <dhcpsrv/memfile_lease_mgr.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <exceptions/exceptions.h>
 #include <util/pid_file.h>
 #include <util/process_spawn.h>
@@ -73,9 +74,12 @@ public:
     /// @c Memfile_LeaseMgr class.
     ///
     /// @param callback A pointer to the callback function.
-    /// @param io_service An io service used to create the interval timer.
-    LFCSetup(asiolink::IntervalTimer::Callback callback,
-             asiolink::IOService& io_service);
+    LFCSetup(asiolink::IntervalTimer::Callback callback);
+
+    /// @brief Destructor.
+    ///
+    /// Unregisters LFC timer.
+    ~LFCSetup();
 
     /// @brief Sets the new configuration for the %Lease File Cleanup.
     ///
@@ -92,11 +96,6 @@ public:
     /// @brief Spawns a new process.
     void execute();
 
-    /// @brief Returns interval at which the cleanup is performed.
-    ///
-    /// @return Interval in milliseconds.
-    long getInterval() const;
-
     /// @brief Checks if the lease file cleanup is in progress.
     ///
     /// @return true if the lease file cleanup is being executed.
@@ -107,9 +106,6 @@ public:
 
 private:
 
-    /// @brief Interval timer for LFC.
-    asiolink::IntervalTimer timer_;
-
     /// @brief A pointer to the @c ProcessSpawn object used to execute
     /// the LFC.
     boost::scoped_ptr<util::ProcessSpawn> process_;
@@ -119,11 +115,40 @@ private:
 
     /// @brief A PID of the last executed LFC process.
     pid_t pid_;
+
+    /// @brief Pointer to the timer manager.
+    ///
+    /// We have to hold this pointer here to make sure that the timer
+    /// manager is not destroyed before the lease manager.
+    TimerMgrPtr timer_mgr_;
 };
 
-LFCSetup::LFCSetup(asiolink::IntervalTimer::Callback callback,
-                   asiolink::IOService& io_service)
-    : timer_(io_service), process_(), callback_(callback), pid_(0) {
+LFCSetup::LFCSetup(asiolink::IntervalTimer::Callback callback)
+    : process_(), callback_(callback), pid_(0),
+      timer_mgr_(TimerMgr::instance()) {
+}
+
+LFCSetup::~LFCSetup() {
+    try {
+        // If we're here it means that either the process is terminating
+        // or we're reconfiguring the server. In the latter case the
+        // thread has been stopped probably, but we need to handle the
+        // former case so we call stopThread explicitly here.
+        timer_mgr_->stopThread();
+        // This may throw exception if the timer hasn't been registered
+        // but if the LFC Setup instance exists it means that the timer
+        // must have been registered or such registration have been
+        // attempted. The registration may fail if the duplicate timer
+        // exists or if the TimerMgr's worker thread is running but if
+        // this happens it is a programming error. In any case, we
+        // don't want exceptions being thrown from the destructor so
+        // we just log an error here.
+        timer_mgr_->unregisterTimer("memfile-lfc");
+
+    } catch (const std::exception& ex) {
+        LOG_ERROR(dhcpsrv_logger, DHCPSRV_MEMFILE_LFC_UNREGISTER_TIMER_FAILED)
+            .arg(ex.what());
+    }
 }
 
 void
@@ -143,13 +168,6 @@ LFCSetup::setup(const uint32_t lfc_interval,
             executable = c_executable;
         }
 
-        // Set the timer to call callback function periodically.
-        LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LFC_SETUP).arg(lfc_interval);
-        // Multiple the lfc_interval value by 1000 as this value specifies
-        // a timeout in seconds, whereas the setup() method expects the
-        // timeout in milliseconds.
-        timer_.setup(callback_, lfc_interval * 1000);
-
         // Start preparing the command line for kea-lfc.
 
         // Gather the base file name.
@@ -187,12 +205,17 @@ LFCSetup::setup(const uint32_t lfc_interval,
 
         // Create the process (do not start it yet).
         process_.reset(new util::ProcessSpawn(executable, args));
-    }
-}
 
-long
-LFCSetup::getInterval() const {
-    return (timer_.getInterval());
+        // Set the timer to call callback function periodically.
+        LOG_INFO(dhcpsrv_logger, DHCPSRV_MEMFILE_LFC_SETUP).arg(lfc_interval);
+
+        // Multiple the lfc_interval value by 1000 as this value specifies
+        // a timeout in seconds, whereas the setup() method expects the
+        // timeout in milliseconds.
+        timer_mgr_->registerTimer("memfile-lfc", callback_, lfc_interval * 1000,
+                                  asiolink::IntervalTimer::REPEATING);
+        timer_mgr_->setup("memfile-lfc");
+    }
 }
 
 void
@@ -227,9 +250,7 @@ const int Memfile_LeaseMgr::MAJOR_VERSION;
 const int Memfile_LeaseMgr::MINOR_VERSION;
 
 Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
-    : LeaseMgr(parameters),
-      lfc_setup_(new LFCSetup(boost::bind(&Memfile_LeaseMgr::lfcCallback, this),
-                              *getIOService()))
+    : LeaseMgr(parameters), lfc_setup_()
     {
     // Check the universe and use v4 file or v6 file.
     std::string universe = getParameter("universe");
@@ -255,7 +276,7 @@ Memfile_LeaseMgr::Memfile_LeaseMgr(const ParameterMap& parameters)
         LOG_WARN(dhcpsrv_logger, DHCPSRV_MEMFILE_NO_STORAGE);
 
     } else  {
-        lfcSetup();
+       lfcSetup();
     }
 }
 
@@ -785,12 +806,6 @@ Memfile_LeaseMgr::appendSuffix(const std::string& file_name,
     return (name);
 }
 
-
-uint32_t
-Memfile_LeaseMgr::getIOServiceExecInterval() const {
-    return (static_cast<uint32_t>(lfc_setup_->getInterval() / 1000));
-}
-
 std::string
 Memfile_LeaseMgr::getDefaultLeaseFilePath(Universe u) const {
     std::ostringstream s;
@@ -943,6 +958,7 @@ Memfile_LeaseMgr::lfcSetup() {
     }
 
     if (lfc_interval > 0) {
+        lfc_setup_.reset(new LFCSetup(boost::bind(&Memfile_LeaseMgr::lfcCallback, this)));
         lfc_setup_->setup(lfc_interval, lease_file4_, lease_file6_);
     }
 }

src/lib/dhcpsrv/memfile_lease_mgr.h

@@ -49,12 +49,7 @@ class LFCSetup;
 /// The backend installs an @c asiolink::IntervalTimer to periodically execute
 /// the @c Memfile_LeaseMgr::lfcCallback. This callback function controls
 /// the startup of the background process which removes redundant information
-/// from the lease file(s). Note that the @c asiolink::IntervalTimer uses
-/// @c asiolink::IOService to execute the callback. The @c LeaseMgr class
-/// creates this object, which can be obtained by the caller using the
-/// @c LeaseMgr::getIOService. The caller should later call an appropriate
-/// method, @c boost::asio::io_service::poll_one to execute the callback when
-/// the timer is ready.
+/// from the lease file(s).
 ///
 /// When the backend is starting up, it reads leases from the lease file (one
 /// by one) and adds them to the in-memory container as follows:
@@ -458,19 +453,6 @@ public:
     ///       about the state of the backend.
     //@{
 
-    /// @brief Returns the interval at which the @c IOService events should
-    /// be released.
-    ///
-    /// The Memfile backend may install a timer to execute the %Lease File
-    /// Cleanup periodically. If this timer is installed, the method returns
-    /// the LFC interval in milliseconds.
-    ///
-    /// @return A maximum interval (in seconds) at which the @c IOService
-    /// should be executed. A value of 0 means that no timers are installed
-    /// and that there is no requirement for the @c IOService to be
-    /// executed at any specific interval.
-    virtual uint32_t getIOServiceExecInterval() const;
-
     /// @brief Returns default path to the lease file.
     ///
     /// @param u Universe (V4 or V6).

src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc

@@ -14,16 +14,20 @@
 
 #include <config.h>
 
+#include <boost/asio.hpp>
 #include <asiolink/io_address.h>
 #include <dhcp/duid.h>
+#include <dhcp/iface_mgr.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <dhcpsrv/lease_mgr_factory.h>
 #include <dhcpsrv/memfile_lease_mgr.h>
+#include <dhcpsrv/timer_mgr.h>
 #include <dhcpsrv/tests/lease_file_io.h>
 #include <dhcpsrv/tests/test_utils.h>
 #include <dhcpsrv/tests/generic_lease_mgr_unittest.h>
 #include <util/pid_file.h>
+#include <util/stopwatch.h>
 #include <gtest/gtest.h>
 
 #include <boost/bind.hpp>
@@ -106,8 +110,7 @@ public:
     MemfileLeaseMgrTest() :
         io4_(getLeaseFilePath("leasefile4_0.csv")),
         io6_(getLeaseFilePath("leasefile6_0.csv")),
-        io_service_(),
-        fail_on_callback_(false) {
+        timer_mgr_(TimerMgr::instance()) {
 
         std::ostringstream s;
         s << KEA_LFC_BUILD_DIR << "/kea-lfc";
@@ -134,21 +137,10 @@ public:
     ///
     /// destroys lease manager backend.
     virtual ~MemfileLeaseMgrTest() {
-        // Explicitly destroy the timer and the IO service. Note that they
-        // must be destroyed in this order because test_timer_ depends on
-        // the io_service_ as it is passed its reference during the
-        // construction. It usually doesn't matter unless the timer is
-        // running (hasn't been cancelled). Destroying an underlying
-        // IO service before cancelling the timer relying on it may lead
-        // to a crash. This has been proven on CentOS 6, running boost
-        // version 1.41. Note that destroying the timer also cancels it.
-        // In theory, we could avoid this explicit release of these objects
-        // and rely on the order in which they are declared in the class.
-        // But, this seems to be better as it makes it more visible
-        // what we are doing here.
-        test_timer_.reset();
-        io_service_.reset();
-
+        // Stop TimerMgr worker thread if it is running.
+        timer_mgr_->stopThread();
+        // Make sure there are no timers registered.
+        timer_mgr_->unregisterTimers();
         LeaseMgrFactory::destroy();
         // Remove lease files and products of Lease File Cleanup.
         removeFiles(getLeaseFilePath("leasefile4_0.csv"));
@@ -219,18 +211,15 @@ public:
         lmptr_ = &(LeaseMgrFactory::instance());
     }
 
+    /// @brief Runs @c IfaceMgr::receive6 in a look for a specified time.
+    ///
+    /// @param ms Duration in milliseconds.
     void setTestTime(const uint32_t ms) {
-        IntervalTimer::Callback cb =
-            boost::bind(&MemfileLeaseMgrTest::testTimerCallback, this);
-        test_timer_.reset(new IntervalTimer(*io_service_));
-        test_timer_->setup(cb, ms, IntervalTimer::ONE_SHOT);
-    }
-
-    /// @brief Test timer callback function.
-    void testTimerCallback() {
-        io_service_->stop();
-        if (fail_on_callback_) {
-            FAIL() << "Test timeout reached";
+        // Measure test time and exit if timeout hit.
+        Stopwatch stopwatch;
+        while (stopwatch.getTotalMilliseconds() < ms) {
+            // Block for one 1 millisecond.
+            IfaceMgr::instance().receive6(0, 1000);
         }
     }
 
@@ -258,15 +247,8 @@ public:
     /// @brief Object providing access to v6 lease IO.
     LeaseFileIO io6_;
 
-    /// @brief IO service object used for the timer tests.
-    asiolink::IOServicePtr io_service_;
-
-    /// @brief Test timer for the test.
-    boost::shared_ptr<IntervalTimer> test_timer_;
-
-    /// @brief Indicates if the @c testTimerCallback should cause test failure.
-    bool fail_on_callback_;
-
+    /// @brief Pointer to the instance of the @c TimerMgr.
+    TimerMgrPtr timer_mgr_;
 };
 
 // This test checks if the LeaseMgr can be instantiated and that it
@@ -368,16 +350,14 @@ TEST_F(MemfileLeaseMgrTest, lfcTimer) {
     boost::scoped_ptr<LFCMemfileLeaseMgr>
         lease_mgr(new LFCMemfileLeaseMgr(pmap));
 
-    // Check that the interval is correct.
-    EXPECT_EQ(1, lease_mgr->getIOServiceExecInterval());
-
-    io_service_ = lease_mgr->getIOService();
+    // Start worker thread to execute LFC periodically.
+    ASSERT_NO_THROW(timer_mgr_->startThread());
 
     // Run the test for at most 2.9 seconds.
     setTestTime(2900);
 
-    // Run the IO service to execute timers.
-    io_service_->run();
+    // Stop worker thread.
+    ASSERT_NO_THROW(timer_mgr_->stopThread());
 
     // Within 2.9 we should record two LFC executions.
     EXPECT_EQ(2, lease_mgr->getLFCCount());
@@ -397,15 +377,16 @@ TEST_F(MemfileLeaseMgrTest, lfcTimerDisabled) {
     boost::scoped_ptr<LFCMemfileLeaseMgr>
         lease_mgr(new LFCMemfileLeaseMgr(pmap));
 
-    EXPECT_EQ(0, lease_mgr->getIOServiceExecInterval());
-
-    io_service_ = lease_mgr->getIOService();
+    // Start worker thread to execute LFC periodically.
+    ASSERT_NO_THROW(timer_mgr_->startThread());
 
     // Run the test for at most 1.9 seconds.
     setTestTime(1900);
 
-    // Run the IO service to execute timers.
-    io_service_->run();
+    // 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());
@@ -739,34 +720,6 @@ TEST_F(MemfileLeaseMgrTest, leaseFileCopy) {
     ASSERT_FALSE(input_file.exists());
 }
 
-
-// Test that the backend returns a correct value of the interval
-// at which the IOService must be executed to run the handlers
-// for the installed timers.
-TEST_F(MemfileLeaseMgrTest, getIOServiceExecInterval) {
-    LeaseMgr::ParameterMap pmap;
-    pmap["type"] = "memfile";
-    pmap["universe"] = "4";
-    pmap["name"] = getLeaseFilePath("leasefile4_0.csv");
-
-    // The lfc-interval is not set, so the returned value should be 0.
-    boost::scoped_ptr<LFCMemfileLeaseMgr> lease_mgr(new LFCMemfileLeaseMgr(pmap));
-    EXPECT_EQ(0, lease_mgr->getIOServiceExecInterval());
-
-    // lfc-interval = 10
-    pmap["lfc-interval"] = "10";
-    lease_mgr.reset();
-    lease_mgr.reset(new LFCMemfileLeaseMgr(pmap));
-    EXPECT_EQ(10, lease_mgr->getIOServiceExecInterval());
-
-    // lfc-interval = 20
-    pmap["lfc-interval"] = "20";
-    lease_mgr.reset();
-    lease_mgr.reset(new LFCMemfileLeaseMgr(pmap));
-    EXPECT_EQ(20, lease_mgr->getIOServiceExecInterval());
-
-}
-
 // Checks that adding/getting/deleting a Lease6 object works.
 TEST_F(MemfileLeaseMgrTest, addGetDelete6) {
     startBackend(V6);

src/lib/dhcpsrv/tests/timer_mgr_unittest.cc

@@ -208,6 +208,7 @@ TEST_F(TimerMgrTest, registerTimer) {
 TEST_F(TimerMgrTest, unregisterTimer) {
     // Register a timer and start it.
     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());
 
@@ -223,10 +224,13 @@ TEST_F(TimerMgrTest, unregisterTimer) {
 
     // Check that an attempt to unregister a non-existing timer would
     // result in exeception.
-    EXPECT_THROW(timer_mgr_->unregisterTimer("timer2"), BadValue);
+    ASSERT_THROW(timer_mgr_->unregisterTimer("timer2"), BadValue);
+    // Number of timers shouldn't have changed.
+    ASSERT_EQ(1, timer_mgr_->timersCount());
 
     // Now unregister the correct one.
     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());
@@ -239,11 +243,6 @@ TEST_F(TimerMgrTest, unregisterTimer) {
 }
 
 // This test verifies taht it is possible to unregister all timers.
-/// @todo This test is disabled because it may occassionally hang
-/// due to bug in the ASIO implementation shipped with Kea.
-/// Replacing it with the ASIO implementation from BOOST does
-/// solve the problem. See ticket #4009. Until this ticket is
-/// implemented, the test should remain disabled.
 TEST_F(TimerMgrTest, unregisterTimers) {
     // Register 10 timers.
     for (int i = 1; i <= 20; ++i) {
@@ -252,6 +251,8 @@ TEST_F(TimerMgrTest, unregisterTimers) {
         ASSERT_NO_FATAL_FAILURE(registerTimer(s.str(), 1))
             << "fatal failure occurred while registering "
             << s.str();
+        ASSERT_EQ(i, timer_mgr_->timersCount())
+            << "invalid number of registered timers returned";
         ASSERT_NO_THROW(timer_mgr_->setup(s.str()))
             << "exception thrown while calling setup() for the "
             << s.str();
@@ -278,6 +279,9 @@ TEST_F(TimerMgrTest, unregisterTimers) {
     // Let's unregister all timers.
     ASSERT_NO_THROW(timer_mgr_->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);

src/lib/dhcpsrv/timer_mgr.cc

@@ -168,6 +168,9 @@ public:
     /// process.
     void unregisterTimers();
 
+    /// @brief Returns the number of registered timers.
+    size_t timersCount() const;
+
     /// @brief Schedules the execution of the interval timer.
     ///
     /// This method schedules the timer, i.e. the callback will be executed
@@ -406,6 +409,11 @@ TimerMgrImpl::unregisterTimers() {
     }
 }
 
+size_t
+TimerMgrImpl::timersCount() const {
+    return (registered_timers_.size());
+}
+
 void
 TimerMgrImpl::setup(const std::string& timer_name) {
 
@@ -652,6 +660,11 @@ TimerMgr::unregisterTimers() {
     impl_->unregisterTimers();
 }
 
+size_t
+TimerMgr::timersCount() const {
+    return (impl_->timersCount());
+}
+
 void
 TimerMgr::setup(const std::string& timer_name) {
 

src/lib/dhcpsrv/timer_mgr.h

@@ -176,6 +176,9 @@ public:
     /// @brief Unregisters all timers.
     void unregisterTimers();
 
+    /// @brief Returns the number of registered timers.
+    size_t timersCount() const;
+
     /// @brief Schedules the execution of the interval timer.
     ///
     /// This method schedules the timer, i.e. the callback will be executed

src/lib/testutils/dhcp_test_lib.sh.in

@@ -247,6 +247,12 @@ cleanup() {
 
     # Remove temporary files.
     rm -rf ${LOG_FILE}
+    # Use asterisk to remove all files starting with the given name,
+    # in case the LFC has been run. LFC creates files with postfixes
+    # appended to the lease file name.
+    if [ ! -z "${LEASE_FILE}" ]; then
+        rm -rf ${LEASE_FILE}*
+    fi
     rm -rf ${CFG_FILE}
     rm -rf ${KEACTRL_CFG_FILE}
 }

src/lib/util/Makefile.am

@@ -10,6 +10,7 @@ lib_LTLIBRARIES = libkea-util.la
 libkea_util_la_SOURCES  = boost_time_utils.h boost_time_utils.cc
 libkea_util_la_SOURCES += csv_file.h csv_file.cc
 libkea_util_la_SOURCES += filename.h filename.cc
+libkea_util_la_SOURCES += fork_detector.h
 libkea_util_la_SOURCES += strutil.h strutil.cc
 libkea_util_la_SOURCES += buffer.h io_utilities.h
 libkea_util_la_SOURCES += time_utilities.h time_utilities.cc

src/lib/util/fork_detector.h

@@ -0,0 +1,62 @@
+// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef FORK_DETECTOR_H
+#define FORK_DETECTOR_H
+
+#include <sys/types.h>
+#include <unistd.h>
+
+namespace isc {
+namespace util {
+
+/// @brief Class which detects being child process.
+///
+/// This class detects if it is in the child process, by checking if the
+/// process PID matches the PID of the process which created instance of
+/// the class.
+///
+/// Detecting if we're in the child process is important when the application
+/// spawns new process using fork/exec. If exec step fails for any reason
+/// the child process exits. However, to exit gracefully the process may
+/// need to know if it is a child or parent and take a different path
+/// during destruction.
+class ForkDetector {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// Stores the PID of the process creating this instance.
+    ForkDetector()
+        : creator_pid_(getpid()) {
+    }
+
+    /// @brief Check if the process is a parent process;
+    ///
+    /// @return true if the process is a parent process.
+    bool isParent() const {
+        return (getpid() == creator_pid_);
+    }
+
+private:
+
+    /// @brief PID of the process which created instance of this class.
+    pid_t creator_pid_;
+
+};
+
+} // namespace isc::util
+} // namespace isc
+
+#endif // FORK_DETECTOR_H

src/lib/util/threads/tests/thread_unittest.cc

@@ -14,14 +14,18 @@
 
 #include <config.h>
 
+#include <util/process_spawn.h>
+#include <util/threads/sync.h>
 #include <util/threads/thread.h>
 #include <util/unittests/check_valgrind.h>
 
 #include <boost/bind.hpp>
+#include <boost/scoped_ptr.hpp>
 
 #include <gtest/gtest.h>
 
 #include <signal.h>
+#include <unistd.h>
 
 // This file tests the Thread class. It's hard to test an actual thread is
 // started, but we at least check the function is run and exceptions are
@@ -34,38 +38,166 @@
 // started in parallel (the other tests wait for the previous one to terminate
 // before starting new one).
 
+using namespace isc::util;
 using namespace isc::util::thread;
 
 namespace {
 const size_t iterations = 200;
 const size_t detached_iterations = 25;
 
-void
-doSomething(int*) { }
+/// @brief Implements a thread which can be stopped.
+///
+/// This class implements a worker thread which can be stopped by calling
+/// StoppableThread::stop. The call to this function blocks until the thread
+/// terminates. This class is useful for testing scenarios when the thread
+/// needs to be run for a specific amount of time.
+class StoppableThread {
+public:
+
+    /// @brief Constructor.
+    ///
+    /// It doesn't run the thread yet. It merely initializes required
+    /// class members.
+    StoppableThread()
+        : stopping_(false), mutex_(), thread_() {
+    }
+
+    /// @brief Destructor.
+    ///
+    /// Detaches the thread.
+    ~StoppableThread() {
+    }
+
+    /// @brief Starts the execution of the thread.
+    ///
+    /// This method will not start the thread if the thread has been stopped.
+    /// In order to start the thread again, @c StoppableThread::reset must be
+    /// called first.
+    void run() {
+        if (!amStopping()) {
+            thread_.reset(new Thread(boost::bind(&StoppableThread::loop, this)));
+        }
+    }
+
+    /// @brief Stops the thread as soon as possible.
+    void stop() {
+        if (!amStopping()) {
+            setStopping(true);
+            if (thread_) {
+                thread_->wait();
+                thread_.reset();
+            }
+        }
+    }
+
+    /// @brief Resets the thread state so as it can be ran again.
+    void reset() {
+        setStopping(false);
+    }
+
+private:
+
+    /// @brief Checks if the thread is being stopped.
+    ///
+    /// @return true if the thread is being stopped.
+    bool amStopping() {
+        Mutex::Locker lock(mutex_);
+        return (stopping_);
+    }
+
+    /// @brief Sets the stopping state.
+    ///
+    /// @param stopping New value for @c stopping_ state.
+    void setStopping(const bool stopping) {
+        Mutex::Locker lock(mutex_);
+        stopping_ = stopping;
+    }
+
+    /// @brief Worker thread loop.
+    ///
+    /// It runs until the @c StoppableThread::stop is called.
+    void loop() {
+        for (;;) {
+            if (amStopping()) {
+                break;
+            }
+            usleep(100);
+        }
+    }
+
+    /// @brief Flag indicating if the thread is being stopped.
+    bool stopping_;
+    /// @brief Mutex used for protecting @c stopping_ flag.
+    Mutex mutex_;
+    /// @brief Pointer to the thread instance.
+    boost::scoped_ptr<Thread> thread_;
+};
+
+/// @brief Static instance of the stoppable thread.
+boost::scoped_ptr<StoppableThread> thread;
+
+/// @brief Test fixture class for testing @c Thread.
+class ThreadTest : public ::testing::Test {
+public:
+
+    /// @brief Destructor.
+    ///
+    /// Stops the thread and resets the static pointer to
+    /// @c StoppableThread.
+    virtual ~ThreadTest() {
+        if (thread) {
+            thread->stop();
+        }
+        thread.reset();
+    }
+
+    /// @brief No-op method.
+    static void doSomething(int*) { }
+
+    /// @brief Marks specified boolean value as true to indicate that the
+    /// function has been run.
+    static void markRun(bool* mark) {
+        EXPECT_FALSE(*mark);
+        *mark = true;
+    }
+
+    /// @brief Throws 42.
+    static void throwSomething() {
+        throw 42; // Throw something really unusual, to see everything is caught.
+    }
+
+    /// @brief Throws standard exception.
+    static void throwException() {
+        throw std::exception();
+    }
+
+    /// @brief Returns signal mask set for a thread.
+    ///
+    /// @parm mask Pointer to signal mask set for the calling thread.
+    static void getSignalMask(sigset_t* mask) {
+        pthread_sigmask(SIG_SETMASK, 0, mask);
+    }
+};
+
+
 
 // We just test that we can forget about the thread and nothing
 // bad will happen on our side.
-TEST(ThreadTest, detached) {
+TEST_F(ThreadTest, detached) {
     if (!isc::util::unittests::runningOnValgrind()) {
         int x;
         for (size_t i = 0; i < detached_iterations; ++i) {
-            Thread thread(boost::bind(&doSomething, &x));
+            Thread thread(boost::bind(&ThreadTest::doSomething, &x));
         }
     }
 }
 
-void
-markRun(bool* mark) {
-    EXPECT_FALSE(*mark);
-    *mark = true;
-}
-
 // Wait for a thread to end first. The variable must be set at the time.
-TEST(ThreadTest, wait) {
+TEST_F(ThreadTest, wait) {
     if (!isc::util::unittests::runningOnValgrind()) {
         for (size_t i = 0; i < iterations; ++i) {
             bool mark = false;
-            Thread thread(boost::bind(markRun, &mark));
+            Thread thread(boost::bind(&ThreadTest::markRun, &mark));
             thread.wait();
             ASSERT_TRUE(mark) << "Not finished yet in " << i << "th iteration";
             // Can't wait second time
@@ -74,30 +206,20 @@ TEST(ThreadTest, wait) {
     }
 }
 
-void
-throwSomething() {
-    throw 42; // Throw something really unusual, to see everything is caught.
-}
-
-void
-throwException() {
-    throw std::exception();
-}
-
 // Exception in the thread we forget about should not do anything to us
-TEST(ThreadTest, detachedException) {
+TEST_F(ThreadTest, detachedException) {
     if (!isc::util::unittests::runningOnValgrind()) {
         for (size_t i = 0; i < detached_iterations; ++i) {
-            Thread thread(throwSomething);
+            Thread thread(&ThreadTest::throwSomething);
         }
         for (size_t i = 0; i < detached_iterations; ++i) {
-            Thread thread(throwException);
+            Thread thread(&ThreadTest::throwException);
         }
     }
 }
 
 // An uncaught exception in the thread should propagate through wait
-TEST(ThreadTest, exception) {
+TEST_F(ThreadTest, exception) {
     if (!isc::util::unittests::runningOnValgrind()) {
         for (size_t i = 0; i < iterations; ++i) {
             Thread thread(throwSomething);
@@ -108,21 +230,57 @@ TEST(ThreadTest, exception) {
     }
 }
 
-// Returns signal mask set for a thread.
-void
-getSignalMask(sigset_t* mask) {
-    pthread_sigmask(SIG_SETMASK, 0, mask);
-}
-
 // Verify that all signals are blocked.
-TEST(ThreadTest, sigmask) {
+TEST_F(ThreadTest, sigmask) {
     sigset_t mask;
     sigemptyset(&mask);
-    Thread thread(boost::bind(getSignalMask, &mask));
+    Thread thread(boost::bind(&ThreadTest::getSignalMask, &mask));
     ASSERT_NO_THROW(thread.wait());
     EXPECT_EQ(1, sigismember(&mask, SIGHUP));
     EXPECT_EQ(1, sigismember(&mask, SIGINT));
     EXPECT_EQ(1, sigismember(&mask, SIGTERM));
 }
 
+
+/// The @c ProcessSpawn class spawns new processes using the fork/exec
+/// scheme. If the call to exec fails, child process exits with the
+/// EXIT_FAILURE status code. The call to exit triggers destruction of
+/// all static objects, i.e. destructors of statically declared
+/// @c Thread objects are called in the child process. The call to
+/// fork doesn't clone threads existing in the main process. So, the
+/// @c Thread objects in the child process have invalid state, because
+/// they point to non-existing threads. As a result any attempts to
+/// detach or join the thread may result in errors or asserts.
+///
+/// This test verifies that the @c Thread class protects against such
+/// errors. It is supposed to detect that the @c Thread object is in
+/// the child process and not assert when the destruction fails.
+TEST_F(ThreadTest, spawnProcessWithThread) {
+    // Initialize and run the stoppable thread. Note that the 'thread'
+    // is a static variable, which will be 'cloned' into the child
+    // process. Its destructor will be called when the child process
+    // terminates with EXIT_FAILURE status. So in fact the destructor
+    // of the @c StoppableThread will be called twice: in the main
+    // process and in the child process.
+    thread.reset(new StoppableThread());
+    thread->run();
+
+    // Spawn the new process, using some non-existing executable. The
+    // current process will fork but the execvp should fail.
+    ProcessSpawn process_spawn("kea-dhcp4-a86570943h");
+    pid_t pid = process_spawn.spawn();
+    // Wait for the process to terminate.
+    while (process_spawn.isRunning(pid)) {
+        usleep(100);
+    }
+    // When the child process terminates it will call destructors of
+    // static objects. This means that it will call the destructor of
+    // the 'thread' object too. The 'thread' object in the child
+    // process points to non-existing thread, but we expect the
+    // graceful destruction, i.e. the destructor should not assert.
+    // If the destructor asserts the exit code returned here will
+    // be 0 - same as if we aborted.
+    EXPECT_EQ(EXIT_FAILURE, process_spawn.getExitStatus(pid));
+}
+
 }

src/lib/util/threads/thread.cc

@@ -12,6 +12,7 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <util/fork_detector.h>
 #include <util/threads/thread.h>
 #include <util/threads/sync.h>
 
@@ -74,7 +75,8 @@ public:
         // and the creating thread needs to release it.
         waiting_(2),
         main_(main),
-        exception_(false)
+        exception_(false),
+        fork_detector_()
     {}
     // Another of the waiting events is done. If there are no more, delete
     // impl.
@@ -125,6 +127,8 @@ public:
     Mutex mutex_;
     // Which thread are we talking about anyway?
     pthread_t tid_;
+    // Class to detect if we're in the child or parent process.
+    ForkDetector fork_detector_;
 };
 
 Thread::Thread(const boost::function<void ()>& main) :
@@ -148,8 +152,15 @@ Thread::Thread(const boost::function<void ()>& main) :
 
 Thread::~Thread() {
     if (impl_ != NULL) {
-        // In case we didn't call wait yet
-        const int result = pthread_detach(impl_->tid_);
+
+        int result = pthread_detach(impl_->tid_);
+        // If the error indicates that thread doesn't exist but we're
+        // in child process (after fork) it is expected. We should
+        // not cause an assert.
+        if (result == ESRCH && !impl_->fork_detector_.isParent()) {
+            result = 0;
+        }
+
         Impl::done(impl_);
         impl_ = NULL;
         // If the detach ever fails, something is screwed rather badly.
@@ -164,13 +175,20 @@ Thread::wait() {
                   "Wait called and no thread to wait for");
     }
 
+    // Was there an exception in the thread?
+    scoped_ptr<UncaughtException> ex;
+
     const int result = pthread_join(impl_->tid_, NULL);
     if (result != 0) {
-        isc_throw(isc::InvalidOperation, std::strerror(result));
+        // We will not throw exception if the error indicates that the
+        // thread doesn't exist and we are in the child process (forked).
+        // For the child process it is expected that the thread is not
+        // re-created when we fork.
+        if (result != ESRCH || impl_->fork_detector_.isParent()) {
+            isc_throw(isc::InvalidOperation, std::strerror(result));
+        }
     }
 
-    // Was there an exception in the thread?
-    scoped_ptr<UncaughtException> ex;
     // Something here could in theory throw. But we already terminated the thread, so
     // we need to make sure we are in consistent state even in such situation (like
     // releasing the mutex and impl_).