From: Thomas Markwalder Date: Fri, 16 Nov 2018 14:04:41 +0000 (-0500) Subject: [#260,!120] Removed Mutexing from IfaceMgr, PacketQueue must now be thread-safe X-Git-Tag: 204-move-models-base~4^2~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f72ce56b1bbd056e10192c684c7078aa1dfdedc7;p=thirdparty%2Fkea.git [#260,!120] Removed Mutexing from IfaceMgr, PacketQueue must now be thread-safe Removed Mutexing from IfaceMgr, added it to default kea packet queue implementations. PacketQueue derivations are required to be thread-safe, thus leaving upto implementers to choose how. --- diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index cdcbf5cacc..4a0dd9cfc2 100644 --- a/src/lib/dhcp/iface_mgr.cc +++ b/src/lib/dhcp/iface_mgr.cc @@ -227,11 +227,6 @@ Receiver::stop() { last_error_ = "thread stopped"; } -isc::util::thread::Mutex& -Receiver::getLock() { - return(lock_); -} - void Receiver::setError(const std::string& error_msg) { last_error_ = error_msg; @@ -1125,14 +1120,9 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec / } // If we're here it should only be because there are DHCP packets waiting. - // Protected packet queue access. - Pkt4Ptr pkt; - { - Mutex::Locker lock(receiver_->getLock()); - pkt = getPacketQueue4()->dequeuePacket(); - if (!pkt) { - receiver_->clearReady(Receiver::RCV_READY); - } + Pkt4Ptr pkt = getPacketQueue4()->dequeuePacket(); + if (!pkt) { + receiver_->clearReady(Receiver::RCV_READY); } return (pkt); @@ -1456,14 +1446,9 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ } // If we're here it should only be because there are DHCP packets waiting. - // Protected packet queue access. - Pkt6Ptr pkt; - { - Mutex::Locker lock(receiver_->getLock()); - pkt = getPacketQueue6()->dequeuePacket(); - if (!pkt) { - receiver_->clearReady(Receiver::RCV_READY); - } + Pkt6Ptr pkt = getPacketQueue6()->dequeuePacket(); + if (!pkt) { + receiver_->clearReady(Receiver::RCV_READY); } return (pkt); @@ -1643,7 +1628,6 @@ IfaceMgr::receiveDHCP4Packet(Iface& iface, const SocketInfo& socket_info) { } if (pkt) { - Mutex::Locker lock(receiver_->getLock()); getPacketQueue4()->enqueuePacket(pkt, socket_info); receiver_->markReady(Receiver::RCV_READY); } @@ -1675,7 +1659,6 @@ IfaceMgr::receiveDHCP6Packet(const SocketInfo& socket_info) { } if (pkt) { - Mutex::Locker lock(receiver_->getLock()); getPacketQueue6()->enqueuePacket(pkt, socket_info); receiver_->markReady(Receiver::RCV_READY); } diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h index dc950e9847..428523ea82 100644 --- a/src/lib/dhcp/iface_mgr.h +++ b/src/lib/dhcp/iface_mgr.h @@ -19,7 +19,6 @@ #include #include #include -#include #include #include @@ -524,13 +523,6 @@ public: /// not done in the destructor to avoid race conditions. void stop(); - /// @brief Fetches the receiver's thread mutex. - /// - /// This is used for instantation mutex locks to protect code blocks. - /// - /// @return a reference to the mutex - isc::util::thread::Mutex& getLock(); - /// @brief Sets the receiver error state /// /// This records the given error message and sets the error watch @@ -557,9 +549,6 @@ public: /// Marked as ready when the DHCP packet receiver thread should terminate. isc::util::WatchSocket sockets_[RCV_TERMINATE + 1]; - /// DHCP packet thread mutex. - isc::util::thread::Mutex lock_; - /// DHCP packet receiver thread. isc::util::thread::ThreadPtr thread_ ; }; diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h index 0d33a4ef26..004bd36dff 100644 --- a/src/lib/dhcp/packet_queue.h +++ b/src/lib/dhcp/packet_queue.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -40,6 +41,8 @@ enum class QueueEnd { /// This class serves as the abstract interface for packet queue /// implementations which may be used by @c IfaceMgr to store /// inbound packets until they are a dequeued for processing. +/// @note Derivations of this class MUST BE thread-safe. +/// @endnote /// template class PacketQueue { @@ -245,12 +248,14 @@ public: /// @brief Pushes a packet onto the queue /// - /// Adds a packet onto the end of queue specified. + /// Adds a packet onto the end of queue specified. Note that this + /// function is protected by a Mutex. /// /// @param packet packet to add to the queue /// @param to specifies the end of the queue to which the packet /// should be added. virtual void pushPacket(PacketTypePtr& packet, const QueueEnd& to=QueueEnd::BACK) { + isc::util::thread::Mutex::Locker lock(mutex_); if (to == QueueEnd::BACK) { queue_.push_back(packet); } else { @@ -260,7 +265,8 @@ public: /// @brief Pops a packet from the queue /// - /// Removes a packet from the end of the queue specified and returns it. + /// Removes a packet from the end of the queue specified and returns it. Note + /// that this function is protected by a Mutex. /// /// @param from specifies the end of the queue from which the packet /// should be taken. @@ -268,6 +274,7 @@ public: /// @return A pointer to dequeued packet, or an empty pointer /// if the queue is empty. virtual PacketTypePtr popPacket(const QueueEnd& from = QueueEnd::FRONT) { + isc::util::thread::Mutex::Locker lock(mutex_); PacketTypePtr packet; if (queue_.empty()) { return (packet); @@ -352,6 +359,9 @@ private: /// @brief Packet queue boost::circular_buffer queue_; + + /// @brief Mutex for protecting queue accesses. + isc::util::thread::Mutex mutex_; };