]> git.ipfire.org Git - thirdparty/kea.git/commitdiff
[3970] Propagate errors emitted when closing the WatchSocket.
authorMarcin Siodelski <marcin@isc.org>
Mon, 24 Aug 2015 12:25:15 +0000 (14:25 +0200)
committerMarcin Siodelski <marcin@isc.org>
Mon, 24 Aug 2015 12:25:15 +0000 (14:25 +0200)
src/lib/dhcp_ddns/dhcp_ddns_messages.mes
src/lib/dhcp_ddns/ncr_udp.cc
src/lib/dhcp_ddns/ncr_udp.h
src/lib/util/tests/watch_socket_unittests.cc
src/lib/util/watch_socket.cc
src/lib/util/watch_socket.h

index fd0091a36879cfba23d26d2542080e4b88ff6549..b379d477d5acd0e37dffdfac3fd2c0c3b540b713 100644 (file)
@@ -89,14 +89,8 @@ caught in the application's send completion handler.  This is a programmatic
 error that needs to be reported.  Dependent upon the nature of the error the
 client may or may not continue operating normally.
 
-% DHCP_DDNS_WATCH_SINK_CLOSE_ERROR Sink-side watch socket failed to close: %1
+% DHCP_DDNS_UDP_SENDER_WATCH_SOCKET_CLOSE_ERROR watch socket failed to close: %1
 This is an error message that indicates the application was unable to close
-the inbound side of a NCR sender's watch socket.  While technically possible
-this error is highly unlikely to occur and should not impair the application's
-ability to process requests.
-
-% DHCP_DDNS_WATCH_SOURCE_CLOSE_ERROR Source-side watch socket failed to close: %1
-This is an error message that indicates the application was unable to close
-the outbound side of a NCR sender's watch socket.  While technically possible
-this error is highly unlikely to occur and should not impair the application's
-ability to process requests.
+the inbound or outbound side of a NCR sender's watch socket. While technically
+possible the error is highly unlikely to occur and should not impair the
+application's ability to process requests.
index 683147f04bc4fc7c882011f13bb66d5acccc8700..28e665f8d67a8f56786f879fe6ad3d57c37e96c2 100644 (file)
@@ -260,6 +260,7 @@ NameChangeUDPSender::open(isc::asiolink::IOService& io_service) {
 
     send_callback_->setDataSource(server_endpoint_);
 
+    closeWatchSocket();
     watch_socket_.reset(new util::WatchSocket());
 }
 
@@ -288,6 +289,7 @@ NameChangeUDPSender::close() {
 
     socket_.reset();
 
+    closeWatchSocket();
     watch_socket_.reset();
 }
 
@@ -372,7 +374,17 @@ NameChangeUDPSender::ioReady() {
     return (false);
 }
 
-
+void
+NameChangeUDPSender::closeWatchSocket() {
+    if (watch_socket_) {
+        std::string error_string;
+        watch_socket_->closeSocket(error_string);
+        if (!error_string.empty()) {
+            LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_UDP_SENDER_WATCH_SOCKET_CLOSE_ERROR)
+                .arg(error_string);
+        }
+    }
+}
 
 }; // end of isc::dhcp_ddns namespace
 }; // end of isc namespace
index 89646d118c575337b30d1722a51cf70d68f8c2e7..c927f9220ecfe8d091f2fd81c24915d6f18467d3 100644 (file)
@@ -548,6 +548,13 @@ public:
     virtual bool ioReady();
 
 private:
+
+    /// @brief Closes watch socket if the socket is open.
+    ///
+    /// This method closes watch socket if its instance exists and if the
+    /// socket is open. An error message is logged when this operation fails.
+    void closeWatchSocket();
+
     /// @brief IP address from which to send.
     isc::asiolink::IOAddress ip_address_;
 
index 465084c94ebbb7400464bce204caa353019bdc57..a17730c10813ed68e2273a323528842f03ba3aab 100644 (file)
@@ -231,4 +231,27 @@ TEST(WatchSocketTest, badReadOnClear) {
     ASSERT_THROW(watch->markReady(), WatchSocketError);
 }
 
