[#4248] Completed last changes from #4225

diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc
index 36ddc78d8cfd8eeee9ab838e055d17ab07c154ca..fc29a964a548b91eb4c108f23694ee58f446e8e3 100644 (file)
--- a/src/hooks/dhcp/high_availability/ha_service.cc
+++ b/src/hooks/dhcp/high_availability/ha_service.cc
@@ -3344,7 +3344,8 @@ HAService::socketReadyHandler(int tcp_native_fd) {
 
 void
 HAService::clientCloseHandler(int tcp_native_fd) {
-    if (tcp_native_fd >= 0) {
+    if ((tcp_native_fd >= 0) &&
+        IfaceMgr::instance().isExternalSocket(tcp_native_fd)) {
         IfaceMgr::instance().deleteExternalSocket(tcp_native_fd);
     }
 }

diff --git a/src/hooks/dhcp/high_availability/tests/ha_test.cc b/src/hooks/dhcp/high_availability/tests/ha_test.cc
index 531cc5441f8bafaaa513e0a727f0a4ab3f64a87e..f65886b92cb2597b0b81d191853f4693bb2a9543 100644 (file)
--- a/src/hooks/dhcp/high_availability/tests/ha_test.cc
+++ b/src/hooks/dhcp/high_availability/tests/ha_test.cc
@@ -119,6 +119,8 @@ HATest::runIOServiceInThread() {
     io_service_->post(std::bind(&HATest::signalServiceRunning, this, std::ref(running),
                                 std::ref(mutex), std::ref(condvar)));
 
+    IfaceMgr::instance().setCheckThreadId(false);
+
     auto f = [](IOServicePtr io_service) {
         try {
             io_service->run();
@@ -127,6 +129,7 @@ HATest::runIOServiceInThread() {
         } catch (...) {
             ADD_FAILURE() << "error while running IOService::run";
         }
+        IfaceMgr::instance().setCheckThreadId(true);
     };
 
     boost::shared_ptr<std::thread>

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 3e6b6856bf316a8c7d1775433f9af43ac6250690..c817cc54692fb82e4b98d288cc9900a02c1ba3b1 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -180,7 +180,7 @@ bool Iface::delSocket(const uint16_t sockfd) {
 IfaceMgr::IfaceMgr()
     : packet_filter_(new PktFilterInet()),
       packet_filter6_(new PktFilterInet6()),
-      test_mode_(false), allow_loopback_(false) {
+      test_mode_(false), check_thread_id_(true), allow_loopback_(false) {
     id_ = std::this_thread::get_id();
 
     // Ensure that PQMs have been created to guarantee we have
@@ -334,7 +334,7 @@ IfaceMgr::addExternalSocket(int socketfd, SocketCallback callback) {
         isc_throw(BadValue, "Attempted to install callback for invalid socket "
                   << socketfd);
     }
-    if (std::this_thread::get_id() != id_) {
+    if (check_thread_id_ && std::this_thread::get_id() != id_) {
         LOG_ERROR(dhcp_logger, DHCP_ADD_EXTERNAL_SOCKET)
                 .arg(socketfd)
                 .arg(std::this_thread::get_id());
@@ -365,7 +365,7 @@ IfaceMgr::deleteExternalSocket(int socketfd) {
 
 void
 IfaceMgr::deleteExternalSocketInternal(int socketfd) {
-    if (std::this_thread::get_id() != id_) {
+    if (check_thread_id_ && std::this_thread::get_id() != id_) {
         LOG_ERROR(dhcp_logger, DHCP_DELETE_EXTERNAL_SOCKET)
                 .arg(socketfd)
                 .arg(std::this_thread::get_id());
@@ -405,7 +405,7 @@ IfaceMgr::isExternalSocketUnusable(int fd) {
 
 void
 IfaceMgr::deleteAllExternalSockets() {
-    if (std::this_thread::get_id() != id_) {
+    if (check_thread_id_ && std::this_thread::get_id() != id_) {
         LOG_ERROR(dhcp_logger, DHCP_DELETE_ALL_EXTERNAL_SOCKETS)
                 .arg(std::this_thread::get_id());
     }

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index c22ef21e405105151ed1622df4213b1c4fd12150..7e35bb58db0e2a7acd60eebb14b837ffed974446 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -29,6 +29,7 @@
 #include <boost/scoped_array.hpp>
 #include <boost/shared_ptr.hpp>
 
+#include <atomic>
 #include <functional>
 #include <list>
 #include <mutex>
@@ -769,6 +770,23 @@ public:
         return (test_mode_);
     }
 
+    /// @brief Get the flag which indicates if thread ID is checked when
+    /// performing operations with external sockets.
+    ///
+    /// @return true if the @c IfaceMgr checks thread ID, false otherwise.
+    bool getCheckThreadId() const {
+        return (check_thread_id_);
+    }
+
+    /// @brief Set the flag which indicates if thread ID is checked when
+    /// performing operations with external sockets.
+    ///
+    /// @param check A flag which indicates if thread ID is checked when
+    /// performing operations with external sockets.
+    void setCheckThreadId(const bool check) {
+        check_thread_id_ = check;
+    }
+
     /// @brief Allows or disallows the loopback interface
     ///
     /// By default the loopback interface is not considered when opening
@@ -1192,6 +1210,17 @@ public:
     ///
     /// @param socketfd socket descriptor
     /// @param callback callback function
+    ///
+    /// @note: all operations an external sockets should be performed
+    /// from the main thread as it does not make sense (and does not
+    /// work as expected) to use an external socket which is in fact
+    /// managed by an I/O service of a thread pool. For instance
+    /// a new external socket is scanned by select or poll only after
+    /// the next call to the receive method. Same argument applies
+    /// when an external socket is deleted and closed...
+    ///
+    /// @note: the callback is called when read available, hup and
+    /// error conditions: the callback is assumed to not block.
     void addExternalSocket(int socketfd, SocketCallback callback);
 
     /// @brief Checks if socket's file description is registered.
@@ -1215,6 +1244,8 @@ public:
 
     /// @brief Deletes all external sockets.
     ///
+    /// @note: to be used only in unit tests.
+    ///
     /// External sockets should be removed from IfaceMgr before being closed
     /// by the external API.
     void deleteAllExternalSockets();
@@ -1663,6 +1694,9 @@ private:
     /// @brief Indicates if the IfaceMgr is in the test mode.
     bool test_mode_;
 
+    /// Check thread ID when performing operations with external sockets.
+    std::atomic<bool> check_thread_id_;
+
     /// @brief Detect callback used to perform actions before system dependent
     /// function calls.
     ///