Browse Source

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

JINMEI Tatuya 13 years ago
parent
commit
12649b6a44
2 changed files with 27 additions and 12 deletions
  1. 14 8
      src/lib/util/io/socketsession.cc
  2. 13 4
      src/lib/util/tests/socketsession_unittest.cc

+ 14 - 8
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

+ 13 - 4
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);