From: Razvan Becheriu <razvan@isc.org>
Date: Tue, 9 Apr 2019 18:17:14 +0000 (+0300)
Subject: base class should take the lock before derived class implementation executes pick... 
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=adf6ca0122f07b89b355ca1b5005e3c4c7bb7506;p=thirdparty%2Fkea.git

base class should take the lock before derived class implementation executes pick address
---

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 2c665e746c..7317389ada 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -25,7 +25,6 @@
 #include <dhcpsrv/callout_handle_store.h>
 #include <stats/stats_mgr.h>
 #include <util/stopwatch.h>
-#include <util/threads/lock_guard.h>
 #include <hooks/server_hooks.h>
 #include <hooks/hooks_manager.h>
 
@@ -157,12 +156,10 @@ AllocEngine::IterativeAllocator::increaseAddress(const isc::asiolink::IOAddress&
 }
 
 isc::asiolink::IOAddress
-AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet,
-                                             const ClientClasses& client_classes,
-                                             const DuidPtr&,
-                                             const IOAddress&) {
-    LockGuard<std::mutex> lock(&mutex_);
-
+AllocEngine::IterativeAllocator::pickAddressInternal(const SubnetPtr& subnet,
+                                                     const ClientClasses& client_classes,
+                                                     const DuidPtr&,
+                                                     const IOAddress&) {
     // Is this prefix allocation?
     bool prefix = pool_type_ == Lease::TYPE_PD;
     uint8_t prefix_len = 0;
@@ -292,10 +289,10 @@ AllocEngine::HashedAllocator::HashedAllocator(Lease::Type lease_type) :
 }
 
 isc::asiolink::IOAddress
-AllocEngine::HashedAllocator::pickAddress(const SubnetPtr&,
-                                          const ClientClasses&,
-                                          const DuidPtr&,
-                                          const IOAddress&) {
+AllocEngine::HashedAllocator::pickAddressInternal(const SubnetPtr&,
+                                                  const ClientClasses&,
+                                                  const DuidPtr&,
+                                                  const IOAddress&) {
     isc_throw(NotImplemented, "Hashed allocator is not implemented");
 }
 
@@ -305,10 +302,10 @@ AllocEngine::RandomAllocator::RandomAllocator(Lease::Type lease_type) :
 }
 
 isc::asiolink::IOAddress
-AllocEngine::RandomAllocator::pickAddress(const SubnetPtr&,
-                                          const ClientClasses&,
-                                          const DuidPtr&,
-                                          const IOAddress&) {
+AllocEngine::RandomAllocator::pickAddressInternal(const SubnetPtr&,
+                                                  const ClientClasses&,
+                                                  const DuidPtr&,
+                                                  const IOAddress&) {
     isc_throw(NotImplemented, "Random allocator is not implemented");
 }
 
diff --git a/src/lib/dhcpsrv/alloc_engine.h b/src/lib/dhcpsrv/alloc_engine.h
index 74758d0187..b4b0ed7e87 100644
--- a/src/lib/dhcpsrv/alloc_engine.h
+++ b/src/lib/dhcpsrv/alloc_engine.h
@@ -18,6 +18,7 @@
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/lease_mgr.h>
 #include <hooks/callout_handle.h>
+#include <util/threads/lock_guard.h>
 
 #include <boost/function.hpp>
 #include <boost/shared_ptr.hpp>
@@ -90,23 +91,35 @@ protected:
         pickAddress(const SubnetPtr& subnet,
                     const ClientClasses& client_classes,
                     const DuidPtr& duid,
-                    const isc::asiolink::IOAddress& hint) = 0;
+                    const isc::asiolink::IOAddress& hint) {
+            isc::util::thread::LockGuard<std::mutex> lock(&mutex_);
+            return pickAddressInternal(subnet, client_classes, duid, hint);
+        }
 
         /// @brief Default constructor.
         ///
         /// Specifies which type of leases this allocator will assign
         /// @param pool_type specifies pool type (addresses, temp. addr or prefixes)
-        Allocator(Lease::Type pool_type)
-            :pool_type_(pool_type) {
+        Allocator(Lease::Type pool_type) : pool_type_(pool_type) {
         }
 
         /// @brief virtual destructor
         virtual ~Allocator() {
         }
 
+    private:
+        virtual isc::asiolink::IOAddress
+        pickAddressInternal(const SubnetPtr& subnet,
+                            const ClientClasses& client_classes,
+                            const DuidPtr& duid,
+                            const isc::asiolink::IOAddress& hint) = 0;
+
     protected:
         /// @brief defines pool type allocation
         Lease::Type pool_type_;
+
+    private:
+        std::mutex mutex_;
     };
 
     /// defines a pointer to allocator
@@ -126,6 +139,7 @@ protected:
         /// @param type - specifies allocation type
         IterativeAllocator(Lease::Type type);
 
+    private:
         /// @brief returns the next address from pools in a subnet
         ///
         /// @param subnet next address will be returned from pool of that subnet
@@ -134,10 +148,10 @@ protected:
         /// @param hint client's hint (ignored)
         /// @return the next address
         virtual isc::asiolink::IOAddress
-            pickAddress(const SubnetPtr& subnet,
-                        const ClientClasses& client_classes,
-                        const DuidPtr& duid,
-                        const isc::asiolink::IOAddress& hint);
+        pickAddressInternal(const SubnetPtr& subnet,
+                            const ClientClasses& client_classes,
+                            const DuidPtr& duid,
+                            const isc::asiolink::IOAddress& hint);
 
     protected:
         /// @brief Returns the next prefix
@@ -166,9 +180,6 @@ protected:
         static isc::asiolink::IOAddress
         increaseAddress(const isc::asiolink::IOAddress& address,
                         bool prefix, const uint8_t prefix_len);
-
-    private:
-        std::mutex mutex_;
     };
 
     /// @brief Address/prefix allocator that gets an address based on a hash
@@ -180,6 +191,7 @@ protected:
         /// @param type - specifies allocation type
         HashedAllocator(Lease::Type type);
 
+    private:
         /// @brief returns an address based on hash calculated from client's DUID.
         ///
         /// @todo: Implement this method
@@ -190,10 +202,10 @@ protected:
         /// @param hint a hint (last address that was picked)
         /// @return selected address
         virtual isc::asiolink::IOAddress
-            pickAddress(const SubnetPtr& subnet,
-                        const ClientClasses& client_classes,
-                        const DuidPtr& duid,
-                        const isc::asiolink::IOAddress& hint);
+        pickAddressInternal(const SubnetPtr& subnet,
+                            const ClientClasses& client_classes,
+                            const DuidPtr& duid,
+                            const isc::asiolink::IOAddress& hint);
     };
 
     /// @brief Random allocator that picks address randomly
@@ -205,6 +217,7 @@ protected:
         /// @param type - specifies allocation type
         RandomAllocator(Lease::Type type);
 
+    private:
         /// @brief returns a random address from pool of specified subnet
         ///
         /// @todo: Implement this method
@@ -215,10 +228,10 @@ protected:
         /// @param hint the last address that was picked (ignored)
         /// @return a random address from the pool
         virtual isc::asiolink::IOAddress
-        pickAddress(const SubnetPtr& subnet,
-                    const ClientClasses& client_classes,
-                    const DuidPtr& duid,
-                    const isc::asiolink::IOAddress& hint);
+        pickAddressInternal(const SubnetPtr& subnet,
+                            const ClientClasses& client_classes,
+                            const DuidPtr& duid,
+                            const isc::asiolink::IOAddress& hint);
     };
 
 public: