Browse Source

[4047] Fixed issues with static deinitialization fiasco in TimerMgr.

Marcin Siodelski 9 years ago
parent
commit
cc1d142670
3 changed files with 40 additions and 16 deletions
  1. 6 1
      src/lib/dhcp/iface_mgr.cc
  2. 24 3
      src/lib/dhcp/iface_mgr.h
  3. 10 12
      src/lib/dhcpsrv/timer_mgr.cc

+ 6 - 1
src/lib/dhcp/iface_mgr.cc

@@ -50,7 +50,12 @@ namespace dhcp {
 
 IfaceMgr&
 IfaceMgr::instance() {
-    static IfaceMgr iface_mgr;
+    return (*instancePtr());
+}
+
+const IfaceMgrPtr&
+IfaceMgr::instancePtr() {
+    static IfaceMgrPtr iface_mgr(new IfaceMgr());
     return (iface_mgr);
 }
 

+ 24 - 3
src/lib/dhcp/iface_mgr.h

@@ -508,6 +508,12 @@ private:
 
 typedef boost::shared_ptr<Iface> IfacePtr;
 
+/// @brief Forward declaration to the @c IfaceMgr.
+class IfaceMgr;
+
+/// @brief Type definition for the pointer to the @c IfaceMgr.
+typedef boost::shared_ptr<IfaceMgr> IfaceMgrPtr;
+
 /// @brief This type describes the callback function invoked when error occurs
 /// in the IfaceMgr.
 ///
@@ -560,9 +566,26 @@ public:
     /// @return the only existing instance of interface manager
     static IfaceMgr& instance();
 
+    /// @brief Returns pointer to the sole instance of the interface manager.
+    ///
+    /// This method returns the pointer to the instance of the interface manager
+    /// which can be held in singleton objects that depend on it. This will
+    /// eliminate issues with the static deinitialization fiasco between this
+    /// object and dependent singleton objects.
+    ///
+    /// The @c IfaceMgr::instance method should be considered deprecated.
+    ///
+    /// @return Shared pointer to the @c IfaceMgr instance.
+    static const IfaceMgrPtr& instancePtr();
+
+    /// @brief Destructor.
+    ///
+    /// Closes open sockets.
+    virtual ~IfaceMgr();
+
     /// @brief Sets or clears the test mode for @c IfaceMgr.
     ///
-    /// Various unit test may set this flag to true, to indicate that the 
+    /// Various unit test may set this flag to true, to indicate that the
     /// @c IfaceMgr is in the test mode. There are places in the code that
     /// modify the behavior depending if the @c IfaceMgr is in the test
     /// mode or not.
@@ -1074,8 +1097,6 @@ protected:
     /// anyone to create instances of IfaceMgr. Use instance() method instead.
     IfaceMgr();
 
-    virtual ~IfaceMgr();
-
     /// @brief Opens IPv4 socket.
     ///
     /// Please do not use this method directly. Use openSocket instead.

+ 10 - 12
src/lib/dhcpsrv/timer_mgr.cc

@@ -277,6 +277,9 @@ private:
     /// @brief Signals that a watch socket has been cleared.
     void signalSocketClearing();
 
+    /// @brief Pointer to the @c IfaceMgr.
+    IfaceMgrPtr iface_mgr_;
+
     /// @brief Pointer to the io service.
     asiolink::IOServicePtr io_service_;
 
@@ -304,12 +307,11 @@ private:
     /// the map. The timer is associated with an instance of the @c WatchSocket
     /// which is marked ready when the interval for the particular elapses.
     TimerInfoMap registered_timers_;
-
 };
 
 TimerMgrImpl::TimerMgrImpl() :
-    io_service_(new IOService()), thread_(), mutex_(), cond_var_(),
-    stopping_(false), registered_timers_() {
+    iface_mgr_(IfaceMgr::instancePtr()), io_service_(new IOService()), thread_(),
+    mutex_(), cond_var_(), stopping_(false), registered_timers_() {
 }
 
 void
@@ -348,9 +350,9 @@ TimerMgrImpl::registerTimer(const std::string& timer_name,
     // to be executed when the data is received over this socket. The only time
     // this may fail is when the socket failed to open which would have caused
     // an exception in the previous call. So we should be safe here.
-    IfaceMgr::instance().addExternalSocket(timer_info->watch_socket_.getSelectFd(),
-                                           boost::bind(&TimerMgrImpl::ifaceMgrCallback,
-                                                       this, timer_name));
+    iface_mgr_->addExternalSocket(timer_info->watch_socket_.getSelectFd(),
+                                  boost::bind(&TimerMgrImpl::ifaceMgrCallback,
+                                              this, timer_name));
 
     // Actually register the timer.
     registered_timers_.insert(std::pair<std::string, TimerInfoPtr>(timer_name,
@@ -380,7 +382,7 @@ TimerMgrImpl::unregisterTimer(const std::string& timer_name) {
     const TimerInfoPtr& timer_info = timer_info_it->second;
 
     // Unregister the watch socket from the IfaceMgr.
-    IfaceMgr::instance().deleteExternalSocket(timer_info->watch_socket_.getSelectFd());
+    iface_mgr_->deleteExternalSocket(timer_info->watch_socket_.getSelectFd());
 
     // Remove the timer.
     registered_timers_.erase(timer_info_it);
@@ -614,12 +616,8 @@ TimerMgr::TimerMgr()
 }
 
 TimerMgr::~TimerMgr() {
-    // Stop the thread, but do not unregister any timers. Unregistering
-    // the timers could cause static deinitialization fiasco between the
-    // TimerMgr and IfaceMgr. By now, the caller should have unregistered
-    // the timers.
     stopThread();
-
+    unregisterTimers();
     delete impl_;
 }