made asiolink-tests clang++ friendly:

- avoid relying on ASIO details in the main test (this is good anyway in
   terms of the original intent of asio link)
 - move internal-dependent tests to internal/tests with a workaround compiler
   flag

Also fixed a potentially common bug: use blocking recv() in recvUDP() with a fail safe timeout.  When compiled with clang++ this test sometimes failed due to the failure of recv(), and after looking at the code closely I found it possible to happen for other environments.


git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac448@4011 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/asiolink/Makefile.am

@@ -1,4 +1,4 @@
-SUBDIRS = . tests
+SUBDIRS = . tests internal
 
 AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += $(BOOST_INCLUDES)

src/lib/asiolink/internal/tests/run_unittests.cc

@@ -0,0 +1,21 @@
+// Copyright (C) 2010  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <gtest/gtest.h>
+
+int
+main(int argc, char* argv[]) {
+    ::testing::InitGoogleTest(&argc, argv);
+    return (RUN_ALL_TESTS());
+}

src/lib/asiolink/tests/Makefile.am

@@ -18,7 +18,6 @@ TESTS += run_unittests
 run_unittests_SOURCES = $(top_srcdir)/src/lib/dns/tests/unittest_util.h
 run_unittests_SOURCES += $(top_srcdir)/src/lib/dns/tests/unittest_util.cc
 run_unittests_SOURCES += asiolink_unittest.cc
-run_unittests_SOURCES += udpdns_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS = $(AM_LDFLAGS) $(GTEST_LDFLAGS)

src/lib/asiolink/tests/asiolink_unittest.cc

@@ -17,6 +17,9 @@
 
 #include <config.h>
 
+#include <sys/socket.h>
+#include <sys/time.h>
+
 #include <string.h>
 
 #include <boost/lexical_cast.hpp>
@@ -31,19 +34,21 @@
 #include <dns/buffer.h>
 #include <dns/message.h>
 
+// IMPORTANT: We shouldn't directly use ASIO definitions in this test.
+// In particular, we must not include asio.hpp in this file.
+// The asiolink module is primarily intended to be a wrapper that hide the
+// details of the underlying implementations.  We need to test the wrapper
+// level behaviors.  In addition, some compilers reject to compile this file
+// if we include asio.hpp unless we specify a special compiler option.
+// If we need to test something at the level of underlying ASIO and need
+// their definition, that test should go to asiolink/internal/tests.
 #include <asiolink/asiolink.h>
 #include <asiolink/iosocket.h>
-#include <asiolink/internal/tcpdns.h>
-#include <asiolink/internal/udpdns.h>
-
-#include <asio.hpp>
 
 using isc::UnitTestUtil;
 using namespace std;
 using namespace asiolink;
 using namespace isc::dns;
-using namespace asio;
-using asio::ip::udp;
 
 namespace {
 const char* const TEST_SERVER_PORT = "53535";
@@ -326,10 +331,20 @@ protected:
         // ... and this one will block until the send has completed
         io_service_->run_one();
 
-        // Now we attempt to recv() whatever was sent
+        // Now we attempt to recv() whatever was sent.
-        const int ret = recv(sock_, buffer, size, MSG_DONTWAIT);
+        // XXX: there's no guarantee the receiving socket can immediately get
+        // the packet.  Normally we can perform blocking recv to wait for it,
+        // but in theory it's even possible that the packet is lost.
+        // In order to prevent the test from hanging in such a worst case
+        // we add an ad hoc timeout.
+        const struct timeval timeo = { 10, 0 };
+        if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo,
+                       sizeof(timeo))) {
+            isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno));
+        }
+        const int ret = recv(sock_, buffer, size, 0);
         if (ret < 0) {
-            isc_throw(IOError, "recvfrom failed");
+            isc_throw(IOError, "recvfrom failed: " << strerror(errno));
         }
         
         // Pass the message size back via the size parameter
