[3970] Add parameter to run pending callbacks when thread is stopped.

src/lib/dhcpsrv/tests/timer_mgr_unittest.cc

@@ -220,8 +220,8 @@ TEST_F(TimerMgrTest, unregisterTimer) {
     // Wait for the timer to execute several times.
     doWait(100);
 
-    // Stop the thread.
-    ASSERT_NO_THROW(timer_mgr.stopThread());
+    // 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"];
@@ -237,7 +237,7 @@ TEST_F(TimerMgrTest, unregisterTimer) {
     // Start the thread again and wait another 100ms.
     ASSERT_NO_THROW(timer_mgr.startThread());
     doWait(100);
-    ASSERT_NO_THROW(timer_mgr.stopThread());
+    ASSERT_NO_THROW(timer_mgr.stopThread(true));
 
     // The number of calls for the timer1 shouldn't change as the
     // timer had been unregistered.
@@ -268,7 +268,7 @@ TEST_F(TimerMgrTest, DISABLED_unregisterTimers) {
     // Start worker thread and wait for 500ms.
     ASSERT_NO_THROW(timer_mgr.startThread());
     doWait(500);
-    ASSERT_NO_THROW(timer_mgr.stopThread());
+    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();
@@ -289,7 +289,7 @@ TEST_F(TimerMgrTest, DISABLED_unregisterTimers) {
     // Start worker thread again and wait for 500ms.
     ASSERT_NO_THROW(timer_mgr.startThread());
     doWait(500);
-    ASSERT_NO_THROW(timer_mgr.stopThread());
+    ASSERT_NO_THROW(timer_mgr.stopThread(true));
 
     // The calls counter shouldn't change because there are
     // no timers registered.
@@ -381,7 +381,7 @@ TEST_F(TimerMgrTest, scheduleTimers) {
 
     // Stop the worker thread, which would halt the execution of
     // the timers.
-    timer_mgr.stopThread();
+    timer_mgr.stopThread(true);
 
     // We have been running the timer for 500ms at the interval of
     // 1 ms. The maximum number of callbacks is 500. However, the

src/lib/dhcpsrv/timer_mgr.cc

@@ -212,7 +212,7 @@ TimerMgr::startThread() {
 }
 
 void
-TimerMgr::stopThread() {
+TimerMgr::stopThread(const bool run_pending_callbacks) {
     // If thread is not running, this is no-op.
     if (thread_) {
         // Only log it if we really have something to stop.
@@ -222,11 +222,12 @@ TimerMgr::stopThread() {
         // Stop the IO Service. This will break the IOService::run executed in the
         // worker thread. The thread will now terminate.
         getIOService().stop();
-        // When the worker thread may be waiting on the call to
-        // WatchSocket::markReady until main thread clears the socket.
-        // To unblock the thread we have to clear all sockets to make
-        // sure that the thread doesn't remain blocked.
-        clearReadySockets();
+        // 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.
@@ -241,7 +242,6 @@ TimerMgr::getIOService() const {
     return (*io_service_);
 }
 
-
 void
 TimerMgr::timerCallback(const std::string& timer_name) {
     // Find the specified timer setup.
@@ -260,7 +260,6 @@ TimerMgr::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()) {
-        const TimerInfoPtr& timer_info = timer_info_it->second;
         // 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
@@ -268,25 +267,34 @@ TimerMgr::ifaceMgrCallback(const std::string& timer_name) {
         // ready. Then execute the callback function supplied by the
         // TimerMgr user to perform custom actions on the expiration of
         // the given timer.
-        timer_info->watch_socket_.clearReady();
-
-        // 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_name);
-        timer_info->user_callback_();
+        handleReadySocket(timer_info_it, true);
     }
 }
 
 void
-TimerMgr::clearReadySockets() {
+TimerMgr::clearReadySockets(const bool run_pending_callbacks) {
     for (TimerInfoMap::iterator timer_info_it = registered_timers_.begin();
          timer_info_it != registered_timers_.end(); ++timer_info_it) {
-        timer_info_it->second->watch_socket_.clearReady();
+        handleReadySocket(timer_info_it, run_pending_callbacks);
    }
 }
 
+template<typename Iterator>
+void
+TimerMgr::handleReadySocket(Iterator timer_info_iterator,
+                            const bool run_callback) {
+    timer_info_iterator->second->watch_socket_.clearReady();
+
+    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);
+        timer_info_iterator->second->user_callback_();
+    }
+}
+
 } // end of namespace isc::dhcp
 } // end of namespace isc

src/lib/dhcpsrv/timer_mgr.h

@@ -180,8 +180,34 @@ public:
 
     /// @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.
-    void stopThread();
+    ///
+    /// @param run_pending_callbacks Indicates if the pending callbacks
+    /// should be executed (if true).
+    void stopThread(const bool run_pending_callbacks = false);
 
     //@}
 
@@ -235,7 +261,37 @@ private:
 
     //@}
 
-    void clearReadySockets();
+    /// @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 Pointer to the io service.
     asiolink::IOServicePtr io_service_;