BIND 10 trac1452, updated. 12649b6a44db756795a3160d1a70c7fe3a6fc9b1 [1452] explicitly set socket recvbuf, too. use blocking read + alarm in some cases as non blocking can make the test with large data.

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Dec 13 17:21:40 UTC 2011


The branch, trac1452 has been updated
       via  12649b6a44db756795a3160d1a70c7fe3a6fc9b1 (commit)
      from  8cd3a3f50336eff26b80af13326daf3df337a234 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 12649b6a44db756795a3160d1a70c7fe3a6fc9b1
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Dec 13 09:20:55 2011 -0800

    [1452] explicitly set socket recvbuf, too.  use blocking read + alarm
    in some cases as non blocking can make the test with large data.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/util/io/socketsession.cc             |   22 ++++++++++++++--------
 src/lib/util/tests/socketsession_unittest.cc |   17 +++++++++++++----
 2 files changed, 27 insertions(+), 12 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/util/io/socketsession.cc b/src/lib/util/io/socketsession.cc
index afee6a3..65303cc 100644
--- a/src/lib/util/io/socketsession.cc
+++ b/src/lib/util/io/socketsession.cc
@@ -59,10 +59,10 @@ struct SocketSessionForwarder::ForwarderImpl {
 const size_t DEFAULT_HEADER_BUFLEN = 2 + sizeof(uint32_t) * 6 +
     sizeof(struct sockaddr_storage) * 2;
 
-// The (default) socket buffer size for the forwarder.  This is chosen to
-// be sufficiently large to store two full-size DNS messages.  We may want to
-// customize this value in future.
-const int FORWARDER_BUFSIZE = (DEFAULT_HEADER_BUFLEN + 65536) * 2;
+// The (default) socket buffer size for the forwarder and receptor.  This is
+// chosen to be sufficiently large to store two full-size DNS messages.  We
+// may want to customize this value in future.
+const int SOCKSESSION_BUFSIZE = (DEFAULT_HEADER_BUFLEN + 65536) * 2;
 
 SocketSessionForwarder::SocketSessionForwarder(const std::string& unix_file) :
     impl_(NULL)
@@ -123,10 +123,10 @@ SocketSessionForwarder::connectToReceptor() {
                   "Failed to make UNIX domain socket non blocking: " <<
                   strerror(errno));
     }
-    if (setsockopt(impl_->fd_, SOL_SOCKET, SO_SNDBUF, &FORWARDER_BUFSIZE,
-                   sizeof(FORWARDER_BUFSIZE)) == -1) {
+    if (setsockopt(impl_->fd_, SOL_SOCKET, SO_SNDBUF, &SOCKSESSION_BUFSIZE,
+                   sizeof(SOCKSESSION_BUFSIZE)) == -1) {
         close();
-        isc_throw(SocketSessionError, "Failed to enlarge send buffer size");
+        isc_throw(SocketSessionError, "Failed to set send buffer size");
     }
     if (connect(impl_->fd_, convertSockAddr(&impl_->sock_un_),
                 impl_->sock_un_len_) == -1) {
@@ -238,7 +238,13 @@ struct SocketSessionReceptor::ReceptorImpl {
                            sa_local_(convertSockAddr(&ss_local_)),
                            sa_remote_(convertSockAddr(&ss_remote_)),
                            header_buf_(DEFAULT_HEADER_BUFLEN), data_buf_(512)
-    {}
+    {
+        if (setsockopt(fd_, SOL_SOCKET, SO_RCVBUF, &SOCKSESSION_BUFSIZE,
+                       sizeof(SOCKSESSION_BUFSIZE)) == -1) {
+            isc_throw(SocketSessionError,
+                      "Failed to set receive buffer size");
+        }
+    }
 
     const int fd_;
     struct sockaddr_storage ss_local_; // placeholder
diff --git a/src/lib/util/tests/socketsession_unittest.cc b/src/lib/util/tests/socketsession_unittest.cc
index d4e264e..489156c 100644
--- a/src/lib/util/tests/socketsession_unittest.cc
+++ b/src/lib/util/tests/socketsession_unittest.cc
@@ -20,6 +20,7 @@
 #include <fcntl.h>
 #include <string.h>
 #include <netdb.h>
+#include <unistd.h>
 
 #include <string>
 #include <utility>
@@ -379,7 +380,11 @@ ForwarderTest::checkPushAndPop(int family, int type, int protocol,
         startListen();
         forwarder_.connectToReceptor();
         accept_sock_.reset(acceptForwarder());
-        setNonBlock(accept_sock_.fd, true);
+
+        // Make sure the socket is *blocking*.  We may pass large data, through
+        // it, and apparently non blocking read could cause some unexpected
+        // partial read on some systems.
+        setNonBlock(accept_sock_.fd, false);
     }
 
     // Then push one socket session via the forwarder.
@@ -387,9 +392,13 @@ ForwarderTest::checkPushAndPop(int family, int type, int protocol,
                     *remote.first, data, data_len);
 
     // Pop the socket session we just pushed from a local receptor, and
-    // check the content
+    // check the content.  Since we do blocking read on the receptor's socket,
+    // we set up an alarm to prevent hangup in case there's a bug that really
+    // makes the blocking happen.
     SocketSessionReceptor receptor(accept_sock_.fd);
+    alarm(1);                   // set up 1-sec timer, an arbitrary choice.
     const SocketSession sock_session = receptor.pop();
+    alarm(0);                   // then cancel it.
     const ScopedSocket passed_sock(sock_session.getSocket());
     EXPECT_LE(0, passed_sock.fd);
     // The passed FD should be different from the original FD
@@ -451,7 +460,7 @@ TEST_F(ForwarderTest, pushAndPop) {
     }
 
     // Pass a UDP/IPv4 session.  This reuses the same pair of forwarder and
-    // acceptor, which should be usable for multiple attempts of passing,
+    // receptor, which should be usable for multiple attempts of passing,
     // regardless of family of the passed session
     const SockAddrInfo sai_local4(getSockAddr("127.0.0.1", TEST_PORT));
     const SockAddrInfo sai_remote4(getSockAddr("192.0.2.2", "5300"));
@@ -544,7 +553,7 @@ TEST_F(ForwarderTest, badPush) {
                                  NULL, sizeof(TEST_DATA)),
                  SocketSessionError);
 
-    // Close the acceptor before push.  It will result in SIGPIPE (should be
+    // Close the receptor 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);




More information about the bind10-changes mailing list