[master] Merge branch 'trac2349'

Conflicts:
	src/lib/util/threads/tests/lock_unittest.cc

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"

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

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.

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
 }
 

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
 }

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

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,

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
 

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

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

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
 

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
 }
 

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
 

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

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

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