From: Marcin Siodelski Date: Fri, 4 Sep 2015 09:01:48 +0000 (+0200) Subject: [3970] Add parameter to run pending callbacks when thread is stopped. X-Git-Tag: fd4o6_base~5^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=79d70c0695ce4beb03237ce91ea3cbcce0d07588;p=thirdparty%2Fkea.git [3970] Add parameter to run pending callbacks when thread is stopped. --- diff --git a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc index 66cfd154cd..8f7b00fcbe 100644 --- a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc +++ b/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 diff --git a/src/lib/dhcpsrv/timer_mgr.cc b/src/lib/dhcpsrv/timer_mgr.cc index 841dad282e..9fe5da1803 100644 --- a/src/lib/dhcpsrv/timer_mgr.cc +++ b/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 +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 diff --git a/src/lib/dhcpsrv/timer_mgr.h b/src/lib/dhcpsrv/timer_mgr.h index 26e3c5b76a..5465f892f2 100644 --- a/src/lib/dhcpsrv/timer_mgr.h +++ b/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 + void handleReadySocket(Iterator timer_info_iterator, const bool run_callback); + + //@} /// @brief Pointer to the io service. asiolink::IOServicePtr io_service_;