[880-libdhcp-vs-thread-sanitizer] Checkpoint: first point addressed

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index afcdb03863bb86318a4f20b3e973e58572985427..c1ca2cf55f5502a15fd680fb82f9b8bebcee9700 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -1032,7 +1032,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
     // DHCP packets are waiting so we don't starve external
     // sockets under heavy DHCP load.
     struct timeval select_timeout;
-    if (getPacketQueue4()->empty()) {
+    if (!getPacketQueue4()->canDequeue()) {
         select_timeout.tv_sec = timeout_sec;
         select_timeout.tv_usec = timeout_usec;
     } else {
@@ -1045,7 +1045,7 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
 
     int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
 
-    if ((result == 0) && getPacketQueue4()->empty()) {
+    if ((result == 0) && !getPacketQueue4()->canDequeue()) {
         // nothing received and timeout has been reached
         return (Pkt4Ptr());
     } else if (result < 0) {
@@ -1373,7 +1373,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     // DHCP packets are waiting so we don't starve external
     // sockets under heavy DHCP load.
     struct timeval select_timeout;
-    if (getPacketQueue6()->empty()) {
+    if (!getPacketQueue6()->canDequeue()) {
         select_timeout.tv_sec = timeout_sec;
         select_timeout.tv_usec = timeout_usec;
     } else {
@@ -1386,7 +1386,7 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
 
     int result = select(maxfd + 1, &sockets, NULL, NULL, &select_timeout);
 
-    if ((result == 0) && getPacketQueue6()->empty()) {
+    if ((result == 0) && !getPacketQueue6()->canDequeue()) {
         // nothing received and timeout has been reached
         return (Pkt6Ptr());
     } else if (result < 0) {

diff --git a/src/lib/dhcp/packet_queue.h b/src/lib/dhcp/packet_queue.h
index aea1c95e2abbdc5b3247bd8c8cbcbfd9b6e98f87..375d97d157ea1fc6e9bb2a55bc8503ac86fb2faf 100644 (file)
--- a/src/lib/dhcp/packet_queue.h
+++ b/src/lib/dhcp/packet_queue.h
@@ -82,6 +82,11 @@ public:
     /// @brief return True if the queue is empty.
     virtual bool empty() const = 0;
 
+    /// @brief return True if dequeue will not return null.
+    ///
+    /// In fact it is !empty within a lock guard.
+    virtual bool canDequeue() = 0;
+
     /// @brief Returns the current number of packets in the buffer.
     virtual size_t getSize() const = 0;
 

diff --git a/src/lib/dhcp/packet_queue_ring.h b/src/lib/dhcp/packet_queue_ring.h
index 24b8890a269b085d4dc3e93584a7553627f764db..3db20088cc5c2400f27c6752d02dd644f451364d 100644 (file)
--- a/src/lib/dhcp/packet_queue_ring.h
+++ b/src/lib/dhcp/packet_queue_ring.h
@@ -123,12 +123,13 @@ public:
     /// @return A pointer to dequeued packet, or an empty pointer
     /// if the queue is empty.
     virtual PacketTypePtr popPacket(const QueueEnd& from = QueueEnd::FRONT) {
+        std::lock_guard<std::mutex> lock(mutex_);
+
         PacketTypePtr packet;
         if (queue_.empty()) {
             return (packet);
         }
 
-        std::lock_guard<std::mutex> lock(mutex_);
         if (from == QueueEnd::FRONT) {
             packet = queue_.front();
             queue_.pop_front();
@@ -161,7 +162,14 @@ public:
 
     /// @brief Returns True if the queue is empty.
     virtual bool empty() const {
-        return(queue_.empty());
+        return (queue_.empty());
+    }
+
+    /// @brief return True if dequeue will not return null.
+    /// Use the mutex so race free with a concurrent enqueue.
+    virtual bool canDequeue() {
+        std::lock_guard<std::mutex> lock(mutex_);
+        return (!queue_.empty());
     }
 
     /// @brief Returns the maximum number of packets allowed in the buffer.