From: Thomas Markwalder Date: Mon, 19 Nov 2018 16:18:52 +0000 (-0500) Subject: [#260, !120] PacketQueueMgr4/6 are no longer singletons X-Git-Tag: 204-move-models-base~4^2~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0741a035c46f338cf50749fbf4bd6fc350471a82;p=thirdparty%2Fkea.git [#260, !120] PacketQueueMgr4/6 are no longer singletons dhcp::IfaceMgr simply owns instances of the packet queue managers, rather than them being singletons. --- diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc index 3320feacb7..06e1c3927d 100644 --- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc +++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc @@ -640,7 +640,7 @@ ControlledDhcpv4Srv::processConfig(isc::data::ConstElementPtr config) { qc = CfgMgr::instance().getStagingCfg()->getDHCPQueueControl(); if (IfaceMgr::instance().configureDHCPPacketQueue(AF_INET, qc)) { LOG_INFO(dhcp4_logger, DHCP4_CONFIG_PACKET_QUEUE) - .arg(PacketQueueMgr4::instance().getPacketQueue()->getInfoStr()); + .arg(IfaceMgr::instance().getPacketQueue4()->getInfoStr()); } } catch (const std::exception& ex) { diff --git a/src/bin/dhcp6/ctrl_dhcp6_srv.cc b/src/bin/dhcp6/ctrl_dhcp6_srv.cc index c9cf5a1ff9..989840089f 100644 --- a/src/bin/dhcp6/ctrl_dhcp6_srv.cc +++ b/src/bin/dhcp6/ctrl_dhcp6_srv.cc @@ -659,7 +659,7 @@ ControlledDhcpv6Srv::processConfig(isc::data::ConstElementPtr config) { qc = CfgMgr::instance().getStagingCfg()->getDHCPQueueControl(); if (IfaceMgr::instance().configureDHCPPacketQueue(AF_INET6, qc)) { LOG_INFO(dhcp6_logger, DHCP6_CONFIG_PACKET_QUEUE) - .arg(PacketQueueMgr6::instance().getPacketQueue()->getInfoStr()); + .arg(IfaceMgr::instance().getPacketQueue6()->getInfoStr()); } } catch (const std::exception& ex) { diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 4620bfb3e0..91af356b01 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -253,8 +253,8 @@ IfaceMgr::IfaceMgr() // Ensure that PQMs have been created to guarantee we have // default packet queues in place. try { - PacketQueueMgr4::create(); - PacketQueueMgr6::create(); + packet_queue_mgr4_.reset(new PacketQueueMgr4()); + packet_queue_mgr6_.reset(new PacketQueueMgr6()); } catch (const std::exception& ex) { isc_throw(Unexpected, "Failed to create PacketQueueManagers: " << ex.what()); } @@ -357,14 +357,14 @@ void IfaceMgr::stopDHCPReceiver() { if (isReceiverRunning()) { receiver_->stop(); receiver_.reset(); + } - if (getPacketQueue4()) { - getPacketQueue4()->clear(); - } + if (getPacketQueue4()) { + getPacketQueue4()->clear(); + } - if (getPacketQueue6()) { - getPacketQueue6()->clear(); - } + if (getPacketQueue6()) { + getPacketQueue6()->clear(); } } @@ -373,9 +373,6 @@ IfaceMgr::~IfaceMgr() { control_buf_len_ = 0; closeSockets(); - // Explicitly delete PQM singletons. - PacketQueueMgr4::destroy(); - PacketQueueMgr6::destroy(); } bool @@ -1780,16 +1777,16 @@ IfaceMgr::configureDHCPPacketQueue(uint16_t family, data::ConstElementPtr queue_ if (enable_queue) { // Try to create the queue as configured. if (family == AF_INET) { - PacketQueueMgr4::instance().createPacketQueue(queue_control); + packet_queue_mgr4_->createPacketQueue(queue_control); } else { - PacketQueueMgr6::instance().createPacketQueue(queue_control); + packet_queue_mgr6_->createPacketQueue(queue_control); } } else { // Destroy the current queue (if one), this inherently disables threading. if (family == AF_INET) { - PacketQueueMgr4::instance().destroyPacketQueue(); + packet_queue_mgr4_->destroyPacketQueue(); } else { - PacketQueueMgr6::instance().destroyPacketQueue(); + packet_queue_mgr6_->destroyPacketQueue(); } } diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index a32912b83d..af9d4cfce3 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -1127,22 +1127,38 @@ public: /// @return true if there is a socket bound to the specified address. bool hasOpenSocket(const isc::asiolink::IOAddress& addr) const; - /// @brief DHCPv4 receiver packet queue. + /// @brief Fetches the DHCPv4 packet queue manager + /// + /// @return pointer to the packet queue mgr + PacketQueueMgr4Ptr getPacketQueueMgr4() { + return (packet_queue_mgr4_); + } + + /// @brief Fetches the DHCPv4 receiver packet queue. /// /// Incoming packets are read by the receiver thread and /// added to this queue. @c receive4() dequeues and /// returns them. + /// @return pointer to the packet queue PacketQueue4Ptr getPacketQueue4() { - return (PacketQueueMgr4::instance().getPacketQueue()); + return (packet_queue_mgr4_->getPacketQueue()); + } + + /// @brief Fetches the DHCPv6 packet queue manager + /// + /// @return pointer to the packet queue mgr + PacketQueueMgr6Ptr getPacketQueueMgr6() { + return (packet_queue_mgr6_); } - /// @brief DHCPv6 receiver packet queue. + /// @brief Fetches the DHCPv6 receiver packet queue. /// /// Incoming packets are read by the receiver thread and /// added to this queue. @c receive6() dequeues and /// returns them. + /// @return pointer to the packet queue PacketQueue6Ptr getPacketQueue6() { - return (PacketQueueMgr6::instance().getPacketQueue()); + return (packet_queue_mgr6_->getPacketQueue()); } /// @brief Starts DHCP packet receiver. @@ -1468,6 +1484,12 @@ private: /// @brief Allows to use loopback bool allow_loopback_; + /// @brief Manager for DHCPv4 packet implementations and queues + PacketQueueMgr4Ptr packet_queue_mgr4_; + + /// @brief Manager for DHCPv6 packet implementations and queues + PacketQueueMgr6Ptr packet_queue_mgr6_; + /// DHCP packet receiver. ReceiverPtr receiver_; }; diff --git a/src/lib/dhcp/packet_queue_mgr4.cc b/src/lib/dhcp/packet_queue_mgr4.cc index 4a805f520d..b7ba4ffeb0 100644 --- a/src/lib/dhcp/packet_queue_mgr4.cc +++ b/src/lib/dhcp/packet_queue_mgr4.cc @@ -31,30 +31,5 @@ PacketQueueMgr4::PacketQueueMgr4() { }); } -boost::scoped_ptr& -PacketQueueMgr4::getPacketQueueMgr4Ptr() { - static boost::scoped_ptr packet_mgr; - return (packet_mgr); -} - -void -PacketQueueMgr4::create() { - getPacketQueueMgr4Ptr().reset(new PacketQueueMgr4()); -} - -void -PacketQueueMgr4::destroy() { - getPacketQueueMgr4Ptr().reset(); -} - -PacketQueueMgr4& -PacketQueueMgr4::instance() { - boost::scoped_ptr& packet_mgr = getPacketQueueMgr4Ptr(); - if (!packet_mgr) { - create(); - } - return (*packet_mgr); -} - } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/lib/dhcp/packet_queue_mgr4.h b/src/lib/dhcp/packet_queue_mgr4.h index 6828c9deb3..33fd4e400f 100644 --- a/src/lib/dhcp/packet_queue_mgr4.h +++ b/src/lib/dhcp/packet_queue_mgr4.h @@ -19,53 +19,23 @@ namespace dhcp { /// Implements the "manager" class which holds information about the /// supported DHCPv4 packet queue implementations and provides management /// of the current queue instance. -/// -/// It is implemented as a singleton that can be accessed from any place -/// within the server code. This includes server configuration, data -/// fetching during normal server operation and data management, including -/// processing of control commands implemented within hooks libraries. -class PacketQueueMgr4 : public PacketQueueMgr, - public boost::noncopyable { +class PacketQueueMgr4 : public PacketQueueMgr { + public: /// @brief Logical name of the pre-registered, default queue implementation static const std::string DEFAULT_QUEUE_TYPE4; - /// @brief virtual Destructor - virtual ~PacketQueueMgr4(){} - - /// @brief Creates new instance of the @c PacketQueueMgr4. - /// - /// If an instance of the @c PacketQueueMgr4 already exists, - /// it will be replaced by the new instance. Thus, all factories - /// will be unregistered and config databases will be dropped. - static void create(); - - /// @brief Destroys the instance of the @c PacketQueueMgr4. - /// - /// If an instance of the @c PacketQueueMgr4 exists, - /// it will be destroyed. Thus, all factories will be unregistered - /// and config databases will be dropped. - static void destroy(); - - /// @brief Returns a sole instance of the @c PacketQueueMgr4. - /// - /// This method is used to retrieve the instance of the of the - /// @c PacketQueueMgr4 created by the @c create method. If the - /// instance doesn't exist yet, it is created using the @c create - /// method. - static PacketQueueMgr4& instance(); - -private: - /// @brief Private constructor. - /// /// It registers a default factory for DHCPv4 queues. PacketQueueMgr4(); - /// @brief Returns a pointer to the currently instance of the - /// @c PacketQueueMgr4. - static boost::scoped_ptr& getPacketQueueMgr4Ptr(); + /// @brief virtual Destructor + virtual ~PacketQueueMgr4(){} }; +/// @brief Defines a shared pointer to PacketQueueMgr4 +typedef boost::shared_ptr PacketQueueMgr4Ptr; + + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcp/packet_queue_mgr6.cc b/src/lib/dhcp/packet_queue_mgr6.cc index 50ea7ab388..bcb83ec527 100644 --- a/src/lib/dhcp/packet_queue_mgr6.cc +++ b/src/lib/dhcp/packet_queue_mgr6.cc @@ -31,30 +31,5 @@ PacketQueueMgr6::PacketQueueMgr6() { }); } -boost::scoped_ptr& -PacketQueueMgr6::getPacketQueueMgr6Ptr() { - static boost::scoped_ptr packet_mgr; - return (packet_mgr); -} - -void -PacketQueueMgr6::create() { - getPacketQueueMgr6Ptr().reset(new PacketQueueMgr6()); -} - -void -PacketQueueMgr6::destroy() { - getPacketQueueMgr6Ptr().reset(); -} - -PacketQueueMgr6& -PacketQueueMgr6::instance() { - boost::scoped_ptr& packet_mgr = getPacketQueueMgr6Ptr(); - if (!packet_mgr) { - create(); - } - return (*packet_mgr); -} - } // end of isc::dhcp namespace } // end of isc namespace diff --git a/src/lib/dhcp/packet_queue_mgr6.h b/src/lib/dhcp/packet_queue_mgr6.h index 36d93d8bd1..7711ac9e1a 100644 --- a/src/lib/dhcp/packet_queue_mgr6.h +++ b/src/lib/dhcp/packet_queue_mgr6.h @@ -19,53 +19,24 @@ namespace dhcp { /// Implements the "manager" class which holds information about the /// supported DHCPv6 packet queue implementations and provides management /// of the current queue instance. -/// -/// It is implemented as a singleton that can be accessed from any place -/// within the server code. This includes server configuration, data -/// fetching during normal server operation and data management, including -/// processing of control commands implemented within hooks libraries. class PacketQueueMgr6 : public PacketQueueMgr, public boost::noncopyable { public: /// @brief Logical name of the pre-registered, default queue implementation static const std::string DEFAULT_QUEUE_TYPE6; - /// @brief virtual Destructor - virtual ~PacketQueueMgr6(){} - - /// @brief Creates new instance of the @c PacketQueueMgr6. - /// - /// If an instance of the @c PacketQueueMgr6 already exists, - /// it will be replaced by the new instance. Thus, all factories - /// will be unregistered and config databases will be dropped. - static void create(); - - /// @brief Destroys the instance of the @c PacketQueueMgr6. - /// - /// If an instance of the @c PacketQueueMgr6 exists, - /// it will be destroyed. Thus, all factories will be unregistered - /// and config databases will be dropped. - static void destroy(); - - /// @brief Returns a sole instance of the @c PacketQueueMgr6. - /// - /// This method is used to retrieve the instance of the of the - /// @c PacketQueueMgr6 created by the @c create method. If the - /// instance doesn't exist yet, it is created using the @c create - /// method. - static PacketQueueMgr6& instance(); - -private: - /// @brief Private constructor. + /// @brief constructor. /// /// It registers a default factory for DHCPv6 queues. PacketQueueMgr6(); - /// @brief Returns a pointer to the currently used instance of the - /// @c PacketQueueMgr6. - static boost::scoped_ptr& getPacketQueueMgr6Ptr(); + /// @brief virtual Destructor + virtual ~PacketQueueMgr6(){} }; +/// @brief Defines a shared pointer to PacketQueueMgr6 +typedef boost::shared_ptr PacketQueueMgr6Ptr; + } // end of namespace isc::dhcp } // end of namespace isc diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc index 8559139fe5..9f240a9b0b 100644 --- a/src/lib/dhcp/tests/iface_mgr_unittest.cc +++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc @@ -866,13 +866,9 @@ TEST_F(IfaceMgrTest, packetQueue4) { // Verify that we can create a queue with default factory. data::ConstElementPtr config = makeQueueConfig(PacketQueueMgr4::DEFAULT_QUEUE_TYPE4, 2000); - ASSERT_NO_THROW(PacketQueueMgr4::instance().createPacketQueue(config)); + ASSERT_NO_THROW(ifacemgr.getPacketQueueMgr4()->createPacketQueue(config)); CHECK_QUEUE_INFO(ifacemgr.getPacketQueue4(), "{ \"capacity\": 2000, \"queue-type\": \"" << PacketQueueMgr4::DEFAULT_QUEUE_TYPE4 << "\", \"size\": 0 }"); - - // Verify that fetching the queue via IfaceMgr and PacketQueueMgr - // returns the same queue. - ASSERT_EQ(ifacemgr.getPacketQueue4(), PacketQueueMgr4::instance().getPacketQueue()); } // Verify that we have the expected default DHCPv6 packet queue. @@ -884,13 +880,9 @@ TEST_F(IfaceMgrTest, packetQueue6) { // Verify that we can create a queue with default factory. data::ConstElementPtr config = makeQueueConfig(PacketQueueMgr6::DEFAULT_QUEUE_TYPE6, 2000); - ASSERT_NO_THROW(PacketQueueMgr6::instance().createPacketQueue(config)); + ASSERT_NO_THROW(ifacemgr.getPacketQueueMgr6()->createPacketQueue(config)); CHECK_QUEUE_INFO(ifacemgr.getPacketQueue6(), "{ \"capacity\": 2000, \"queue-type\": \"" << PacketQueueMgr6::DEFAULT_QUEUE_TYPE6 << "\", \"size\": 0 }"); - - // Verify that fetching the queue via IfaceMgr and PacketQueueMgr - // returns the same queue. - ASSERT_EQ(ifacemgr.getPacketQueue6(), PacketQueueMgr6::instance().getPacketQueue()); } TEST_F(IfaceMgrTest, receiveTimeout6) { @@ -3282,12 +3274,10 @@ TEST_F(ReceiverTest, receiverClassBasics) { ASSERT_FALSE(receiver_->isReady(Receiver::RCV_TERMINATE)); EXPECT_EQ("thread expired", receiver_->getLastError()); - // This one is a little wonky, as a thread function expiring needs to be - // supported in Receiver. There needs to be something in Receiver, so it - // nows the thread exited. Thread exists but I think it's underlying - // impl does not. + // Thread is technically still running, so let's stop it. EXPECT_TRUE(receiver_->isRunning()); ASSERT_NO_THROW(receiver_->stop()); + ASSERT_FALSE(receiver_->isRunning()); /// Now we'll test stopping a thread. /// Start the receiver, let it run a little and then tell it to stop. diff --git a/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc b/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc index 947c3696ad..0de5557f2e 100644 --- a/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc +++ b/src/lib/dhcp/tests/packet_queue_mgr4_unittest.cc @@ -41,15 +41,11 @@ public: /// Note that it instantiates the PQM singleton. PacketQueueMgr4Test() : default_queue_type_(PacketQueueMgr4::DEFAULT_QUEUE_TYPE4) { - PacketQueueMgr4::create(); + packet_queue_mgr4_.reset(new PacketQueueMgr4()); } /// @brief Destructor - /// - /// It destroys the PQM singleton. - virtual ~PacketQueueMgr4Test(){ - PacketQueueMgr4::destroy(); - } + virtual ~PacketQueueMgr4Test(){} /// @brief Registers a queue type factory /// @@ -85,7 +81,7 @@ public: /// @brief Fetches a pointer to the PQM singleton PacketQueueMgr4& mgr() { - return (PacketQueueMgr4::instance()); + return (*packet_queue_mgr4_); }; /// @brief Tests the current packet queue info against expected content @@ -96,7 +92,11 @@ public: checkInfo((mgr().getPacketQueue()), exp_json); } + /// @brief default queue type used for a given test std::string default_queue_type_; + + /// @brief Packet Queue manager instance + PacketQueueMgr4Ptr packet_queue_mgr4_; }; // Verifies that DHCPv4 PQM provides a default queue factory @@ -109,12 +109,6 @@ TEST_F(PacketQueueMgr4Test, defaultQueue) { ASSERT_NO_THROW(mgr().createPacketQueue(config)); CHECK_QUEUE_INFO (mgr().getPacketQueue(), "{ \"capacity\": 2000, \"queue-type\": \"" << default_queue_type_ << "\", \"size\": 0 }"); - - // We should be able to recreate the manager. - ASSERT_NO_THROW(PacketQueueMgr4::create()); - - // Should not be a queue. - ASSERT_FALSE(mgr().getPacketQueue()); } // Verifies that PQM registry and creation of custome queue implementations. diff --git a/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc b/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc index a1e98b2f08..89cd49a48a 100644 --- a/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc +++ b/src/lib/dhcp/tests/packet_queue_mgr6_unittest.cc @@ -27,15 +27,14 @@ public: /// Note that it instantiates the PQM singleton. PacketQueueMgr6Test() : default_queue_type_(PacketQueueMgr6::DEFAULT_QUEUE_TYPE6) { - PacketQueueMgr6::create(); + packet_queue_mgr6_.reset(new PacketQueueMgr6()); + } /// @brief Destructor /// /// It destroys the PQM singleton. - virtual ~PacketQueueMgr6Test(){ - PacketQueueMgr6::destroy(); - } + virtual ~PacketQueueMgr6Test(){} /// @brief Registers a queue type factory /// @@ -71,7 +70,7 @@ public: /// @brief Fetches a pointer to the PQM singleton PacketQueueMgr6& mgr() { - return (PacketQueueMgr6::instance()); + return (*packet_queue_mgr6_); }; /// @brief Tests the current packet queue info against expected content @@ -82,7 +81,11 @@ public: checkInfo((mgr().getPacketQueue()), exp_json); } + /// @brief default queue type used for a given test std::string default_queue_type_; + + /// @brief Packet Queue manager instance + PacketQueueMgr6Ptr packet_queue_mgr6_; }; // Verifies that DHCPv6 PQM provides a default queue factory @@ -95,12 +98,6 @@ TEST_F(PacketQueueMgr6Test, defaultQueue) { ASSERT_NO_THROW(mgr().createPacketQueue(config)); CHECK_QUEUE_INFO (mgr().getPacketQueue(), "{ \"capacity\": 2000, \"queue-type\": \"" << default_queue_type_ << "\", \"size\": 0 }"); - - // We should be able to recreate the manager. - ASSERT_NO_THROW(PacketQueueMgr6::create()); - - // Should not be a queue. - ASSERT_FALSE(mgr().getPacketQueue()); } // Verifies that PQM registry and creation of custome queue implementations.