[#1147] Reorganized v6 client handler

diff --git a/src/bin/dhcp6/client_handler.cc b/src/bin/dhcp6/client_handler.cc
index f4c9085def317059eee95979b16b2f520b112617..ba821e06c259c48c01da01e8a937695c1ca9d698 100644 (file)
--- a/src/bin/dhcp6/client_handler.cc
+++ b/src/bin/dhcp6/client_handler.cc
@@ -18,20 +18,6 @@ using namespace isc::util;
 namespace isc {
 namespace dhcp {
 
-mutex ClientHandler::mutex_;
-
-ClientHandler::ClientContainer ClientHandler::clients_;
-
-ClientHandler::ClientHandler() : client_(), locked_() {
-}
-
-ClientHandler::~ClientHandler() {
-    if (locked_) {
-        lock_guard<mutex> lock_(mutex_);
-        unLock();
-    }
-}
-
 ClientHandler::Client::Client(Pkt6Ptr query, DuidPtr client_id)
     : query_(query), thread_(this_thread::get_id()) {
     // Sanity checks.
@@ -45,6 +31,10 @@ ClientHandler::Client::Client(Pkt6Ptr query, DuidPtr client_id)
     duid_ = client_id->getDuid();
 }
 
+mutex ClientHandler::mutex_;
+
+ClientHandler::ClientContainer ClientHandler::clients_;
+
 ClientHandler::ClientPtr
 ClientHandler::lookup(const DuidPtr& duid) {
     // Sanity check.
@@ -60,37 +50,34 @@ ClientHandler::lookup(const DuidPtr& duid) {
 }
 
 void
-ClientHandler::lock() {
+ClientHandler::add(const ClientPtr& client) {
     // Sanity check.
-    if (!locked_) {
-        isc_throw(Unexpected, "nothing to lock in ClientHandler::lock");
+    if (!client) {
+        isc_throw(InvalidParameter, "client is null in ClientHandler::add");
     }
 
     // Assume insert will never fail so not checking its result.
-    clients_.insert(client_);
+    clients_.insert(client);
 }
 
 void
-ClientHandler::unLock() {
+ClientHandler::del(const DuidPtr& duid) {
     // Sanity check.
-    if (!locked_) {
-        isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock");
+    if (!duid) {
+        isc_throw(InvalidParameter, "duid is null in ClientHandler::del");
     }
 
     // Assume erase will never fail so not checking its result.
-    clients_.erase(locked_->getDuid());
-    locked_.reset();
-    if (!client_ || !client_->cont_) {
-        return;
-    }
+    clients_.erase(duid->getDuid());
+}
 
-    // 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(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_PACKET_QUEUE_FULL);
-        }
+ClientHandler::ClientHandler() : client_(), locked_() {
+}
+
+ClientHandler::~ClientHandler() {
+    if (locked_) {
+        lock_guard<mutex> lock_(mutex_);
+        unLock();
     }
 }
 
@@ -158,5 +145,37 @@ ClientHandler::tryLock(Pkt6Ptr query, ContinuationPtr cont) {
     return (false);
 }
 
+void
+ClientHandler::lock() {
+    // Sanity check.
+    if (!locked_) {
+        isc_throw(Unexpected, "nothing to lock in ClientHandler::lock");
+    }
+    add(client_);
+}
+
+void
+ClientHandler::unLock() {
+    // Sanity check.
+    if (!locked_) {
+        isc_throw(Unexpected, "nothing to unlock in ClientHandler::unLock");
+    }
+
+    del(locked_);
+    locked_.reset();
+    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(dhcp6_logger, DBG_DHCP6_BASIC, DHCP6_PACKET_QUEUE_FULL);
+        }
+    }
+}
+
 }  // namespace dhcp
 }  // namespace isc

diff --git a/src/bin/dhcp6/client_handler.h b/src/bin/dhcp6/client_handler.h
index d7dd8a015d34c1b70d6f074af1603a0b4b85b377..9ad5a7b719b033414fb9acfb5ab8092be241d60f 100644 (file)
--- a/src/bin/dhcp6/client_handler.h
+++ b/src/bin/dhcp6/client_handler.h
@@ -35,31 +35,10 @@ inline ContinuationPtr makeContinuation(Continuation&& cont) {
 
 /// @brief Client race avoidance RAII handler.
 class ClientHandler : public boost::noncopyable {
-public:
-
-    /// @brief Constructor.
-    ClientHandler();
-
-    /// @brief Destructor.
-    ///
-    /// Releases the client if it was acquired.
-    virtual ~ClientHandler();
-
-    /// @brief Tries to acquires a client.
-    ///
-    /// Lookup the client:
-    ///  - if not found insert the client in the clients map and return true
-    ///  - if found, if has a continuation put it in the holder,
-    ///    and return false
-    ///
-    /// @param query The query from the client.
-    /// @param cont The continuation in the case the client was held.
-    /// @return true if the client was acquired, false if there is already
-    /// a query from the same client.
-    bool tryLock(Pkt6Ptr query, ContinuationPtr cont = ContinuationPtr());
-
 private:
 
+    /// Class (aka static) types, methods and members.
+
     /// @brief Structure representing a client.
     struct Client {
 
@@ -95,25 +74,85 @@ private:
     /// @brief The type of shared pointers to clients.
     typedef boost::shared_ptr<Client> ClientPtr;
 
-    /// @brief Local client.
-    ClientPtr client_;
+    /// @brief The type of the client container.
+    typedef boost::multi_index_container<
 
-    /// @brief Client ID locked by this handler.
-    DuidPtr locked_;
+        // This container stores pointers to client objects.
+        ClientPtr,
 
-    /// @brief Mutex to protect the client container.
-    ///
-    /// The mutex is used only by public methods for guards.
-    static std::mutex mutex_;
+        // Start specification of indexes here.
+        boost::multi_index::indexed_by<
+
+            // First index is used to search by Duid.
+            boost::multi_index::hashed_unique<
+
+                // Client ID binary content as a member of the Client object.
+                boost::multi_index::member<
+                    Client, std::vector<uint8_t>, &Client::duid_
+                >
+            >
+        >
+    > ClientContainer;
 
     /// @brief Lookup a client.
     ///
     /// The mutex must be held by the caller.
     ///
     /// @param duid The duid of the query from the client.
-    /// @return The held client or null.
+    /// @return The client found in the container or null.
     static ClientPtr lookup(const DuidPtr& duid);
 
+    /// @brief Add a client.
+    ///
+    /// The mutex must be held by the caller.
+    ///
+    /// @param client The client to insert into the client container.
+    static void add(const ClientPtr& client);
+
+    /// @brief Delete a client.
+    ///
+    /// The mutex must be held by the caller.
+    ///
+    /// @param duid The duid to delete from the client container.
+    static void del(const DuidPtr& duid);
+
+    /// @brief Mutex to protect the client container.
+    ///
+    /// The mutex is used only by public methods for guards.
+    static std::mutex mutex_;
+
+    /// @brief The client container.
+    static ClientContainer clients_;
+
+public:
+
+    /// Public interface.
+
+    /// @brief Constructor.
+    ClientHandler();
+
+    /// @brief Destructor.
+    ///
+    /// Releases the client if it was acquired.
+    virtual ~ClientHandler();
+
+    /// @brief Tries to acquires a client.
+    ///
+    /// Lookup the client:
+    ///  - if not found insert the client in the clients map and return true
+    ///  - if found, if has a continuation put it in the holder,
+    ///    and return false
+    ///
+    /// @param query The query from the client.
+    /// @param cont The continuation in the case the client was held.
+    /// @return true if the client was acquired, false if there is already
+    /// a query from the same client.
+    bool tryLock(Pkt6Ptr query, ContinuationPtr cont = ContinuationPtr());
+
+private:
+
+    /// Instance methods and members.
+
     /// @brief Acquire a client.
     ///
     /// The mutex must be held by the caller.
@@ -124,31 +163,15 @@ private:
     /// If the client has a continuation, push it at front of the thread
     /// packet queue.
     ///
-    /// The mutex must be held by the caller.
+    /// The mutex must be held by the only caller: the destructor.
     void unLock();
 
-    /// @brief The type of the client container.
-    typedef boost::multi_index_container<
-
-        // This container stores pointers to client objects.
-        ClientPtr,
-
-        // Start specification of indexes here.
-        boost::multi_index::indexed_by<
-
-            // First index is used to search by Duid.
-            boost::multi_index::hashed_unique<
+    /// @brief Local client.
+    ClientPtr client_;
 
-                // Client ID binary content as a member of the Client object.
-                boost::multi_index::member<
-                    Client, std::vector<uint8_t>, &Client::duid_
-                >
-            >
-        >
-    > ClientContainer;
+    /// @brief Client ID locked by this handler.
+    DuidPtr locked_;
 
-    /// @brief The client container.
-    static ClientContainer clients_;
 };
 
 } // namespace isc