From: Francis Dupont <fdupont@isc.org>
Date: Thu, 28 May 2020 11:55:57 +0000 (+0200)
Subject: [#1147] Reviewed last changes
X-Git-Tag: Kea-1.7.9~110
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1e9bf791681eaabbc7407880a2d5a495bfb659a3;p=thirdparty%2Fkea.git

[#1147] Reviewed last changes
---

diff --git a/src/bin/dhcp4/client_handler.cc b/src/bin/dhcp4/client_handler.cc
index 6aabcf5c62..fbab5d4959 100644
--- a/src/bin/dhcp4/client_handler.cc
+++ b/src/bin/dhcp4/client_handler.cc
@@ -136,9 +136,26 @@ ClientHandler::ClientHandler()
 }
 
 ClientHandler::~ClientHandler() {
-    if (locked_client_id_ || locked_hwaddr_) {
-        lock_guard<mutex> lk(mutex_);
-        unLock();
+    bool unlocked = false;
+    lock_guard<mutex> lk(mutex_);
+    if (locked_client_id_) {
+        unlocked = true;
+        unLockById();
+    }
+    if (locked_hwaddr_) {
+        unlocked = true;
+        unLockByHWAddr();
+    }
+    if (!unlocked || !client_ || !client_->cont_) {
+        return;
+    }
+    // Try to process next query. As the caller holds the mutex of
+    // the handler class the continuation will be resumed after.
+    MultiThreadingMgr& mt_mgr = MultiThreadingMgr::instance();
+    if (mt_mgr.getMode()) {
+        if (!mt_mgr.getThreadPool().addFront(client_->cont_)) {
+            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_PACKET_QUEUE_FULL);
+        }
     }
 }
 
@@ -178,38 +195,37 @@ ClientHandler::tryLock(Pkt4Ptr query, ContinuationPtr cont) {
     Pkt4Ptr next_query_hw;
     client_.reset(new Client(query, duid, hwaddr));
 
-    {
+    // Try first duid.
+    if (duid) {
+        // Try to acquire the by-client-id lock and return the holder
+        // when it failed.
         lock_guard<mutex> lk(mutex_);
-        // Try first duid.
-        if (duid) {
-            // Try to acquire the by-client-id lock and return the holder
-            // when it failed.
-            holder_id = lookup(duid);
-            if (!holder_id) {
-                locked_client_id_ = duid;
-                lockById();
-            } else if (cont) {
-                next_query_id = holder_id->next_query_;
-                holder_id->next_query_ = query;
-                holder_id->cont_ = cont;
-            }
-        }
+        holder_id = lookup(duid);
         if (!holder_id) {
-            if (!hwaddr) {
-                return (true);
-            }
-            // Try to acquire the by-hw-addr lock and return the holder
-            // when it failed.
-            holder_hw = lookup(hwaddr);
-            if (!holder_hw) {
-                locked_hwaddr_ = hwaddr;
-                lockByHWAddr();
-                return (true);
-            } else if (cont) {
-                next_query_hw = holder_hw->next_query_;
-                holder_hw->next_query_ = query;
-                holder_hw->cont_ = cont;
-            }
+            locked_client_id_ = duid;
+            lockById();
+        } else if (cont) {
+            next_query_id = holder_id->next_query_;
+            holder_id->next_query_ = query;
+            holder_id->cont_ = cont;
+        }
+    }
+    if (!holder_id) {
+        if (!hwaddr) {
+            return (true);
+        }
+        // Try to acquire the by-hw-addr lock and return the holder
+        // when it failed.
+        lock_guard<mutex> lk(mutex_);
+        holder_hw = lookup(hwaddr);
+        if (!holder_hw) {
+            locked_hwaddr_ = hwaddr;
+            lockByHWAddr();
+            return (true);
+        } else if (cont) {
+            next_query_hw = holder_hw->next_query_;
+            holder_hw->next_query_ = query;
+            holder_hw->cont_ = cont;
         }
     }
 
@@ -288,29 +304,6 @@ ClientHandler::lockByHWAddr() {
     addByHWAddr(client_);
 }
 
-void
-ClientHandler::unLock() {
-    if (locked_client_id_) {
-        unLockById();
-    }
-    if (locked_hwaddr_) {
-        unLockByHWAddr();
-    }
-
-    if (!client_ || !client_->cont_) {
-        return;
-    }
-
-    // Try to process next query. As the caller holds the mutex of
-    // the handler class the continuation will be resumed after.
-    MultiThreadingMgr& mt_mgr = MultiThreadingMgr::instance();
-    if (mt_mgr.getMode()) {
-        if (!mt_mgr.getThreadPool().addFront(client_->cont_)) {
-            LOG_DEBUG(dhcp4_logger, DBG_DHCP4_BASIC, DHCP4_PACKET_QUEUE_FULL);
-        }
-    }
-}
-
 void
 ClientHandler::unLockById() {
     // Sanity check.
diff --git a/src/bin/dhcp4/client_handler.h b/src/bin/dhcp4/client_handler.h
index 8ae393ea89..9b2b0c0e2d 100644
--- a/src/bin/dhcp4/client_handler.h
+++ b/src/bin/dhcp4/client_handler.h
@@ -222,14 +222,6 @@ private:
     /// The mutex must be held by the caller.
     void lockByHWAddr();
 
-    /// @brief Release a client.
-    ///
-    /// If the client has a continuation, push it at front of the thread
-    /// packet queue.
-    ///
-    /// The mutex must be held by the only caller: the destructor.
-    void unLock();
-
     /// @brief Release a client by client ID option.
     ///
     /// The mutex must be held by the caller.