From ab8afd6cfc1ae854dc1343a2b0cd6a20a9a8b107 Mon Sep 17 00:00:00 2001 From: Marcin Siodelski Date: Thu, 27 Aug 2015 18:50:12 +0200 Subject: [PATCH] [3970] Updated implementation of TimerMgr and make it almost ready. The TimerMgr now uses the IfaceMgr to register WatchSockets. It also comes with a full doxygen documentation and extensive commentary in the code. --- src/lib/dhcpsrv/tests/timer_mgr_unittest.cc | 78 ++++++- src/lib/dhcpsrv/timer_mgr.cc | 161 +++++++++++++- src/lib/dhcpsrv/timer_mgr.h | 222 ++++++++++++++++++-- src/lib/util/watch_socket.h | 4 +- 4 files changed, 425 insertions(+), 40 deletions(-) diff --git a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc index d89575f3d2..1d9600e572 100644 --- a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc @@ -14,7 +14,9 @@ #include #include +#include #include +#include #include #include @@ -36,6 +38,8 @@ private: /// @brief IO service used by the test fixture class. IOService io_service_; + bool timeout_; + public: /// @brief Wrapper method for registering a new timer. @@ -64,6 +68,8 @@ public: /// be increased. void timerCallback(const std::string& timer_name); + boost::function makeCallback(const std::string& timer_name); + /// @brief Callback for timeout. void timeoutCallback(); @@ -78,12 +84,16 @@ public: void TimerMgrTest::SetUp() { calls_count_.clear(); -// TimerMgr::instance().startThread(); + timeout_ = false; + // Make sure there are no dangling threads. + TimerMgr::instance().stopThread(); } void TimerMgrTest::TearDown() { -// TimerMgr::instance().stopThread(); + // Make sure there are no dangling threads. + TimerMgr::instance().stopThread(); + TimerMgr::instance().deregisterTimers(); } void @@ -92,9 +102,8 @@ TimerMgrTest::registerTimer(const std::string& timer_name, const long timer_inte TimerMgr& timer_mgr = TimerMgr::instance(); ASSERT_NO_THROW( - timer_mgr.registerTimer(timer_name, boost::bind(&TimerMgrTest::timerCallback, - this, timer_name), - timer_interval, timer_mode) + timer_mgr.registerTimer(timer_name, makeCallback(timer_name), timer_interval, + timer_mode) ); calls_count_[timer_name] = 0; @@ -107,7 +116,12 @@ TimerMgrTest::doWait(const long timeout) { timeout_timer.setup(boost::bind(&TimerMgrTest::timeoutCallback, this), timeout, IntervalTimer::ONE_SHOT); - io_service_.run_one(); + while (!timeout_) { + IfaceMgr::instance().receive6(0, timeout * 1000); + io_service_.get_io_service().poll_one(); + } + + timeout_ = false; } void @@ -118,24 +132,70 @@ TimerMgrTest::timerCallback(const std::string& timer_name) { TimerMgr::instance().setup(timer_name); } +boost::function +TimerMgrTest::makeCallback(const std::string& timer_name) { + return (boost::bind(&TimerMgrTest::timerCallback, this, timer_name)); +} + + void TimerMgrTest::timeoutCallback() { io_service_.stop(); + io_service_.get_io_service().reset(); + timeout_ = true; } TEST_F(TimerMgrTest, registerTimer) { TimerMgr& timer_mgr = TimerMgr::instance(); + // Empty timer name is not allowed. + ASSERT_THROW(timer_mgr.registerTimer("", makeCallback("timer1"), 1, + IntervalTimer::ONE_SHOT), + BadValue); + + // Add a timer with a correct name. + ASSERT_NO_THROW(timer_mgr.registerTimer("timer2", makeCallback("timer2"), 1, + IntervalTimer::ONE_SHOT)); + // Adding the timer with the same name as the existing timer is not + // allowed. + ASSERT_THROW(timer_mgr.registerTimer("timer2", makeCallback("timer2"), 1, + IntervalTimer::ONE_SHOT), + BadValue); + + // Start worker thread. + ASSERT_NO_THROW(timer_mgr.startThread()); +} + +TEST_F(TimerMgrTest, scheduleTimers) { + TimerMgr& timer_mgr = TimerMgr::instance(); + ASSERT_NO_FATAL_FAILURE(registerTimer("timer1", 1)); + ASSERT_NO_FATAL_FAILURE(registerTimer("timer2", 5)); - TimerMgr::instance().startThread(); - TimerMgr::instance().setup("timer1"); + timer_mgr.startThread(); + timer_mgr.setup("timer1"); + timer_mgr.setup("timer2"); doWait(500); - TimerMgr::instance().stopThread(); + timer_mgr.stopThread(); EXPECT_GT(calls_count_["timer1"], 1); + EXPECT_GT(calls_count_["timer2"], 5); + + EXPECT_GT(calls_count_["timer1"], calls_count_["timer2"]); + + ASSERT_NO_THROW(timer_mgr.deregisterTimer("timer1")); + + unsigned int calls_count_timer1 = calls_count_["timer1"]; + unsigned int calls_count_timer2 = calls_count_["timer2"]; + + timer_mgr.startThread(); + + doWait(500); + + EXPECT_EQ(calls_count_timer1, calls_count_["timer1"]); + EXPECT_GT(calls_count_["timer2"], calls_count_timer2); } } // end of anonymous namespace diff --git a/src/lib/dhcpsrv/timer_mgr.cc b/src/lib/dhcpsrv/timer_mgr.cc index 447c72d545..e4db08203a 100644 --- a/src/lib/dhcpsrv/timer_mgr.cc +++ b/src/lib/dhcpsrv/timer_mgr.cc @@ -13,10 +13,12 @@ // PERFORMANCE OF THIS SOFTWARE. #include +#include +#include #include #include #include -#include +#include using namespace isc; using namespace isc::asiolink; @@ -33,7 +35,15 @@ TimerMgr::instance() { } TimerMgr::TimerMgr() - : io_service_(new IOService()), thread_() { + : thread_(), registered_timers_() { +} + +TimerMgr::~TimerMgr() { + // Stop the thread, but do not deregister any timers. Deregistering + // the timers could cause static deinitialization fiasco between the + // TimerMgr and IfaceMgr. By now, the caller should have deregistered + // the timers. + stopThread(); } void @@ -42,62 +52,191 @@ TimerMgr::registerTimer(const std::string& timer_name, const long interval, const IntervalTimer::Mode& scheduling_mode) { + // Timer name must not be empty. if (timer_name.empty()) { isc_throw(BadValue, "registered timer name must not be empty"); } + // Must not register two timers under the same name. if (registered_timers_.find(timer_name) != registered_timers_.end()) { isc_throw(BadValue, "trying to register duplicate timer '" << 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"); } - IntervalTimerPtr interval_timer(new IntervalTimer(*io_service_)); + // 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. + TimerInfo timer_info(getIOService(), callback, interval, scheduling_mode); - WatchSocket watch_socket; - TimerInfo timer_info(watch_socket, interval_timer, 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. + IfaceMgr::instance().addExternalSocket(timer_info.watch_socket_->getSelectFd(), + boost::bind(&TimerMgr::ifaceMgrCallback, + this, timer_name)); + + // Actually register the timer. registered_timers_.insert(std::pair(timer_name, timer_info)); } void TimerMgr::deregisterTimer(const std::string& timer_name) { + // Find the timer with specified name. + TimerInfoMap::iterator timer_info_it = registered_timers_.find(timer_name); + + // Check if the timer has been registered. + if (timer_info_it == registered_timers_.end()) { + isc_throw(BadValue, "unable to deregister non existing timer '" + << timer_name << "'"); + } + + TimerInfo& timer_info = timer_info_it->second; + + // Cancel any pending asynchronous operation and stop the timer. + timer_info.interval_timer_->cancel(); + + // Unregister the watch socket from the IfaceMgr. + IfaceMgr::instance().deleteExternalSocket(timer_info.watch_socket_->getSelectFd()); + + // Remove the timer. + registered_timers_.erase(timer_info_it); +} + +void +TimerMgr::deregisterTimers() { + // Copy the map holding timers configuration. This is required so as + // we don't cut the branch which we're sitting on when we will be + // erasing the timers. We're going to iterate over the register timers + // and remove them with the call to deregisterTimer function. But this + // function will remove them from the register_timers_ map. If we + // didn't work on the copy here, our iterator would invalidate. The + // TimerInfo structure is copyable and since it is using the shared + // pointers the copy is not expensive. Also this function is called when + // the process terminates so it is not critical for performance. + TimerInfoMap registered_timers_copy(registered_timers_); + + // Iterate over the existing timers and deregister them. + for (TimerInfoMap::iterator timer_info_it = registered_timers_copy.begin(); + timer_info_it != registered_timers_copy.end(); ++timer_info_it) { + deregisterTimer(timer_info_it->first); + } } void TimerMgr::setup(const std::string& timer_name) { + // Check if the specified timer exists. TimerInfoMap::const_iterator timer_info_it = registered_timers_.find(timer_name); if (timer_info_it == registered_timers_.end()) { isc_throw(BadValue, "unable to setup timer '" << timer_name << "': " "no such timer registered"); } + // Schedule the execution of the timer using the parameters supplied + // during the registration. const TimerInfo& timer_info = timer_info_it->second; - timer_info.interval_timer_->setup(timer_info.callback_, timer_info.interval_, + timer_info.interval_timer_->setup(boost::bind(&TimerMgr::timerCallback, this, timer_name), + timer_info.interval_, timer_info.scheduling_mode_); } +void +TimerMgr::cancel(const std::string& timer_name) { + // Find the timer of our interest. + TimerInfoMap::const_iterator timer_info_it = registered_timers_.find(timer_name); + if (timer_info_it == registered_timers_.end()) { + isc_throw(BadValue, "unable to cancel timer '" << timer_name << "': " + "no such timer registered"); + } + // Cancel the timer. + timer_info_it->second.interval_timer_->cancel(); +} + void TimerMgr::startThread() { - thread_.reset(new Thread(boost::bind(&IOService::run, &getIOService()))); + // Do not start the thread if the thread is already there. + if (!thread_) { + // The thread will simply run IOService::run(), which is a blocking call + // to keep running handlers for all timers according to how they have + // been scheduled. + thread_.reset(new Thread(boost::bind(&IOService::run, &getIOService()))); + } } void TimerMgr::stopThread() { + // If thread is not running, this is no-op. if (thread_) { - io_service_->post(boost::bind(&IOService::stop, &getIOService())); + // Stop the IO Service. This will break the IOService::run executed in the + // worker thread. The thread will now terminate. + getIOService().post(boost::bind(&IOService::stop, &getIOService())); + // 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_->get_io_service().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(); } } +IOService& +TimerMgr::getIOService() const { + // The IO service is now created internally by the TimerMgr and we don't allow + // overriding it with anything as there are currently no use cases for it. + // It is possible that someone may want to obtain this instance to use it + // for something else too, so we return a reference to a static object. + // If we decide to allow setting the IO service object we will have to + // replace this static object with a shared pointer allocated in the + // class constructor. + static asiolink::IOService io_service; + return (io_service); +} + + +void +TimerMgr::timerCallback(const std::string& timer_name) { + // Run callback. Value of true says "mark socket ready". + watchSocketCallback(timer_name, true); +} void -TimerMgr::localCallback(const std::string& timer_name) const { +TimerMgr::ifaceMgrCallback(const std::string& timer_name) { + // Run callback. Value of false says "clear ready socket". + watchSocketCallback(timer_name, false); +} + +void +TimerMgr::watchSocketCallback(const std::string& timer_name, const bool mark_ready) { + // Find the specified timer setup. + TimerInfoMap::iterator timer_info_it = registered_timers_.find(timer_name); + if (timer_info_it != registered_timers_.end()) { + TimerInfo& timer_info = timer_info_it->second; + // This is 'true' when we're executing a callback for the elapsed timer. + // It is supposed to mark watch socket ready - write data to a socket to + // interrupt the execution of the select() function. This is executed + // from the worker thrad and will likely block the thread until the socket + // is cleared. + if (mark_ready) { + timer_info.watch_socket_->markReady(); + + } else { + // 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. + timer_info.watch_socket_->clearReady(); + timer_info.user_callback_(); + } + } } } // end of namespace isc::dhcp diff --git a/src/lib/dhcpsrv/timer_mgr.h b/src/lib/dhcpsrv/timer_mgr.h index 7f3c0c572e..624468a5b9 100644 --- a/src/lib/dhcpsrv/timer_mgr.h +++ b/src/lib/dhcpsrv/timer_mgr.h @@ -27,30 +27,163 @@ namespace isc { namespace dhcp { +/// @brief Manages a pool of asynchronous interval timers for DHCP server. +/// +/// This class holds a pool of asynchronous interval timers which are +/// capable of interrupting the blocking call to @c select() function in +/// the main threads of the DHCP servers. The main thread can then +/// timely execute the callback function associated with the particular +/// timer. +/// +/// This class is useful for performing periodic actions at the specified +/// intervals, e.g. act upon expired leases (leases reclamation) or +/// return declined leases back to the address pool. Other applications +/// may be added in the future. +/// +/// The @c TimerMgr is a singleton, thus its instance is available from +/// different places in the server code. This is convenient because timers +/// can be installed by different configuration parsers. +/// +/// The timer is registered using the @c TimerMgr::registerTimer method. +/// Each registered timer has a unique name. It is not possible to register +/// multiple timers with the same name. Each registered timer is associated +/// with the callback function supplied by the caller. This callback function +/// performs the tasks to be executed periodically according to the timer's +/// interval. +/// +/// The registered timer is not executed 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 +/// @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. Note that this +/// operation will likely block the worker thread until the +/// socket is cleared. 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. +/// +/// @warning The application (DHCP server) is responsible for +/// deregistering the timers before it terminates: +/// @code +/// TimerMgr::instance().deregisterTimers(); +/// @endcode +/// +/// to avoid the static deinitialization fiasco between the @c TimerMgr +/// and @c IfaceMgr. Note that the @c TimerMgr destructor doesn't +/// deregister the timers to avoid referencing the @c IfaceMgr +/// instance which may not exist at this point. If the timers are +/// not deregistered before the application terminates this will +/// likely result in segmentation fault on some systems. class TimerMgr : public boost::noncopyable { public: + /// @brief Returns sole instance of the @c TimerMgr singleton. static TimerMgr& instance(); + /// @name Registering, deregistering and scheduling the timers. + //@{ + + /// @brief Registers new timers in the @c TimerMgr. + /// + /// @param timer_name Unique name for the timer. + /// @param callback Pointer to the callback function to be invoked + /// when the timer elapses, e.g. function processing expired leases + /// in the DHCP server. + /// @param interval Timer interval in milliseconds. + /// @param scheduling_mode Scheduling mode of the timer as described in + /// @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, const asiolink::IntervalTimer::Mode& scheduling_mode); + /// @brief Deregisters 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. + /// + /// @param timer_name Name of the timer to be deregistered. + /// + /// @throw BadValue if the specified timer hasn't been registered. void deregisterTimer(const std::string& timer_name); + /// @brief Deregisters all timers. + /// + /// This method must be explicitly called prior to termination of the + /// process. + void deregisterTimers(); + + /// @brief Schedules the execution of the interval timer. + /// + /// This method schedules the timer, i.e. the callback will be executed + /// after specified interval elapses. The interval has been specified + /// during timer registration. Depending on the mode selected during the + /// timer registration, the callback will be executed once after it has + /// been scheduled or until it is cancelled. Though, in the former case + /// the timer can be re-scheduled in the callback function. + /// + /// @param timer_name Unique timer name. + /// + /// @throw BadValue if the timer hasn't been registered. void setup(const std::string& timer_name); + /// @brief Cancels the execution of the interval timer. + /// + /// This method has no effect if the timer hasn't been scheduled with + /// the @c TimerMgr::setup method. + /// + /// @param timer_name Unique timer name. + /// + /// @throw BadValue if the timer hasn't been registered. + void cancel(const std::string& timer_name); + + //@} + + /// @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. + /// + /// This method has no effect if the thread is not running. void stopThread(); - asiolink::IOService& getIOService() const { - return (*io_service_); - } + //@} + + /// @brief Returns a reference to IO service used by the @c TimerMgr. + asiolink::IOService& getIOService() const; private: + /// @name Constructor and destructor. + //@{ + /// /// @brief Private default constructor. /// /// The @c TimerMgr is a singleton class which instance must be created @@ -58,56 +191,109 @@ private: /// construction via @c TimerMgr::instance. TimerMgr(); + /// @brief Private destructor. + /// + /// Stops the worker thread if it is running. It doesn't deregister any + /// timers to avoid static deinitialization fiasco with the @c IfaceMgr. + ~TimerMgr(); + + //@} + + /// @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 TimerMgr::ifaceMgrCallback is executed from the main thread by the + /// @c IfaceMgr. + /// /// @param timer_name Unique timer name to be passed to the callback. - void localCallback(const std::string& timer_name) const; + 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); + + /// @brief Common method called by the @c TimerMgr::timerCallback and + /// @c TimerMgr::ifaceMgrCallback. + /// + /// @param timer_name Unique timer name. + /// @param mark_ready If 'true' it indicates that the watch socket should + /// be marked 'ready'. If 'false' the socket is cleared. In this case the + /// callback supplied during the timer registration is also executed. + void watchSocketCallback(const std::string& timer_name, + const bool mark_ready); - /// @brief Holds the pointer to the io service object. - 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::scoped_ptr thread_; /// @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. + /// interval timer and other parameters pertaining to it. struct TimerInfo { /// @brief Instance of the watch socket. - util::WatchSocket watch_socket_; + util::WatchSocketPtr watch_socket_; /// @brief Instance of the interval timer. asiolink::IntervalTimerPtr interval_timer_; - asiolink::IntervalTimer::Callback callback_; + /// @brief Holds the pointer to the callback supplied when registering + /// the timer. + asiolink::IntervalTimer::Callback user_callback_; + /// @brief Interval timer interval supplied during registration. long interval_; + /// @brief Interval timer scheduling mode supplied during registration. asiolink::IntervalTimer::Mode scheduling_mode_; - TimerInfo(const util::WatchSocket& watch_socket, - const asiolink::IntervalTimerPtr& interval_timer, - const asiolink::IntervalTimer::Callback& callback, + /// @brief Constructor. + /// + /// @param io_service Reference to the IO service to be used by the + /// interval timer created. + /// @param user_callback Pointer to the callback function supplied + /// during the timer registration. + /// @param interval Timer interval in milliseconds. + /// @param mode Interval timer scheduling mode. + TimerInfo(asiolink::IOService& io_service, + const asiolink::IntervalTimer::Callback& user_callback, const long interval, const asiolink::IntervalTimer::Mode& mode) - : watch_socket_(watch_socket), - interval_timer_(interval_timer), - callback_(callback), + : watch_socket_(new util::WatchSocket()), + interval_timer_(new asiolink::IntervalTimer(io_service)), + user_callback_(user_callback), interval_(interval), scheduling_mode_(mode) { }; }; + /// @brief A type definition for the map holding timers configuration. typedef std::map TimerInfoMap; - /// @brief Holds mapping of the timer name to the watch socket and timer - /// instance. + /// @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. - std::map registered_timers_; + TimerInfoMap registered_timers_; }; } // end of namespace isc::dhcp diff --git a/src/lib/util/watch_socket.h b/src/lib/util/watch_socket.h index a2e1ad1ef2..4e106eb827 100644 --- a/src/lib/util/watch_socket.h +++ b/src/lib/util/watch_socket.h @@ -18,7 +18,7 @@ /// @file watch_socket.h Defines the class, WatchSocket. #include - +#include #include #include @@ -51,7 +51,7 @@ public: /// such as close, read, or altering behavior flags with fcntl or ioctl can have /// unpredictable results. It is intended strictly use with functions such as select() /// poll() or their variants. -class WatchSocket { +class WatchSocket : public boost::noncopyable { public: /// @brief Value used to signify an invalid descriptor. static const int SOCKET_NOT_VALID = -1; -- 2.47.2