BIND 10 master, updated. 8580400e9333574d49de88aaf34dac87bd270020 [master] Merge branch 'trac2349'

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Oct 18 12:33:15 UTC 2012


The branch, master has been updated
       via  8580400e9333574d49de88aaf34dac87bd270020 (commit)
       via  6fca5f367bef49f3c3002ee6abb4c5c19ab971d9 (commit)
       via  5735b337b8e78502bd54cba748bb622b150f6667 (commit)
       via  b57f65e686286f211ef1b177f69a81fa5baf3021 (commit)
       via  6920774dd67be1221c379e9d5161f0cc932fbd73 (commit)
       via  3a73fff8f44f18188c1f810f3f5395e6ea107a7d (commit)
       via  9c2e94bb8ae9368aac3e0991dbebe3738ee914d7 (commit)
       via  95a47e7f0ac9e87d3d0c6c51c07e4761d0a50932 (commit)
       via  26483ea27726d4c1097485196a0908c565478188 (commit)
       via  030aef19982a37a8b81376adb15864097b208ff8 (commit)
      from  110a33e6c237d71d2efa42d27ae2434b35ddcaa3 (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 8580400e9333574d49de88aaf34dac87bd270020
Merge: 110a33e 6fca5f3
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Oct 18 14:19:35 2012 +0200

    [master] Merge branch 'trac2349'
    
    Conflicts:
    	src/lib/util/threads/tests/lock_unittest.cc

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

Summary of changes:
 configure.ac                                       |    7 ++
 src/lib/log/logger.h                               |   10 +--
 src/lib/log/tests/log_formatter_unittest.cc        |   14 ++--
 src/lib/log/tests/logger_unittest.cc               |   21 +++---
 .../log/tests/message_initializer_2_unittest.cc    |   11 ++--
 src/lib/resolve/recursive_query.cc                 |   47 ++++++++------
 src/lib/resolve/recursive_query.h                  |   45 +++++++++++--
 src/lib/resolve/tests/recursive_query_unittest.cc  |   48 ++++++++------
 .../resolve/tests/recursive_query_unittest_2.cc    |   22 ++++++-
 .../resolve/tests/recursive_query_unittest_3.cc    |   21 +++++-
 src/lib/server_common/tests/portconfig_unittest.cc |   67 +++++++++++---------
 src/lib/util/tests/buffer_unittest.cc              |   23 ++++---
 src/lib/util/threads/tests/lock_unittest.cc        |   22 ++++---
 src/lib/util/unittests/Makefile.am                 |    1 +
 .../unittests/{resource.cc => check_valgrind.cc}   |   28 ++++----
 src/lib/util/unittests/check_valgrind.h            |   50 +++++++++++++++
 16 files changed, 297 insertions(+), 140 deletions(-)
 copy src/lib/util/unittests/{resource.cc => check_valgrind.cc} (67%)
 create mode 100644 src/lib/util/unittests/check_valgrind.h

-----------------------------------------------------------------------
diff --git a/configure.ac b/configure.ac
index 72c7be2..09de67f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1067,6 +1067,13 @@ AM_COND_IF([ENABLE_LOGGER_CHECKS], [AC_DEFINE([ENABLE_LOGGER_CHECKS], [1], [Chec
 AC_PATH_PROG(VALGRIND, valgrind, no)
 AM_CONDITIONAL(HAVE_VALGRIND, test "x$VALGRIND" != "xno")
 
+# Also check for valgrind headers
+# We could consider adding them to the source code tree, as this
+# is the encouraged method of using them; they are BSD-licensed.
+# However, until we find that this is a problem, we just use
+# the system-provided ones, if available
+AC_CHECK_HEADERS(valgrind/valgrind.h, [AC_DEFINE([HAVE_VALGRIND_HEADERS], [1], [Check valgrind headers])])
+
 found_valgrind="not found"
 if test "x$VALGRIND" != "xno"; then
    found_valgrind="found"
diff --git a/src/lib/log/logger.h b/src/lib/log/logger.h
index 6405488..4291eae 100644
--- a/src/lib/log/logger.h
+++ b/src/lib/log/logger.h
@@ -33,9 +33,9 @@ namespace log {
 /// \page LoggingApi Logging API
 /// \section LoggingApiOverview Overview
 /// BIND 10 logging uses the concepts of the widely-used Java logging
-/// package log4j (http://logging.apache.log/log4j), albeit implemented 
+/// package log4j (http://logging.apache.log/log4j), albeit implemented
 /// in C++ using an open-source port.  Features of the system are:
-/// 
+///
 /// - Within the code objects - known as loggers - can be created and
 /// used to log messages.  These loggers have names; those with the
 /// same name share characteristics (such as output destination).
@@ -50,7 +50,7 @@ namespace log {
 /// message is logged, it is output only if it is logged at a level equal
 /// to the logger severity level or greater, e.g. if the logger's severity
 /// is WARN, only messages logged at WARN, ERROR or FATAL will be output.
-/// 
+///
 /// \section LoggingApiLoggerNames BIND 10 Logger Names
 /// Within BIND 10, the root logger root logger is given the name of the
 /// program (via the stand-alone function setRootLoggerName()). Other loggers
@@ -58,7 +58,7 @@ namespace log {
 /// This name appears in logging output, allowing users to identify both
 /// the BIND 10 program and the component within the program that generated
 /// the message.
-/// 
+///
 /// When creating a logger, the abbreviated name "<sublogger>" can be used;
 /// the program name will be prepended to it when the logger is created.
 /// In this way, individual libraries can have their own loggers without
@@ -66,7 +66,7 @@ namespace log {
 /// - The origin of the message will be clearly identified.
 /// - The same component can have different options (e.g. logging severity)
 /// in different programs at the same time.
-/// 
+///
 /// \section LoggingApiLoggingMessages Logging Messages
 /// Instead of embedding the text of messages within the code, each message
 /// is referred to using a symbolic name.  The logging code uses this name as
diff --git a/src/lib/log/tests/log_formatter_unittest.cc b/src/lib/log/tests/log_formatter_unittest.cc
index 435b200..f0a9a59 100644
--- a/src/lib/log/tests/log_formatter_unittest.cc
+++ b/src/lib/log/tests/log_formatter_unittest.cc
@@ -16,6 +16,7 @@
 #include <gtest/gtest.h>
 
 #include <util/unittests/resource.h>
+#include <util/unittests/check_valgrind.h>
 
 #include <log/log_formatter.h>
 #include <log/logger_level.h>
@@ -119,12 +120,13 @@ TEST_F(FormatterTest, mismatchedPlaceholders) {
     // Likewise, if there's a redundant placeholder (or missing argument), the
     // check detects it and aborts the program.  Due to the restriction of the
     // current implementation, it doesn't throw.
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
-        Formatter(isc::log::INFO, s("Too many arguments in %1 %2"), this).
-            arg("only one");
-    }, ".*");
-
+    if (!isc::util::unittests::runningOnValgrind()) {
+        EXPECT_DEATH({
+            isc::util::unittests::dontCreateCoreDumps();
+            Formatter(isc::log::INFO, s("Too many arguments in %1 %2"), this).
+                arg("only one");
+        }, ".*");
+    }
 #endif /* EXPECT_DEATH */
     // Mixed case of above two: the exception will be thrown due to the missing
     // placeholder. The other check is disabled due to that.
diff --git a/src/lib/log/tests/logger_unittest.cc b/src/lib/log/tests/logger_unittest.cc
index a9330a9..7b62d79 100644
--- a/src/lib/log/tests/logger_unittest.cc
+++ b/src/lib/log/tests/logger_unittest.cc
@@ -11,13 +11,10 @@
 // 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 <iostream>
-#include <string>
-
 #include <gtest/gtest.h>
 
 #include <util/unittests/resource.h>
+#include <util/unittests/check_valgrind.h>
 
 #include <log/logger.h>
 #include <log/logger_manager.h>
@@ -27,6 +24,9 @@
 
 #include <util/interprocess_sync_file.h>
 
+#include <iostream>
+#include <string>
+
 using namespace isc;
 using namespace isc::log;
 using namespace std;
@@ -356,7 +356,6 @@ TEST_F(LoggerTest, IsDebugEnabledLevel) {
 
 // Check that if a logger name is too long, it triggers the appropriate
 // assertion.
-
 TEST_F(LoggerTest, LoggerNameLength) {
     // The following statements should just declare a logger and nothing
     // should happen.
@@ -374,12 +373,14 @@ TEST_F(LoggerTest, LoggerNameLength) {
     // Too long a logger name should trigger an assertion failure.
     // Note that we just check that it dies - we don't check what message is
     // output.
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
+    if (!isc::util::unittests::runningOnValgrind()) {
+        EXPECT_DEATH({
+            isc::util::unittests::dontCreateCoreDumps();
 
-        string ok3(Logger::MAX_LOGGER_NAME_SIZE + 1, 'x');
-        Logger l3(ok3.c_str());
-    }, ".*");
+            string ok3(Logger::MAX_LOGGER_NAME_SIZE + 1, 'x');
+            Logger l3(ok3.c_str());
+        }, ".*");
+    }
 #endif
 }
 
diff --git a/src/lib/log/tests/message_initializer_2_unittest.cc b/src/lib/log/tests/message_initializer_2_unittest.cc
index b479eee..8cc78fa 100644
--- a/src/lib/log/tests/message_initializer_2_unittest.cc
+++ b/src/lib/log/tests/message_initializer_2_unittest.cc
@@ -16,6 +16,7 @@
 #include <gtest/gtest.h>
 
 #include <util/unittests/resource.h>
+#include <util/unittests/check_valgrind.h>
 
 using namespace isc::log;
 
@@ -43,10 +44,12 @@ TEST(MessageInitializerTest2, MessageLoadTest) {
     // test for its presence and bypass the test if not available.
 #ifdef EXPECT_DEATH
     // Adding one more should take us over the limit.
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
+    if (!isc::util::unittests::runningOnValgrind()) {
+        EXPECT_DEATH({
+            isc::util::unittests::dontCreateCoreDumps();
 
-        MessageInitializer initializer2(values);
-      }, ".*");
+            MessageInitializer initializer2(values);
+          }, ".*");
+    }
 #endif
 }
diff --git a/src/lib/resolve/recursive_query.cc b/src/lib/resolve/recursive_query.cc
index c5b280f..7eae6fe 100644
--- a/src/lib/resolve/recursive_query.cc
+++ b/src/lib/resolve/recursive_query.cc
@@ -122,8 +122,6 @@ deepestDelegation(Name name, RRClass rrclass,
     return (".");
 }
 
-typedef std::vector<std::pair<std::string, uint16_t> > AddressVector;
-
 // Here we do not use the typedef above, as the SunStudio compiler
 // mishandles this in its name mangling, and wouldn't compile.
 // We can probably use a typedef, but need to move it to a central
@@ -161,7 +159,6 @@ RecursiveQuery::setRttRecorder(boost::shared_ptr<RttRecorder>& recorder) {
 }
 
 namespace {
-
 typedef std::pair<std::string, uint16_t> addr_t;
 
 /*
@@ -171,7 +168,7 @@ typedef std::pair<std::string, uint16_t> addr_t;
  *
  * Used by RecursiveQuery::sendQuery.
  */
-class RunningQuery : public IOFetch::Callback {
+class RunningQuery : public IOFetch::Callback, public AbstractRunningQuery {
 
 class ResolverNSASCallback : public isc::nsas::AddressRequestCallback {
 public:
@@ -730,6 +727,8 @@ public:
         doLookup();
     }
 
+    virtual ~RunningQuery() {};
+
     // called if we have a lookup timeout; if our callback has
     // not been called, call it now. Then stop.
     void lookupTimeout() {
@@ -894,11 +893,13 @@ public:
     // Clear the answer parts of answer_message, and set the rcode
     // to servfail
     void makeSERVFAIL() {
-        isc::resolve::makeErrorMessage(answer_message_, Rcode::SERVFAIL());
+        if (answer_message_) {
+            isc::resolve::makeErrorMessage(answer_message_, Rcode::SERVFAIL());
+        }
     }
 };
 
-class ForwardQuery : public IOFetch::Callback {
+class ForwardQuery : public IOFetch::Callback, public AbstractRunningQuery {
 private:
     // The io service to handle async calls
     IOService& io_;
@@ -999,6 +1000,8 @@ public:
         send();
     }
 
+    virtual ~ForwardQuery() {};
+
     virtual void lookupTimeout() {
         if (!callback_called_) {
             makeSERVFAIL();
@@ -1076,7 +1079,7 @@ public:
 
 }
 
-void
+AbstractRunningQuery*
 RecursiveQuery::resolve(const QuestionPtr& question,
     const isc::resolve::ResolverInterface::CallbackPtr callback)
 {
@@ -1119,16 +1122,17 @@ RecursiveQuery::resolve(const QuestionPtr& question,
             // delete itself when it is done
             LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RECQ_CACHE_NO_FIND)
                       .arg(questionText(*question)).arg(1);
-            new RunningQuery(io, *question, answer_message,
-                             test_server_, buffer, callback,
-                             query_timeout_, client_timeout_,
-                             lookup_timeout_, retries_, nsas_,
-                             cache_, rtt_recorder_);
+            return (new RunningQuery(io, *question, answer_message,
+                                     test_server_, buffer, callback,
+                                     query_timeout_, client_timeout_,
+                                     lookup_timeout_, retries_, nsas_,
+                                     cache_, rtt_recorder_));
         }
     }
+    return (NULL);
 }
 
-void
+AbstractRunningQuery*
 RecursiveQuery::resolve(const Question& question,
                         MessagePtr answer_message,
                         OutputBufferPtr buffer,
@@ -1181,15 +1185,16 @@ RecursiveQuery::resolve(const Question& question,
             // delete itself when it is done
             LOG_DEBUG(isc::resolve::logger, RESLIB_DBG_TRACE, RESLIB_RECQ_CACHE_NO_FIND)
                       .arg(questionText(question)).arg(2);
-            new RunningQuery(io, question, answer_message,
-                             test_server_, buffer, crs, query_timeout_,
-                             client_timeout_, lookup_timeout_, retries_,
-                             nsas_, cache_, rtt_recorder_);
+            return (new RunningQuery(io, question, answer_message,
+                                     test_server_, buffer, crs, query_timeout_,
+                                     client_timeout_, lookup_timeout_, retries_,
+                                     nsas_, cache_, rtt_recorder_));
         }
     }
+    return (NULL);
 }
 
-void
+AbstractRunningQuery*
 RecursiveQuery::forward(ConstMessagePtr query_message,
     MessagePtr answer_message,
     OutputBufferPtr buffer,
@@ -1215,9 +1220,9 @@ RecursiveQuery::forward(ConstMessagePtr query_message,
     // everything throught without interpretation, except
     // QID, port number. The response will not be cached.
     // It will delete itself when it is done
-    new ForwardQuery(io, query_message, answer_message,
-                      upstream_, buffer, callback, query_timeout_,
-                      client_timeout_, lookup_timeout_);
+    return (new ForwardQuery(io, query_message, answer_message,
+                             upstream_, buffer, callback, query_timeout_,
+                             client_timeout_, lookup_timeout_));
 }
 
 } // namespace asiodns
diff --git a/src/lib/resolve/recursive_query.h b/src/lib/resolve/recursive_query.h
index a819a94..7cda837 100644
--- a/src/lib/resolve/recursive_query.h
+++ b/src/lib/resolve/recursive_query.h
@@ -52,6 +52,26 @@ private:
     std::vector<uint32_t>   rtt_;   ///< Stored round-trip times
 };
 
+typedef std::vector<std::pair<std::string, uint16_t> > AddressVector;
+
+/// \brief A Running query
+///
+/// This base class represents an active running query object;
+/// i.e. an outstanding query to an authoritative name server or
+/// upstream server (when running in forwarder mode).
+///
+/// It can not be instantiated directly, but is created by
+/// RecursiveQuery::resolve() and RecursiveQuery::forward().
+///
+/// Its only public method is its destructor, and that should in theory
+/// not be called either except in some unit tests. Instances should
+/// delete themselves when the query is finished.
+class AbstractRunningQuery {
+protected:
+    AbstractRunningQuery() {};
+public:
+    virtual ~AbstractRunningQuery() {};
+};
 
 /// \brief Recursive Query
 ///
@@ -126,8 +146,8 @@ public:
     /// \param question The question being answered <qname/qclass/qtype>
     /// \param callback Callback object. See
     ///        \c ResolverInterface::Callback for more information
-    void resolve(const isc::dns::QuestionPtr& question,
-                 const isc::resolve::ResolverInterface::CallbackPtr callback);
+    AbstractRunningQuery* resolve(const isc::dns::QuestionPtr& question,
+        const isc::resolve::ResolverInterface::CallbackPtr callback);
 
 
     /// \brief Initiates resolving for the given question.
@@ -142,10 +162,17 @@ public:
     /// \param buffer An output buffer into which the intermediate responses will
     ///        be copied.
     /// \param server A pointer to the \c DNSServer object handling the client
-    void resolve(const isc::dns::Question& question,
-                 isc::dns::MessagePtr answer_message,
-                 isc::util::OutputBufferPtr buffer,
-                 DNSServer* server);
+    /// \return A pointer to the active AbstractRunningQuery object
+    ///         created by this call (if any); this object should delete
+    ///         itself in normal circumstances, and can normally be ignored
+    ///         by the caller, but a pointer is returned for use-cases
+    ///         such as unit tests.
+    ///         Returns NULL if the data was found internally and no actual
+    ///         query was sent.
+    AbstractRunningQuery* resolve(const isc::dns::Question& question,
+                          isc::dns::MessagePtr answer_message,
+                          isc::util::OutputBufferPtr buffer,
+                          DNSServer* server);
 
     /// \brief Initiates forwarding for the given query.
     ///
@@ -158,7 +185,11 @@ public:
     /// \param server Server object that handles receipt and processing of the
     ///               received messages.
     /// \param callback callback object
-    void forward(isc::dns::ConstMessagePtr query_message,
+    /// \return A pointer to the active ForwardQuery created by this call;
+    ///         this object should delete itself in normal circumstances,
+    ///         and can normally be ignored by the caller, but a pointer
+    ///         is returned for use-cases such as unit tests.
+    AbstractRunningQuery* forward(isc::dns::ConstMessagePtr query_message,
                  isc::dns::MessagePtr answer_message,
                  isc::util::OutputBufferPtr buffer,
                  DNSServer* server,
diff --git a/src/lib/resolve/tests/recursive_query_unittest.cc b/src/lib/resolve/tests/recursive_query_unittest.cc
index 4513458..e5e46e1 100644
--- a/src/lib/resolve/tests/recursive_query_unittest.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest.cc
@@ -173,6 +173,9 @@ protected:
         // It would delete itself, but after the io_service_, which could
         // segfailt in case there were unhandled requests
         resolver_.reset();
+        // In a similar note, we wait until the resolver has been cleaned up
+        // until deleting and active test running_query_
+        delete running_query_;
     }
 
     void SetUp() {
@@ -507,11 +510,13 @@ protected:
     vector<uint8_t> callback_data_;
     ScopedSocket sock_;
     boost::shared_ptr<isc::util::unittests::TestResolver> resolver_;
+    AbstractRunningQuery* running_query_;
 };
 
 RecursiveQueryTest::RecursiveQueryTest() :
     dns_service_(NULL), callback_(NULL), callback_protocol_(0),
-    callback_native_(-1), resolver_(new isc::util::unittests::TestResolver())
+    callback_native_(-1), resolver_(new isc::util::unittests::TestResolver()),
+    running_query_(NULL)
 {
     nsas_.reset(new isc::nsas::NameserverAddressStore(resolver_));
 }
@@ -652,12 +657,12 @@ TEST_F(RecursiveQueryTest, forwarderSend) {
                       singleAddress(TEST_IPV4_ADDR, port));
 
     Question q(Name("example.com"), RRClass::IN(), RRType::TXT());
-    Message query_message(Message::RENDER);
-    isc::resolve::initResponseMessage(q, query_message);
+    MessagePtr query_message(new Message(Message::RENDER));
+    isc::resolve::initResponseMessage(q, *query_message);
 
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.forward(ConstMessagePtr(&query_message), answer, buffer, &server);
+    running_query_ = rq.forward(query_message, answer, buffer, &server);
 
     char data[4096];
     size_t size = sizeof(data);
@@ -749,11 +754,11 @@ TEST_F(RecursiveQueryTest, forwardQueryTimeout) {
     Question question(Name("example.net"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    Message query_message(Message::RENDER);
-    isc::resolve::initResponseMessage(question, query_message);
+    MessagePtr query_message(new Message(Message::RENDER));
+    isc::resolve::initResponseMessage(question, *query_message);
 
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
-    query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
+    running_query_ = query.forward(query_message, answer, buffer, &server, callback);
     // Run the test
     io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -784,11 +789,11 @@ TEST_F(RecursiveQueryTest, forwardClientTimeout) {
                          1000, 10, 4000, 4);
     Question q(Name("example.net"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
-    Message query_message(Message::RENDER);
-    isc::resolve::initResponseMessage(q, query_message);
+    MessagePtr query_message(new Message(Message::RENDER));
+    isc::resolve::initResponseMessage(q, *query_message);
 
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
-    query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
+    running_query_ = query.forward(query_message, answer, buffer, &server, callback);
     // Run the test
     io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -819,11 +824,12 @@ TEST_F(RecursiveQueryTest, forwardLookupTimeout) {
     Question question(Name("example.net"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
 
-    Message query_message(Message::RENDER);
-    isc::resolve::initResponseMessage(question, query_message);
+    //Message query_message(Message::RENDER);
+    MessagePtr query_message(new Message(Message::RENDER));
+    isc::resolve::initResponseMessage(question, *query_message);
 
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
-    query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
+    running_query_ = query.forward(query_message, answer, buffer, &server, callback);
     // Run the test
     io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -854,11 +860,11 @@ TEST_F(RecursiveQueryTest, lowtimeouts) {
     Question question(Name("example.net"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
 
-    Message query_message(Message::RENDER);
-    isc::resolve::initResponseMessage(question, query_message);
+    MessagePtr query_message(new Message(Message::RENDER));
+    isc::resolve::initResponseMessage(question, *query_message);
 
     boost::shared_ptr<MockResolverCallback> callback(new MockResolverCallback(&server));
-    query.forward(ConstMessagePtr(&query_message), answer, buffer, &server, callback);
+    running_query_ = query.forward(query_message, answer, buffer, &server, callback);
     // Run the test
     io_service_.run();
     EXPECT_EQ(callback->result, MockResolverCallback::FAILURE);
@@ -882,7 +888,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendOk) {
     Question q(Name("www.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.resolve(q, answer, buffer, &server);
+    running_query_ = rq.resolve(q, answer, buffer, &server);
     io_service_.run();
 
     // Check that the answer we got matches the one we wanted
@@ -908,7 +914,7 @@ TEST_F(RecursiveQueryTest, DISABLED_recursiveSendNXDOMAIN) {
     Question q(Name("wwwdoesnotexist.isc.org"), RRClass::IN(), RRType::A());
     OutputBufferPtr buffer(new OutputBuffer(0));
     MessagePtr answer(new Message(Message::RENDER));
-    rq.resolve(q, answer, buffer, &server);
+    running_query_ = rq.resolve(q, answer, buffer, &server);
     io_service_.run();
 
     // Check that the answer we got matches the one we wanted
@@ -972,9 +978,9 @@ TEST_F(RecursiveQueryTest, CachedNS) {
     // Prepare the recursive query
     vector<pair<string, uint16_t> > roots;
     roots.push_back(pair<string, uint16_t>("192.0.2.2", 53));
-
+    vector<pair<string, uint16_t> > upstream;
     RecursiveQuery rq(*dns_service_, *nsas_, cache_,
-                      vector<pair<string, uint16_t> >(), roots);
+                      upstream, roots);
     // Ask a question at the bottom. It should not use the lower NS, because
     // it would lead to a loop in NS. But it can use the nsUpper one, it has
     // an IP address and we can avoid asking root.
@@ -984,7 +990,7 @@ TEST_F(RecursiveQueryTest, CachedNS) {
     MessagePtr answer(new Message(Message::RENDER));
     // The server is here so we have something to pass there
     MockServer server(io_service_);
-    rq.resolve(q, answer, buffer, &server);
+    running_query_ = rq.resolve(q, answer, buffer, &server);
     // We don't need to run the service in this test. We are interested only
     // in the place it starts resolving at
 
diff --git a/src/lib/resolve/tests/recursive_query_unittest_2.cc b/src/lib/resolve/tests/recursive_query_unittest_2.cc
index 6cb404d..0b38c59 100644
--- a/src/lib/resolve/tests/recursive_query_unittest_2.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest_2.cc
@@ -149,6 +149,12 @@ public:
     OutputBufferPtr udp_send_buffer_;           ///< Send buffer for UDP I/O
     udp::socket     udp_socket_;                ///< Socket used by UDP server
 
+    /// Some of the tests cause an 'active' running query to be created, but
+    /// don't complete the framework that makes that query delete itself.
+    /// This member can be used to store it so that it is deleted automatically
+    /// when the test is finished.
+    AbstractRunningQuery* running_query_;
+
     /// \brief Constructor
     RecursiveQueryTest2() :
         debug_(DEBUG_PRINT),
@@ -170,8 +176,18 @@ public:
         udp_length_(0),
         udp_receive_buffer_(),
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
-        udp_socket_(service_.get_io_service(), udp::v4())
-    {
+        udp_socket_(service_.get_io_service(), udp::v4()),
+        running_query_(NULL)
+    {}
+
+    ~RecursiveQueryTest2() {
+        // It would delete itself, but after the io_service_, which could
+        // segfailt in case there were unhandled requests
+        resolver_.reset();
+        // In a similar note, we wait until the resolver has been cleaned up
+        // until deleting and active test running_query_
+        delete running_query_;
+        delete nsas_;
     }
 
     /// \brief Set Common Message Bits
@@ -686,7 +702,7 @@ TEST_F(RecursiveQueryTest2, Resolve) {
     // Kick off the resolution process.  We expect the first question to go to
     // "root".
     expected_ = UDP_ROOT;
-    query.resolve(question_, resolver_callback);
+    running_query_ = query.resolve(question_, resolver_callback);
     service_.run();
 
     // Check what ran. (We have to cast the callback to ResolverCallback as we
diff --git a/src/lib/resolve/tests/recursive_query_unittest_3.cc b/src/lib/resolve/tests/recursive_query_unittest_3.cc
index abfea9a..92ec589 100644
--- a/src/lib/resolve/tests/recursive_query_unittest_3.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest_3.cc
@@ -132,6 +132,12 @@ public:
     OutputBufferPtr udp_send_buffer_;           ///< Send buffer for UDP I/O
     udp::socket     udp_socket_;                ///< Socket used by UDP server
 
+    /// Some of the tests cause an 'active' running query to be created, but
+    /// don't complete the framework that makes that query delete itself.
+    /// This member can be used to store it so that it is deleted automatically
+    /// when the test is finished.
+    AbstractRunningQuery* running_query_;
+
     /// \brief Constructor
     RecursiveQueryTest3() :
         service_(),
@@ -154,10 +160,21 @@ public:
         udp_length_(0),
         udp_receive_buffer_(),
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
-        udp_socket_(service_.get_io_service(), udp::v4())
+        udp_socket_(service_.get_io_service(), udp::v4()),
+        running_query_(NULL)
     {
     }
 
+    ~RecursiveQueryTest3() {
+        delete nsas_;
+        // It would delete itself, but after the io_service_, which could
+        // segfailt in case there were unhandled requests
+        resolver_.reset();
+        // In a similar note, we wait until the resolver has been cleaned up
+        // until deleting and active test running_query_
+        delete running_query_;
+    }
+
     /// \brief Set Common Message Bits
     ///
     /// Sets up the common bits of a response message returned by the handlers.
@@ -542,7 +559,7 @@ TEST_F(RecursiveQueryTest3, Resolve) {
 
     // Kick off the resolution process.
     expected_ = EDNS_UDP;
-    query.resolve(question_, resolver_callback);
+    running_query_ = query.resolve(question_, resolver_callback);
     service_.run();
 
     // Check what ran. (We have to cast the callback to ResolverCallback3 as we
diff --git a/src/lib/server_common/tests/portconfig_unittest.cc b/src/lib/server_common/tests/portconfig_unittest.cc
index ac880c0..0c971ee 100644
--- a/src/lib/server_common/tests/portconfig_unittest.cc
+++ b/src/lib/server_common/tests/portconfig_unittest.cc
@@ -18,6 +18,7 @@
 #include <testutils/socket_request.h>
 #include <testutils/mockups.h>
 
+#include <util/unittests/check_valgrind.h>
 #include <util/unittests/resource.h>
 
 #include <cc/data.h>
@@ -311,44 +312,48 @@ typedef InstallListenAddresses InstallListenAddressesDeathTest;
 // We make the socket requestor throw a "fatal" exception, one where we can't be
 // sure the state between processes is consistent. So we abort in that case.
 TEST_F(InstallListenAddressesDeathTest, inconsistent) {
-    AddressList deathAddresses;
-    deathAddresses.push_back(AddressPair("192.0.2.3", 5288));
-    // Make sure it actually kills the application (there should be an abort
-    // in this case)
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
+    if (!isc::util::unittests::runningOnValgrind()) {
+        AddressList deathAddresses;
+        deathAddresses.push_back(AddressPair("192.0.2.3", 5288));
+        // Make sure it actually kills the application (there should be an abort
+        // in this case)
+        EXPECT_DEATH({
+            isc::util::unittests::dontCreateCoreDumps();
 
-        try {
-          installListenAddresses(deathAddresses, store_, dnss_);
-        } catch (...) {
-          // Prevent exceptions killing the application, we need
-          // to make sure it dies the real hard way
-        };
-      }, "");
+            try {
+              installListenAddresses(deathAddresses, store_, dnss_);
+            } catch (...) {
+              // Prevent exceptions killing the application, we need
+              // to make sure it dies the real hard way
+            };
+          }, "");
+    }
 }
 
 // If we are unable to tell the boss we closed a socket, we abort, as we are
 // not consistent with the boss most probably.
 TEST_F(InstallListenAddressesDeathTest, cantClose) {
-    installListenAddresses(valid_, store_, dnss_);
-    AddressList empty;
-    // Instruct it to fail on close
-    sock_requestor_.break_release_ = true;
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
+    if (!isc::util::unittests::runningOnValgrind()) {
+        installListenAddresses(valid_, store_, dnss_);
+        AddressList empty;
+        // Instruct it to fail on close
+        sock_requestor_.break_release_ = true;
+        EXPECT_DEATH({
+            isc::util::unittests::dontCreateCoreDumps();
 
-        try {
-          // Setting to empty will close all current sockets.
-          // And thanks to the break_release_, the close will
-          // throw, which will make it crash.
-          installListenAddresses(empty, store_, dnss_);
-        } catch (...) {
-          // To make sure it is killed by abort, not by some
-          // (unhandled) exception
-        };
-      }, "");
-    // And reset it back, so it can safely clean up itself.
-    sock_requestor_.break_release_ = false;
+            try {
+              // Setting to empty will close all current sockets.
+              // And thanks to the break_release_, the close will
+              // throw, which will make it crash.
+              installListenAddresses(empty, store_, dnss_);
+            } catch (...) {
+              // To make sure it is killed by abort, not by some
+              // (unhandled) exception
+            };
+          }, "");
+        // And reset it back, so it can safely clean up itself.
+        sock_requestor_.break_release_ = false;
+    }
 }
 #endif // EXPECT_DEATH
 
diff --git a/src/lib/util/tests/buffer_unittest.cc b/src/lib/util/tests/buffer_unittest.cc
index 9af3d57..02ca83d 100644
--- a/src/lib/util/tests/buffer_unittest.cc
+++ b/src/lib/util/tests/buffer_unittest.cc
@@ -18,6 +18,7 @@
 
 #ifdef EXPECT_DEATH
 #include <util/unittests/resource.h>
+#include <util/unittests/check_valgrind.h>
 #endif /* EXPECT_DEATH */
 
 #include <util/buffer.h>
@@ -188,16 +189,18 @@ TEST_F(BufferTest, outputBufferReadat) {
     }
 #ifdef EXPECT_DEATH
     // We use assert now, so we check it dies
-    EXPECT_DEATH({
-        isc::util::unittests::dontCreateCoreDumps();
-
-        try {
-            obuffer[sizeof(testdata)];
-        } catch (...) {
-            // Prevent exceptions killing the application, we need
-            // to make sure it dies the real hard way
-        }
-        }, "");
+    if (!isc::util::unittests::runningOnValgrind()) {
+        EXPECT_DEATH({
+            isc::util::unittests::dontCreateCoreDumps();
+
+            try {
+                obuffer[sizeof(testdata)];
+            } catch (...) {
+                // Prevent exceptions killing the application, we need
+                // to make sure it dies the real hard way
+            }
+            }, "");
+    }
 #endif
 }
 
diff --git a/src/lib/util/threads/tests/lock_unittest.cc b/src/lib/util/threads/tests/lock_unittest.cc
index 4ddb6b2..66b930c 100644
--- a/src/lib/util/threads/tests/lock_unittest.cc
+++ b/src/lib/util/threads/tests/lock_unittest.cc
@@ -12,10 +12,12 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <gtest/gtest.h>
+
+#include <util/threads/lock.h>
 #include <util/threads/sync.h>
 #include <util/threads/thread.h>
-
-#include <gtest/gtest.h>
+#include <util/unittests/check_valgrind.h>
 
 #include <boost/bind.hpp>
 #include <unistd.h>
@@ -42,13 +44,15 @@ TEST(MutexTest, lockMultiple) {
 // Destroying a locked mutex is a bad idea as well
 #ifdef EXPECT_DEATH
 TEST(MutexTest, destroyLocked) {
-    EXPECT_DEATH({
-        Mutex* mutex = new Mutex;
-        new Mutex::Locker(*mutex);
-        delete mutex;
-        // This'll leak the locker, but inside the slave process, it should
-        // not be an issue.
-    }, "");
+    if (!isc::util::unittests::runningOnValgrind()) {
+        EXPECT_DEATH({
+            Mutex* mutex = new Mutex;
+            new Mutex::Locker(*mutex);
+            delete mutex;
+            // This'll leak the locker, but inside the slave process, it should
+            // not be an issue.
+        }, "");
+    }
 }
 #endif
 
diff --git a/src/lib/util/unittests/Makefile.am b/src/lib/util/unittests/Makefile.am
index 451ab4e..55e0372 100644
--- a/src/lib/util/unittests/Makefile.am
+++ b/src/lib/util/unittests/Makefile.am
@@ -7,6 +7,7 @@ libutil_unittests_la_SOURCES += newhook.h newhook.cc
 libutil_unittests_la_SOURCES += testdata.h testdata.cc
 if HAVE_GTEST
 libutil_unittests_la_SOURCES += resource.h resource.cc
+libutil_unittests_la_SOURCES += check_valgrind.h check_valgrind.cc
 libutil_unittests_la_SOURCES += run_all.h run_all.cc
 libutil_unittests_la_SOURCES += textdata.h
 libutil_unittests_la_SOURCES += wiredata.h wiredata.cc
diff --git a/src/lib/util/unittests/check_valgrind.cc b/src/lib/util/unittests/check_valgrind.cc
new file mode 100644
index 0000000..2951d25
--- /dev/null
+++ b/src/lib/util/unittests/check_valgrind.cc
@@ -0,0 +1,41 @@
+// Copyright (C) 2012  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 <config.h>
+
+namespace isc {
+namespace util {
+namespace unittests {
+
+#if HAVE_VALGRIND_HEADERS
+#include <valgrind/valgrind.h>
+/// \brief Check if the program is run in valgrind
+///
+/// \return true if valgrind headers are available, and valgrind is running,
+///         false if the headers are not available, or if valgrind is not
+///         running
+bool
+runningOnValgrind() {
+    return (RUNNING_ON_VALGRIND != 0);
+}
+#else
+bool
+runningOnValgrind() {
+    return (false);
+}
+#endif // HAVE_VALGRIND_HEADERS
+
+} // end of namespace unittests
+} // end of namespace util
+} // end of namespace isc
diff --git a/src/lib/util/unittests/check_valgrind.h b/src/lib/util/unittests/check_valgrind.h
new file mode 100644
index 0000000..c08b58b
--- /dev/null
+++ b/src/lib/util/unittests/check_valgrind.h
@@ -0,0 +1,50 @@
+// Copyright (C) 2012  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.
+
+//
+// If we have the valgrind headers available, we can detect whether
+// valgrind is running. This should normally never be done, as you
+// want the to test the actual code in operation with valgrind.
+//
+// However, there is a limited set of operations where we want to
+// skip some tests if run under valgrind, most notably the
+// EXPECT_DEATH tests, as these would report memory leaks by
+// definition.
+//
+// If the valgrind headers are NOT available, the method checkValgrind()
+// always returns false; i.e. it always pretends the program is run
+// natively
+//
+
+#ifndef __UTIL_UNITTESTS_CHECK_VALGRIND_H
+#define __UTIL_UNITTESTS_CHECK_VALGRIND_H 1
+
+#include <config.h>
+
+namespace isc {
+namespace util {
+namespace unittests {
+
+/// \brief Check if the program is run in valgrind
+///
+/// \return true if valgrind headers are available, and valgrind is running,
+///         false if the headers are not available, or if valgrind is not
+///         running
+bool runningOnValgrind();
+
+} // end namespace unittests
+} // end namespace util
+} // end namespace isc
+
+#endif // __UTIL_UNITTESTS_CHECK_VALGRIND_H



More information about the bind10-changes mailing list