Browse Source

[1452] added consideration for the case where the acceptor dies before push.
we needed to ignore SIGPIPE in this case, so added that setup. also test
of course.

JINMEI Tatuya 13 years ago
parent
commit
f37822dcf5

+ 3 - 0
src/lib/util/io/fd_share.cc

@@ -18,6 +18,7 @@
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/uio.h>
+#include <errno.h>
 #include <stdlib.h>             // for malloc and free
 #include "fd_share.h"
 
@@ -130,7 +131,9 @@ send_fd(const int sock, const int fd) {
     *(int*)CMSG_DATA(cmsg) = fd;
 
     const int ret = sendmsg(sock, &msghdr, 0);
+    const int e = errno;
     free(msghdr.msg_control);
+    errno = e;                  // recover errno in case free() changed it
     return (ret >= 0 ? 0 : FD_COMM_ERROR);
 }
 

+ 2 - 1
src/lib/util/io/fd_share.h

@@ -46,7 +46,8 @@ int recv_fd(const int sock);
  * counterpart of recv_fd().
  *
  * \return FD_COMM_ERROR when there's error sending the socket, FD_OTHER_ERROR
- *     for all other possible errors.
+ *     for all other possible errors.  The global 'errno' variable indicates
+ *     the corresponding system error.
  * \param sock The unix domain socket to send to. Tested and it does not
  *     work with a pipe.
  * \param fd The file descriptor to send. It should work with any valid

+ 9 - 3
src/lib/util/io/socketsession.cc

@@ -19,6 +19,7 @@
 #include <netinet/in.h>
 
 #include <errno.h>
+#include <signal.h>
 #include <stdint.h>
 #include <string.h>
 
@@ -53,6 +54,11 @@ struct SocketSessionForwarder::ForwarderImpl {
 SocketSessionForwarder::SocketSessionForwarder(const std::string& unix_file) :
     impl_(NULL)
 {
+    // We need to filter SIGPIPE for subsequent push().  See the description.
+    if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) {
+        isc_throw(Unexpected, "Failed to filter SIGPIPE: " << strerror(errno));
+    }
+
     ForwarderImpl impl;
     if (sizeof(impl.sock_un_.sun_path) - 1 < unix_file.length()) {
         isc_throw(SocketSessionError,
@@ -139,9 +145,9 @@ SocketSessionForwarder::push(int sock, int family, int sock_type, int protocol,
                   << static_cast<int>(remote_end.sa_family) << " given");
     }
 
-    const int e = send_fd(impl_->fd_, sock);
-    if (e != 0) {
-        isc_throw(SocketSessionError, "FD passing failed: " << e);
+    if (send_fd(impl_->fd_, sock) != 0) {
+        isc_throw(SocketSessionError, "FD passing failed: " <<
+                  strerror(errno));
     }
 
     impl_->buf_.clear();

+ 4 - 0
src/lib/util/io/socketsession.h

@@ -33,7 +33,11 @@ public:
 
 class SocketSessionForwarder : boost::noncopyable {
 public:
+    // Note about SIGPIPE.  Assuming this class is not often instantiated
+    // (so the overhead of signal setting should be marginal) and could also be
+    // instantiated by multiple threads, it always set the filter.
     explicit SocketSessionForwarder(const std::string& unix_file);
+
     ~SocketSessionForwarder();
 
     void connectToReceptor();

+ 7 - 6
src/lib/util/tests/socketsession_unittest.cc

@@ -532,14 +532,15 @@ TEST_F(ForwarderTest, badPush) {
                                  TEST_DATA, sizeof(TEST_DATA)),
                  SocketSessionError);
 
-#if 0
+    // Close the acceptor before push.  It will result in SIGPIPE (should be
+    // ignored) and EPIPE, which will be converted to SocketSessionError.
     const int receptor_fd = acceptForwarder();
     close(receptor_fd);
-    forwarder_.push(1, AF_INET, SOCK_DGRAM, IPPROTO_UDP,
-                    *getSockAddr("192.0.2.1", "53").first,
-                    *getSockAddr("192.0.2.2", "53").first,
-                    TEST_DATA, sizeof(TEST_DATA));
-#endif
+    EXPECT_THROW(forwarder_.push(1, AF_INET, SOCK_DGRAM, IPPROTO_UDP,
+                                 *getSockAddr("192.0.2.1", "53").first,
+                                 *getSockAddr("192.0.2.2", "53").first,
+                                 TEST_DATA, sizeof(TEST_DATA)),
+                 SocketSessionError);
 }
 
 TEST(SocketSession, badValue) {