+/// @brief Checks if the socket can be explicitly closed.
+TEST(WatchSocketTest, explicitClose) {
+    WatchSocketPtr watch;
+
+    // Create new instance of the socket.
+    ASSERT_NO_THROW(watch.reset(new WatchSocket()));
+    ASSERT_TRUE(watch);
+
+    // Make sure it has been opened by checking that its descriptor
+    // is valid.
+    EXPECT_NE(watch->getSelectFd(), WatchSocket::SOCKET_NOT_VALID);
+
+    // Close the socket.
+    std::string error_string;
+    ASSERT_TRUE(watch->closeSocket(error_string));
+
+    // Make sure that the descriptor is now invalid which indicates
+    // that the socket has been closed.
+    EXPECT_EQ(WatchSocket::SOCKET_NOT_VALID, watch->getSelectFd());
+    // No errors should be reported.
+    EXPECT_TRUE(error_string.empty());
+}
+
 } // end of anonymous namespace
index a364edcd0486dd1cdb4755eca515ed2ecbe7cdbe..9f496d9e0e7c5bf0d092a0e25133d959068a6df1 100644 (file)
@@ -116,17 +116,19 @@ WatchSocket::clearReady() {
     }
 }
 
-void
-WatchSocket::closeSocket() {
+bool
+WatchSocket::closeSocket(std::string& error_string) {
+    // Clear error string.
+    error_string.clear();
+
     // Close the pipe fds.  Technically a close can fail (hugely unlikely)
     // but there's no recovery for it either.  If one does fail we log it
     // and go on. Plus this is called by the destructor and no one likes
     // destructors that throw.
     if (source_ != SOCKET_NOT_VALID) {
         if (close(source_)) {
-/*            const char* errstr = strerror(errno);
-            LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_WATCH_SOURCE_CLOSE_ERROR)
-                      .arg(errstr); */
+            // An error occured.
+            error_string = strerror(errno);
         }
 
         source_ = SOCKET_NOT_VALID;
@@ -134,13 +136,23 @@ WatchSocket::closeSocket() {
 
     if (sink_ != SOCKET_NOT_VALID) {
         if (close(sink_)) {
-/*            const char* errstr = strerror(errno);
-            LOG_ERROR(dhcp_ddns_logger, DHCP_DDNS_WATCH_SINK_CLOSE_ERROR)
-                      .arg(errstr); */
+            // An error occured.
+            if (error_string.empty()) {
+                error_string = strerror(errno);
+            }
         }
 
         sink_ = SOCKET_NOT_VALID;
     }
+
+    // If any errors have been reported, return false.
+    return (error_string.empty() ? true : false);
+}
+
+void
+WatchSocket::closeSocket() {
+    std::string error_string;
+    closeSocket(error_string);
 }
 
 int
index 0bdca4acf1be28651705bea846fe189b023e3fc5..a2e1ad1ef2a619c573a24a3ee4a6ff921ca980d7 100644 (file)
@@ -114,11 +114,23 @@ public:
     /// pipe.
     int getSelectFd();
 
+    /// @brief Closes the descriptors associated with the socket.
+    ///
+    /// This method is used to close the socket and capture errors that
+    /// may occur during this operation.
+    ///
+    /// @param [out] error_string Holds the error string if closing
+    /// the socket failed. It will hold empty string otherwise.
+    ///
+    /// @return true if the operation was successful, false otherwise.
+    bool closeSocket(std::string& error_string);
+
 private:
+
     /// @brief Closes the descriptors associated with the socket.
     ///
-    /// Used internally in the destructor and if an error occurs marking or
-    /// clearing the socket.
+    /// This method is called by the class destructor and it ignores
+    /// any errors that may occur while closing the sockets.
     void closeSocket();
 
     /// @brief The end of the pipe to which the marker is written