@@ -407,8 +422,7 @@ protected:
     // has completed.
     class MockServer : public DNSServer {
     public:
-        explicit MockServer(asio::io_service& io_service,
+        explicit MockServer(IOService& io_service,
-                            const asio::ip::address& addr, const uint16_t port,
                             SimpleCallback* checkin = NULL,
                             DNSLookup* lookup = NULL,
                             DNSAnswer* answer = NULL) :
@@ -422,9 +436,7 @@ protected:
                         size_t length = 0)
         {}
 
-        void resume(const bool done) {
+        void resume(const bool) { // in our test this shouldn't be called
-            done_ = done;
-            io_.post(*this);
         }
 
         DNSServer* clone() {
@@ -439,7 +451,7 @@ protected:
         }
 
     protected:
-        asio::io_service& io_;
+        IOService& io_;
         bool done_;
 
     private:
@@ -458,8 +470,8 @@ protected:
     // This version of mock server just stops the io_service when it is resumed
     class MockServerStop : public MockServer {
         public:
-            explicit MockServerStop(asio::io_service& io_service, bool* done) :
+            explicit MockServerStop(IOService& io_service, bool* done) :
-                MockServer(io_service, asio::ip::address(), 0),
+                MockServer(io_service),
                 done_(done)
             {}
 
@@ -507,7 +519,6 @@ protected:
     string callback_address_;
     vector<uint8_t> callback_data_;
     int sock_;
-private:
     struct addrinfo* res_;
 };
 
@@ -636,14 +647,12 @@ TEST_F(ASIOLinkTest, recursiveSetupV6) {
 // full code coverage including error cases.
 TEST_F(ASIOLinkTest, recursiveSend) {
     setDNSService(true, false);
-    asio::io_service& io = io_service_->get_io_service();
 
     // Note: We use the test prot plus one to ensure we aren't binding
     // to the same port as the actual server
     uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
-    asio::ip::address addr = asio::ip::address::from_string(TEST_IPV4_ADDR);
 
-    MockServer server(io, addr, port, NULL, NULL, NULL);
+    MockServer server(*io_service_);
     RecursiveQuery rq(*dns_service_, singleAddress(TEST_IPV4_ADDR, port));
 
     Question q(Name("example.com"), RRClass::IN(), RRType::TXT());
@@ -652,7 +661,7 @@ TEST_F(ASIOLinkTest, recursiveSend) {
 
     char data[4096];
     size_t size = sizeof(data);
-    EXPECT_NO_THROW(recvUDP(AF_INET, data, size));
+    ASSERT_NO_THROW(recvUDP(AF_INET, data, size));
 
     Message m(Message::PARSE);
     InputBuffer ibuf(data, size);
@@ -668,34 +677,27 @@ TEST_F(ASIOLinkTest, recursiveSend) {
     EXPECT_EQ(q.getClass(), q2->getClass());
 }
 
-void
-receive_and_inc(udp::socket* socket, int* num) {
-    (*num) ++;
-    static char inbuff[512];
-    socket->async_receive(asio::buffer(inbuff, 512),
-        boost::bind(receive_and_inc, socket, num));
-}
-
 // Test it tries the correct amount of times before giving up
 TEST_F(ASIOLinkTest, recursiveTimeout) {
     // Prepare the service (we do not use the common setup, we do not answer
     setDNSService();
-    asio::io_service& service = io_service_->get_io_service();
 
     // Prepare the socket
-    uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
+    res_ = resolveAddress(AF_INET, IPPROTO_UDP, true);
-    udp::socket socket(service, udp::v4());
+    sock_ = socket(res_->ai_family, res_->ai_socktype, res_->ai_protocol);
-    socket.set_option(socket_base::reuse_address(true));
+    if (sock_ < 0) {
-    socket.bind(udp::endpoint(ip::address::from_string(TEST_IPV4_ADDR), port));
+        isc_throw(IOError, "failed to open test socket");
-    // And count the answers
+    }
-    int num = -1; // One is counted before the receipt of the first one
+    if (bind(sock_, res_->ai_addr, res_->ai_addrlen) < 0) {
-    receive_and_inc(&socket, &num);
+        isc_throw(IOError, "failed to bind test socket");
+    }
 
     // Prepare the server
     bool done(true);
-    MockServerStop server(service, &done);
+    MockServerStop server(*io_service_, &done);
 
     // Do the answer
+    const uint16_t port = boost::lexical_cast<uint16_t>(TEST_CLIENT_PORT);
     RecursiveQuery query(*dns_service_, singleAddress(TEST_IPV4_ADDR, port),
         10, 2);
     Question question(Name("example.net"), RRClass::IN(), RRType::A());
@@ -703,7 +705,22 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
     query.sendQuery(question, buffer, &server);
 
     // Run the test
-    service.run();
+    io_service_->run();
+
+    // Read up to 3 packets.  Use some ad hoc timeout to prevent an infinite
+    // block (see also recvUDP()).
+    const struct timeval timeo = { 10, 0 };
+    if (setsockopt(sock_, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo))) {
+        isc_throw(IOError, "set RCVTIMEO failed: " << strerror(errno));
+    }
+    int num = 0;
+    do {
+        char inbuff[512];
+        if (recv(sock_, inbuff, sizeof(inbuff), 0) < 0) {
+            num = -1;
+            break;
+        }
+    } while (++num < 3);
 
     // The query should fail
     EXPECT_FALSE(done);