[#4141] updated unittests

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 1b3bfd9d2bbd22f9803890947cae02e5c1ada4ca..baf3ffa963fd964a01195dcbceaa24036f005dc3 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -1565,6 +1565,7 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ )
     if (!candidate) {
         isc_throw(SocketFDError, "received data over unknown socket");
     }
+
     // Assuming that packet filter is not null, because its modifier checks it.
     return (packet_filter6_->receive(*candidate));
 }
@@ -1666,8 +1667,6 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
                         ex_sock = s;
                         break;
                     }
-                } else {
-
                 }
             }
         }
@@ -1712,55 +1711,55 @@ IfaceMgr::receiveDHCP4Packets() {
 
     for (;;) {
         try {
-        // Check the watch socket.
-        if (dhcp_receiver_->shouldTerminate()) {
-            return;
-        }
-
-        // zero out the errno to be safe.
-        errno = 0;
+            // Check the watch socket.
+            if (dhcp_receiver_->shouldTerminate()) {
+                return;
+            }
 
-        // Select with null timeouts to wait indefinitely an event
-        int result = receiver_fd_event_handler_->waitEvent(0, 0, false);
+            // zero out the errno to be safe.
+            errno = 0;
 
-        // Re-check the watch socket.
-        if (dhcp_receiver_->shouldTerminate()) {
-            return;
-        }
+            // Select with null timeouts to wait indefinitely an event
+            int result = receiver_fd_event_handler_->waitEvent(0, 0, false);
 
-        if (result == 0) {
-            // nothing received?
-            continue;
-        } else if (result < 0) {
-            // This thread should not get signals?
-            if (errno != EINTR) {
-                // Signal the error to receive4.
-                dhcp_receiver_->setError(strerror(errno));
-                // We need to sleep in case of the error condition to
-                // prevent the thread from tight looping when result
-                // gets negative.
-                sleep(1);
+            // Re-check the watch socket.
+            if (dhcp_receiver_->shouldTerminate()) {
+                return;
             }
-            continue;
-        }
 
-        // Let's find out which interface/socket has data.
-        for (const IfacePtr& iface : ifaces_) {
-            for (const SocketInfo& s : iface->getSockets()) {
-                if (!receiver_fd_event_handler_->readReady(s.sockfd_) &&
-                    !receiver_fd_event_handler_->hasError(s.sockfd_)) {
-                    continue;
-                }
-                if (receiver_fd_event_handler_->hasError(s.sockfd_)) {
-                    handleIfaceSocketError(s);
+            if (result == 0) {
+                // nothing received?
+                continue;
+            } else if (result < 0) {
+                // This thread should not get signals?
+                if (errno != EINTR) {
+                    // Signal the error to receive4.
+                    dhcp_receiver_->setError(strerror(errno));
+                    // We need to sleep in case of the error condition to
+                    // prevent the thread from tight looping when result
+                    // gets negative.
+                    sleep(1);
                 }
-                receiveDHCP4Packet(*iface, s);
-                // Can take time so check one more time the watch socket.
-                if (dhcp_receiver_->shouldTerminate()) {
-                    return;
+                continue;
+            }
+
+            // Let's find out which interface/socket has data.
+            for (const IfacePtr& iface : ifaces_) {
+                for (const SocketInfo& s : iface->getSockets()) {
+                    if (!receiver_fd_event_handler_->readReady(s.sockfd_) &&
+                        !receiver_fd_event_handler_->hasError(s.sockfd_)) {
+                        continue;
+                    }
+                    if (receiver_fd_event_handler_->hasError(s.sockfd_)) {
+                        handleIfaceSocketError(s);
+                    }
+                    receiveDHCP4Packet(*iface, s);
+                    // Can take time so check one more time the watch socket.
+                    if (dhcp_receiver_->shouldTerminate()) {
+                        return;
+                    }
                 }
             }
-        }
         } catch (const std::exception& ex) {
             dhcp_receiver_->setError(string(ex.what()) + " error: " + strerror(errno));
         } catch (...) {
@@ -1789,55 +1788,55 @@ IfaceMgr::receiveDHCP6Packets() {
 
     for (;;) {
         try {
-        // Check the watch socket.
-        if (dhcp_receiver_->shouldTerminate()) {
-            return;
-        }
+            // Check the watch socket.
+            if (dhcp_receiver_->shouldTerminate()) {
+                return;
+            }
 
-        // zero out the errno to be safe.
-        errno = 0;
+            // zero out the errno to be safe.
+            errno = 0;
 
-        // Select with null timeouts to wait indefinitely an event
-        int result = receiver_fd_event_handler_->waitEvent(0, 0, false);
+            // Select with null timeouts to wait indefinitely an event
+            int result = receiver_fd_event_handler_->waitEvent(0, 0, false);
 
-        // Re-check the watch socket.
-        if (dhcp_receiver_->shouldTerminate()) {
-            return;
-        }
-
-        if (result == 0) {
-            // nothing received?
-            continue;
-        } else if (result < 0) {
-            // This thread should not get signals?
-            if (errno != EINTR) {
-                // Signal the error to receive6.
-                dhcp_receiver_->setError(strerror(errno));
-                // We need to sleep in case of the error condition to
-                // prevent the thread from tight looping when result
-                // gets negative.
-                sleep(1);
+            // Re-check the watch socket.
+            if (dhcp_receiver_->shouldTerminate()) {
+                return;
             }
-            continue;
-        }
 
-        // Let's find out which interface/socket has data.
-        for (const IfacePtr& iface : ifaces_) {
-            for (const SocketInfo& s : iface->getSockets()) {
-                if (!receiver_fd_event_handler_->readReady(s.sockfd_) &&
-                    !receiver_fd_event_handler_->hasError(s.sockfd_)) {
-                    continue;
-                }
-                if (receiver_fd_event_handler_->hasError(s.sockfd_)) {
-                    handleIfaceSocketError(s);
+            if (result == 0) {
+                // nothing received?
+                continue;
+            } else if (result < 0) {
+                // This thread should not get signals?
+                if (errno != EINTR) {
+                    // Signal the error to receive6.
+                    dhcp_receiver_->setError(strerror(errno));
+                    // We need to sleep in case of the error condition to
+                    // prevent the thread from tight looping when result
+                    // gets negative.
+                    sleep(1);
                 }
-                receiveDHCP6Packet(s);
-                // Can take time so check one more time the watch socket.
-                if (dhcp_receiver_->shouldTerminate()) {
-                    return;
+                continue;
+            }
+
+            // Let's find out which interface/socket has data.
+            for (const IfacePtr& iface : ifaces_) {
+                for (const SocketInfo& s : iface->getSockets()) {
+                    if (!receiver_fd_event_handler_->readReady(s.sockfd_) &&
+                        !receiver_fd_event_handler_->hasError(s.sockfd_)) {
+                        continue;
+                    }
+                    if (receiver_fd_event_handler_->hasError(s.sockfd_)) {
+                        handleIfaceSocketError(s);
+                    }
+                    receiveDHCP6Packet(s);
+                    // Can take time so check one more time the watch socket.
+                    if (dhcp_receiver_->shouldTerminate()) {
+                        return;
+                    }
                 }
             }
-        }
         } catch (const std::exception& ex) {
             dhcp_receiver_->setError(string(ex.what()) + " error: " + strerror(errno));
         } catch (...) {

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index e151d88002a55f1625eb260295ae69b00d56e6b7..497e465bca8ad97c4c22c0a24d4d495fcc8e3950 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -1625,7 +1625,7 @@ private:
     /// @param socketfd socket descriptor
     void deleteExternalSocketInternal(int socketfd);
 
-    /// @brief Handle closed external socket..
+    /// @brief Handle closed external socket.
     ///
     /// @param s The external socket info.
     void handleClosedExternalSocket(SocketCallbackInfo& s);

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index cc2f2617fc887bab0a72d54278909d7a13d2aff7..966e9e22cdea815268554cc12299defdb7e898e2 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -538,6 +538,22 @@ public:
         // we should accept both values as source ports.
         EXPECT_TRUE((rcvPkt->getRemotePort() == 10546) || (rcvPkt->getRemotePort() == 10547));
 
+        // Close the socket. Further we will test if errors are reported
+        // properly on attempt to use closed socket.
+        close(socket2);
+
+        // @todo Closing the socket does NOT cause a read error out of the
+        // receiveDHCP<X>Packets() select.  Apparently this is because the
+        // thread is already inside the select when the socket is closed,
+        // and (at least under Centos 7.5), this does not interrupt the
+        // select.  For now, we'll only test this for direct receive.
+        if (!queue_enabled) {
+            EXPECT_THROW(ifacemgr->receive6(10), SocketFDError);
+        }
+
+        // Verify write fails.
+        EXPECT_THROW(ifacemgr->send(sendPkt), SocketWriteError);
+
         // Stop the thread.  This should be no harm/no foul if we're not
         // queueuing.  Either way, we should not have a thread afterwards.
         ASSERT_NO_THROW(ifacemgr->stopDHCPReceiver());