[#1147] Reorganized v6 client handler

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

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,38 +74,6 @@ private:
     /// @brief The type of shared pointers to clients.
     typedef boost::shared_ptr<Client> ClientPtr;
 
-    /// @brief Local client.
-    ClientPtr client_;
-
-    /// @brief Client ID locked by this handler.
-    DuidPtr locked_;
-
-    /// @brief Mutex to protect the client container.
-    ///
-    /// The mutex is used only by public methods for guards.
-    static std::mutex mutex_;
-
-    /// @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.
-    static ClientPtr lookup(const DuidPtr& duid);
-
-    /// @brief Acquire a client.
-    ///
-    /// The mutex must be held by the caller.
-    void lock();
-
-    /// @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 caller.
-    void unLock();
-
     /// @brief The type of the client container.
     typedef boost::multi_index_container<
 
@@ -147,8 +94,84 @@ private:
         >
     > 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 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.
+    void lock();
+
+    /// @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 Local client.
+    ClientPtr client_;
+
+    /// @brief Client ID locked by this handler.
+    DuidPtr locked_;
+
 };
 
 } // namespace isc