BIND 10 master, updated. 3811dac0257939f28cefa621226019e6d1c75239 [trac641] Fix unit test

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Mar 16 14:50:51 UTC 2011


The branch, master has been updated
       via  3811dac0257939f28cefa621226019e6d1c75239 (commit)
       via  0c730e6357cf058473dc389820145d64cd5d946d (commit)
       via  788d9505e0bec5369ccecc6f51af957874063fd8 (commit)
       via  e0122c1e3219488a152350666165369a0e9fbe32 (commit)
       via  f1f8de8ad231b05e58abbc8644d5c7ae5bfd9feb (commit)
       via  6a34b5430a7e16c10cc08a957fedb46cd45b5bbf (commit)
       via  c416ba80303b3f6938dd93399ba988600e37c675 (commit)
       via  ddb07c03439bba577baf4f0c1462cbb8749d2d0e (commit)
      from  e909ff99007b9b9b26af58848d3fccdb25d4135d (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 3811dac0257939f28cefa621226019e6d1c75239
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Mar 16 14:35:02 2011 +0000

    [trac641] Fix unit test
    
    This branch removed the shared pointer that the NSAS used to link
    to the resolver.  As a result we now have to make sure that the
    resolver stays in existence while the NSAS is active.  This commit
    fixes that problem in a unit test.

commit 0c730e6357cf058473dc389820145d64cd5d946d
Merge: 788d9505e0bec5369ccecc6f51af957874063fd8 e909ff99007b9b9b26af58848d3fccdb25d4135d
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Mar 16 12:47:47 2011 +0000

    Merge branch 'master' into trac641

commit 788d9505e0bec5369ccecc6f51af957874063fd8
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Mar 16 10:55:18 2011 +0000

    [trac641] Changes after review comments

commit e0122c1e3219488a152350666165369a0e9fbe32
Author: Stephen Morris <stephen at isc.org>
Date:   Tue Mar 15 21:47:59 2011 +0000

    [trac641] Added destructor to TestResolver to cure memory leaks
    
    Reverted ZoneEntry to the original version and added the destructor
    to TestResolver to call all the saved callbacks.  This breaks
    internal shared_ptr loops, allowing the NSAS to destoy itself
    gracefully, and avoiding memory leaks.

commit f1f8de8ad231b05e58abbc8644d5c7ae5bfd9feb
Author: Stephen Morris <stephen at isc.org>
Date:   Tue Mar 15 20:02:28 2011 +0000

    [trac641] Random number generator now throws exceptions on error
    
    Previously when compiled without NDEBUG set it did, if a check
    failed it called assert().  The tests checks for this using
    ASSERT_DEATH.  However, ASSERT_DEATH leaks memory and this was
    obscuring the valgrind output.  The changes here cause an exception
    to be thrown (instead of a call to abort()) in the case of an error
    and the unit tests now EXPECT_THROW instead of ASSERT_DEATH.

commit 6a34b5430a7e16c10cc08a957fedb46cd45b5bbf
Author: Stephen Morris <stephen at isc.org>
Date:   Tue Mar 15 18:46:25 2011 +0000

    [trac641] Remove callback objects's shared pointers to ZoneEntry
    
    The ZoneEntry object was creating a callback and keeping a shared
    pointer to it, but the callback was also using a shared pointer to
    link back to the ZoneEntry.  This change removed the shared pointers
    in the callback objects - they now use a "raw" pointer to point back
    to the ZoneEntry.

commit c416ba80303b3f6938dd93399ba988600e37c675
Author: Stephen Morris <stephen at isc.org>
Date:   Tue Mar 15 16:32:52 2011 +0000

    [trac641] Change headers to use C++-style comments
    
    ... to make it easier to comment out sets of tests for debugging.

commit ddb07c03439bba577baf4f0c1462cbb8749d2d0e
Author: Stephen Morris <stephen at isc.org>
Date:   Tue Mar 15 16:07:35 2011 +0000

    [trac641] First part of tackling memory leaks
    
    In the NSAS, store a pointer to the resolver as a "raw" pointer,
    not a shared pointer.  The NSAS is part of the resolver, although
    it can call back into the resolver.  If both store a shared pointer
    to each other we can have the case where the reference counts can
    never drop to zero.

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

Summary of changes:
 src/lib/nsas/nameserver_address_store.cc           |   13 ++--
 src/lib/nsas/nameserver_address_store.h            |    2 +-
 src/lib/nsas/nameserver_entry.cc                   |    6 +-
 src/lib/nsas/nameserver_entry.h                    |    4 +-
 src/lib/nsas/random_number_generator.h             |   73 ++++++++++++++++----
 .../tests/nameserver_address_store_unittest.cc     |   40 ++++-------
 src/lib/nsas/tests/nameserver_address_unittest.cc  |   12 ++--
 src/lib/nsas/tests/nameserver_entry_unittest.cc    |   30 ++++----
 src/lib/nsas/tests/nsas_test.h                     |   20 ++++--
 .../nsas/tests/random_number_generator_unittest.cc |   16 +++--
 src/lib/nsas/tests/zone_entry_unittest.cc          |   10 ++--
 src/lib/nsas/zone_entry.cc                         |    2 +-
 src/lib/nsas/zone_entry.h                          |    5 +-
 .../resolve/tests/recursive_query_unittest_2.cc    |   17 +++--
 14 files changed, 151 insertions(+), 99 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/nsas/nameserver_address_store.cc b/src/lib/nsas/nameserver_address_store.cc
index ae543c3..e92c177 100644
--- a/src/lib/nsas/nameserver_address_store.cc
+++ b/src/lib/nsas/nameserver_address_store.cc
@@ -54,7 +54,7 @@ NameserverAddressStore::NameserverAddressStore(
         new HashDeleter<ZoneEntry>(*zone_hash_))),
     nameserver_lru_(new LruList<NameserverEntry>((3 * nshashsize),
         new HashDeleter<NameserverEntry>(*nameserver_hash_))),
-    resolver_(resolver)
+    resolver_(resolver.get())
 { }
 
 namespace {
@@ -67,12 +67,12 @@ namespace {
  */
 boost::shared_ptr<ZoneEntry>
 newZone(
-    const boost::shared_ptr<isc::resolve::ResolverInterface>* resolver,
+    isc::resolve::ResolverInterface* resolver,
     const string* zone, const RRClass* class_code,
     const boost::shared_ptr<HashTable<NameserverEntry> >* ns_hash,
     const boost::shared_ptr<LruList<NameserverEntry> >* ns_lru)
 {
-    boost::shared_ptr<ZoneEntry> result(new ZoneEntry(*resolver, *zone, *class_code,
+    boost::shared_ptr<ZoneEntry> result(new ZoneEntry(resolver, *zone, *class_code,
         *ns_hash, *ns_lru));
     return (result);
 }
@@ -84,9 +84,10 @@ NameserverAddressStore::lookup(const string& zone, const RRClass& class_code,
     boost::shared_ptr<AddressRequestCallback> callback, AddressFamily family,
     const GlueHints& glue_hints)
 {
-    pair<bool, boost::shared_ptr<ZoneEntry> > zone_obj(zone_hash_->getOrAdd(HashKey(
-        zone, class_code), boost::bind(newZone, &resolver_, &zone, &class_code,
-        &nameserver_hash_, &nameserver_lru_)));
+    pair<bool, boost::shared_ptr<ZoneEntry> > zone_obj(
+        zone_hash_->getOrAdd(HashKey(zone, class_code),
+                             boost::bind(newZone, resolver_, &zone, &class_code,
+                                         &nameserver_hash_, &nameserver_lru_)));
     if (zone_obj.first) {
         zone_lru_->add(zone_obj.second);
     } else {
diff --git a/src/lib/nsas/nameserver_address_store.h b/src/lib/nsas/nameserver_address_store.h
index 7cbf831..d54be84 100644
--- a/src/lib/nsas/nameserver_address_store.h
+++ b/src/lib/nsas/nameserver_address_store.h
@@ -116,7 +116,7 @@ protected:
     boost::shared_ptr<LruList<NameserverEntry> > nameserver_lru_;
     // The resolver we use
 private:
-    boost::shared_ptr<isc::resolve::ResolverInterface> resolver_;
+    isc::resolve::ResolverInterface* resolver_;
     //}@
 };
 
diff --git a/src/lib/nsas/nameserver_entry.cc b/src/lib/nsas/nameserver_entry.cc
index 5c7873e..367ea0a 100644
--- a/src/lib/nsas/nameserver_entry.cc
+++ b/src/lib/nsas/nameserver_entry.cc
@@ -380,8 +380,7 @@ class NameserverEntry::ResolverCallback :
 };
 
 void
-NameserverEntry::askIP(
-    boost::shared_ptr<isc::resolve::ResolverInterface> resolver,
+NameserverEntry::askIP(isc::resolve::ResolverInterface* resolver,
     const RRType& type, AddressFamily family)
 {
     QuestionPtr question(new Question(Name(getName()), RRClass(getClass()),
@@ -392,8 +391,7 @@ NameserverEntry::askIP(
 }
 
 void
-NameserverEntry::askIP(
-    boost::shared_ptr<isc::resolve::ResolverInterface> resolver,
+NameserverEntry::askIP(isc::resolve::ResolverInterface* resolver,
     boost::shared_ptr<Callback> callback, AddressFamily family)
 {
     Lock lock(mutex_);
diff --git a/src/lib/nsas/nameserver_entry.h b/src/lib/nsas/nameserver_entry.h
index 77937d1..99d7ff5 100644
--- a/src/lib/nsas/nameserver_entry.h
+++ b/src/lib/nsas/nameserver_entry.h
@@ -241,7 +241,7 @@ public:
      *     even when there are addresses, if there are no addresses for this
      *     family.
      */
-    void askIP(boost::shared_ptr<isc::resolve::ResolverInterface> resolver,
+    void askIP(isc::resolve::ResolverInterface* resolver,
         boost::shared_ptr<Callback> callback, AddressFamily family);
     //@}
 
@@ -273,7 +273,7 @@ private:
     /// \short Private version that does the actual asking of one address type
     ///
     /// Call unlocked.
-    void askIP(boost::shared_ptr<isc::resolve::ResolverInterface> resolver,
+    void askIP(isc::resolve::ResolverInterface* resolver,
         const isc::dns::RRType&, AddressFamily);
 };
 
diff --git a/src/lib/nsas/random_number_generator.h b/src/lib/nsas/random_number_generator.h
index e80ebcb..8884d0e 100644
--- a/src/lib/nsas/random_number_generator.h
+++ b/src/lib/nsas/random_number_generator.h
@@ -15,8 +15,12 @@
 #ifndef __NSAS_RANDOM_NUMBER_GENERATOR_H
 #define __NSAS_RANDOM_NUMBER_GENERATOR_H
 
+#include <algorithm>
 #include <cmath>
 #include <numeric>
+
+#include <exceptions/exceptions.h>
+
 #include <boost/random/mersenne_twister.hpp>
 #include <boost/random/uniform_int.hpp>
 #include <boost/random/uniform_real.hpp>
@@ -25,6 +29,26 @@
 namespace isc {
 namespace nsas {
 
+class InvalidLimits : public isc::BadValue {
+public:
+    InvalidLimits(const char* file, size_t line, const char* what) :
+        isc::BadValue(file, line, what) {}
+};
+
+class SumNotOne : public isc::BadValue {
+public:
+    SumNotOne(const char* file, size_t line, const char* what) :
+        isc::BadValue(file, line, what) {}
+};
+
+class InvalidProbValue : public isc::BadValue {
+public:
+    InvalidProbValue(const char* file, size_t line, const char* what) :
+        isc::BadValue(file, line, what) {}
+};
+
+
+
 /// \brief Uniform random integer generator
 ///
 /// Generate uniformly distributed integers in range of [min, max]
@@ -35,8 +59,17 @@ public:
     /// \param min The minimum number in the range
     /// \param max The maximum number in the range
     UniformRandomIntegerGenerator(int min, int max):
-        min_(min), max_(max), dist_(min, max), generator_(rng_, dist_)
+        min_(std::min(min, max)), max_(std::max(min, max)),
+        dist_(min_, max_), generator_(rng_, dist_)
     {
+        // To preserve the restriction of the underlying uniform_int class (and
+        // to retain compatibility with earlier versions of the class), we will
+        // abort if the minimum and maximum given are the wrong way round.
+        if (min > max) {
+            isc_throw(InvalidLimits, "minimum limit is greater than maximum "
+                      "when initializing UniformRandomIntegerGenerator");
+        }
+
         // Init with the current time
         rng_.seed(time(NULL));
     }
@@ -73,8 +106,10 @@ public:
         size_t min = 0):
         dist_(0, 1.0), uniform_real_gen_(rng_, dist_), min_(min)
     {
-        // The probabilities must be valid
-        assert(isProbabilitiesValid(probabilities));
+        // The probabilities must be valid.  Checking is quite an expensive
+        // operation, so is only done in a debug build.
+        assert(areProbabilitiesValid(probabilities));
+
         // Calculate the partial sum of probabilities
         std::partial_sum(probabilities.begin(), probabilities.end(),
                                      std::back_inserter(cumulative_));
@@ -96,8 +131,8 @@ public:
     /// \param min The minimum integer that generated
     void reset(const std::vector<double>& probabilities, size_t min = 0)
     {
-        // The probabilities must be valid
-        assert(isProbabilitiesValid(probabilities));
+        // The probabilities must be valid.
+        assert(areProbabilitiesValid(probabilities));
 
         // Reset the cumulative sum
         cumulative_.clear();
@@ -120,16 +155,24 @@ public:
 private:
     /// \brief Check the validation of probabilities vector
     ///
-    /// The probability must be in range of [0, 1.0] and the sum must be equal to 1.0
-    /// Empty probabilities is also valid.
-    bool isProbabilitiesValid(const std::vector<double>& probabilities) const
+    /// The probability must be in range of [0, 1.0] and the sum must be equal
+    /// to 1.0.  Empty probabilities are also valid.
+    ///
+    /// Checking the probabilities is quite an expensive operation, so it is
+    /// only done during a debug build (via a call through assert()).  However,
+    /// instead of letting assert() call abort(), if this method encounters an
+    /// error, an exception is thrown.  This makes unit testing somewhat easier.
+    ///
+    /// \param probabilities Vector of probabilities.
+    bool areProbabilitiesValid(const std::vector<double>& probabilities) const
     {
         typedef std::vector<double>::const_iterator Iterator;
         double sum = probabilities.empty() ? 1.0 : 0.0;
         for(Iterator it = probabilities.begin(); it != probabilities.end(); ++it){
             //The probability must be in [0, 1.0]
             if(*it < 0.0 || *it > 1.0) {
-                return false;
+                isc_throw(InvalidProbValue,
+                          "probability must be in the range 0..1");
             }
 
             sum += *it;
@@ -137,12 +180,16 @@ private:
 
         double epsilon = 0.0001;
         // The sum must be equal to 1
-        return std::fabs(sum - 1.0) < epsilon;
+       if (std::fabs(sum - 1.0) >= epsilon) {
+           isc_throw(SumNotOne, "Sum of probabilities is not equal to 1");
+       }
+
+       return true;
     }
 
-    std::vector<double> cumulative_;            ///< The partial sum of the probabilities
-    boost::mt19937 rng_;                        ///< Mersenne Twister: A 623-dimensionally equidistributed uniform pseudo-random number generator 
-    boost::uniform_real<> dist_;                ///< Uniformly distributed real numbers
+    std::vector<double> cumulative_;    ///< Partial sum of the probabilities
+    boost::mt19937 rng_;                ///< Mersenne Twister: A 623-dimensionally equidistributed uniform pseudo-random number generator 
+    boost::uniform_real<> dist_;        ///< Uniformly distributed real numbers
 
     // Shortcut typedef
     // This typedef is placed directly before its use, as the sunstudio
diff --git a/src/lib/nsas/tests/nameserver_address_store_unittest.cc b/src/lib/nsas/tests/nameserver_address_store_unittest.cc
index 95b46a8..9133daf 100644
--- a/src/lib/nsas/tests/nameserver_address_store_unittest.cc
+++ b/src/lib/nsas/tests/nameserver_address_store_unittest.cc
@@ -131,7 +131,7 @@ protected:
         for (int i = 1; i <= 9; ++i) {
             std::string name = "zone" + boost::lexical_cast<std::string>(i);
             zones_.push_back(boost::shared_ptr<ZoneEntry>(new ZoneEntry(
-                resolver_, name, RRClass(40 + i),
+                resolver_.get(), name, RRClass(40 + i),
                 boost::shared_ptr<HashTable<NameserverEntry> >(),
                 boost::shared_ptr<LruList<NameserverEntry> >())));
         }
@@ -232,11 +232,9 @@ TEST_F(NameserverAddressStoreTest, NameserverDeletionCheck) {
     EXPECT_EQ(1, nameservers_[1].use_count());
 }
 
-/**
- * \short Try lookup on empty store.
- *
- * Check if it asks correct questions and it keeps correct internal state.
- */
+/// \brief Try lookup on empty store.
+///
+/// Check if it asks correct questions and it keeps correct internal state.
 TEST_F(NameserverAddressStoreTest, emptyLookup) {
     DerivedNsas nsas(resolver_, 10, 10);
     // Ask it a question
@@ -268,11 +266,9 @@ TEST_F(NameserverAddressStoreTest, emptyLookup) {
     }
 }
 
-/**
- * \short Try looking up a zone that does not have any nameservers.
- *
- * It should not ask anything and say it is unreachable right away.
- */
+/// \brief Try looking up a zone that does not have any nameservers.
+///
+/// It should not ask anything and say it is unreachable right away.
 TEST_F(NameserverAddressStoreTest, zoneWithoutNameservers) {
     DerivedNsas nsas(resolver_, 10, 10);
     // Ask it a question
@@ -285,13 +281,11 @@ TEST_F(NameserverAddressStoreTest, zoneWithoutNameservers) {
     EXPECT_FALSE(NSASCallback::results[0].first);
 }
 
-/**
- * \short Try looking up a zone that has only an unreachable nameserver.
- *
- * It should be unreachable. Furthermore, subsequent questions for that zone
- * or other zone with the same nameserver should be unreachable right away,
- * without further asking.
- */
+/// \brief Try looking up a zone that has only an unreachable nameserver.
+///
+/// It should be unreachable. Furthermore, subsequent questions for that zone
+/// or other zone with the same nameserver should be unreachable right away,
+/// without further asking.
 TEST_F(NameserverAddressStoreTest, unreachableNS) {
     DerivedNsas nsas(resolver_, 10, 10);
     // Ask it a question
@@ -326,12 +320,10 @@ TEST_F(NameserverAddressStoreTest, unreachableNS) {
     }
 }
 
-/**
- * \short Try to stress it little bit by having multiple zones and nameservers.
- *
- * Does some asking, on a set of zones that share some nameservers, with
- * slower answering, evicting data, etc.
- */
+/// \short Try to stress it little bit by having multiple zones and nameservers.
+///
+/// Does some asking, on a set of zones that share some nameservers, with
+/// slower answering, evicting data, etc.
 TEST_F(NameserverAddressStoreTest, CombinedTest) {
     // Create small caches, so we get some evictions
     DerivedNsas nsas(resolver_, 1, 1);
diff --git a/src/lib/nsas/tests/nameserver_address_unittest.cc b/src/lib/nsas/tests/nameserver_address_unittest.cc
index 1f924b3..457e61c 100644
--- a/src/lib/nsas/tests/nameserver_address_unittest.cc
+++ b/src/lib/nsas/tests/nameserver_address_unittest.cc
@@ -39,7 +39,9 @@ class NameserverEntrySample {
 public:
     NameserverEntrySample():
         name_("example.org"),
-        rrv4_(new RRset(name_, RRClass::IN(), RRType::A(), RRTTL(1200)))
+        rrv4_(new RRset(name_, RRClass::IN(), RRType::A(), RRTTL(1200))),
+        ns_(new NameserverEntry(name_.toText(), RRClass::IN())),
+        resolver_(new TestResolver())
     {
         // Add some sample A records
         rrv4_->addRdata(ConstRdataPtr(new RdataTest<A>("1.2.3.4")));
@@ -47,10 +49,9 @@ public:
         rrv4_->addRdata(ConstRdataPtr(new RdataTest<A>("9.10.11.12")));
 
         ns_.reset(new NameserverEntry(name_.toText(), RRClass::IN()));
-        boost::shared_ptr<TestResolver> resolver(new TestResolver);
-        ns_->askIP(resolver, boost::shared_ptr<Callback>(new Callback), ANY_OK);
-        resolver->asksIPs(name_, 0, 1);
-        resolver->requests[0].second->success(createResponseMessage(rrv4_));
+        ns_->askIP(resolver_.get(), boost::shared_ptr<Callback>(new Callback), ANY_OK);
+        resolver_->asksIPs(name_, 0, 1);
+        resolver_->requests[0].second->success(createResponseMessage(rrv4_));
     }
 
     // Return the sample NameserverEntry
@@ -75,6 +76,7 @@ private:
     Name name_;                             ///< Name of the sample
     RRsetPtr rrv4_;           ///< Standard RRSet - IN, A, lowercase name
     boost::shared_ptr<NameserverEntry> ns_; ///< Shared_ptr that points to a NameserverEntry object
+    boost::shared_ptr<TestResolver> resolver_;
 
     class Callback : public NameserverEntry::Callback {
         public:
diff --git a/src/lib/nsas/tests/nameserver_entry_unittest.cc b/src/lib/nsas/tests/nameserver_entry_unittest.cc
index 398c568..4225e87 100644
--- a/src/lib/nsas/tests/nameserver_entry_unittest.cc
+++ b/src/lib/nsas/tests/nameserver_entry_unittest.cc
@@ -86,7 +86,7 @@ protected:
         boost::shared_ptr<TestResolver> resolver(new TestResolver);
         boost::shared_ptr<Callback> callback(new Callback);
         // Let it ask for data
-        entry->askIP(resolver, callback, ANY_OK);
+        entry->askIP(resolver.get(), callback, ANY_OK);
         // Check it really asked and sort the queries
         EXPECT_TRUE(resolver->asksIPs(Name(entry->getName()), 0, 1));
         // Respond with answers
@@ -266,7 +266,7 @@ TEST_F(NameserverEntryTest, IPCallbacks) {
     boost::shared_ptr<Callback> callback(new Callback);
     boost::shared_ptr<TestResolver> resolver(new TestResolver);
 
-    entry->askIP(resolver, callback, ANY_OK);
+    entry->askIP(resolver.get(), callback, ANY_OK);
     // Ensure it becomes IN_PROGRESS
     EXPECT_EQ(Fetchable::IN_PROGRESS, entry->getState());
     // Now, there should be two queries in the resolver
@@ -274,12 +274,12 @@ TEST_F(NameserverEntryTest, IPCallbacks) {
     ASSERT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 0, 1));
 
     // Another one might ask
-    entry->askIP(resolver, callback, V4_ONLY);
+    entry->askIP(resolver.get(), callback, V4_ONLY);
     // There should still be only two queries in the resolver
     ASSERT_EQ(2, resolver->requests.size());
 
     // Another one, with need of IPv6 address
-    entry->askIP(resolver, callback, V6_ONLY);
+    entry->askIP(resolver.get(), callback, V6_ONLY);
 
     // Answer one and see that the callbacks are called
     resolver->answer(0, Name(EXAMPLE_CO_UK), RRType::A(),
@@ -316,7 +316,7 @@ TEST_F(NameserverEntryTest, IPCallbacksUnreachable) {
     boost::shared_ptr<TestResolver> resolver(new TestResolver);
 
     // Ask for its IP
-    entry->askIP(resolver, callback, ANY_OK);
+    entry->askIP(resolver.get(), callback, ANY_OK);
     // Check it asks the resolver
     EXPECT_EQ(2, resolver->requests.size());
     ASSERT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 0, 1));
@@ -352,7 +352,7 @@ TEST_F(NameserverEntryTest, DirectAnswer) {
         RRType::AAAA()), RRsetPtr());
 
     // A successfull test first
-    entry->askIP(resolver, callback, ANY_OK);
+    entry->askIP(resolver.get(), callback, ANY_OK);
     EXPECT_EQ(0, resolver->requests.size());
     EXPECT_EQ(1, callback->count);
     NameserverEntry::AddressVector addresses;
@@ -362,7 +362,7 @@ TEST_F(NameserverEntryTest, DirectAnswer) {
     // An unsuccessfull test
     callback->count = 0;
     entry.reset(new NameserverEntry(EXAMPLE_NET, RRClass::IN()));
-    entry->askIP(resolver, callback, ANY_OK);
+    entry->askIP(resolver.get(), callback, ANY_OK);
     EXPECT_EQ(0, resolver->requests.size());
     EXPECT_EQ(1, callback->count);
     addresses.clear();
@@ -381,8 +381,8 @@ TEST_F(NameserverEntryTest, ChangedExpired) {
     boost::shared_ptr<TestResolver> resolver(new TestResolver);
 
     // Ask the first time
-    entry->askIP(resolver, callback, V4_ONLY);
-    entry->askIP(resolver, callback, V6_ONLY);
+    entry->askIP(resolver.get(), callback, V4_ONLY);
+    entry->askIP(resolver.get(), callback, V6_ONLY);
     EXPECT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 0, 1));
     EXPECT_EQ(Fetchable::IN_PROGRESS, entry->getState());
     resolver->answer(0, Name(EXAMPLE_CO_UK), RRType::A(),
@@ -402,8 +402,8 @@ TEST_F(NameserverEntryTest, ChangedExpired) {
 
     // Ask the second time. The callbacks should not fire right away and it
     // should request the addresses again
-    entry->askIP(resolver, callback, V4_ONLY);
-    entry->askIP(resolver, callback, V6_ONLY);
+    entry->askIP(resolver.get(), callback, V4_ONLY);
+    entry->askIP(resolver.get(), callback, V6_ONLY);
     EXPECT_EQ(2, callback->count);
     EXPECT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 2, 3));
     EXPECT_EQ(Fetchable::IN_PROGRESS, entry->getState());
@@ -431,8 +431,8 @@ TEST_F(NameserverEntryTest, KeepRTT) {
     boost::shared_ptr<TestResolver> resolver(new TestResolver);
 
     // Ask the first time
-    entry->askIP(resolver, callback, V4_ONLY);
-    entry->askIP(resolver, callback, V6_ONLY);
+    entry->askIP(resolver.get(), callback, V4_ONLY);
+    entry->askIP(resolver.get(), callback, V6_ONLY);
     EXPECT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 0, 1));
     EXPECT_EQ(Fetchable::IN_PROGRESS, entry->getState());
     resolver->answer(0, Name(EXAMPLE_CO_UK), RRType::A(),
@@ -455,8 +455,8 @@ TEST_F(NameserverEntryTest, KeepRTT) {
 
     // Ask the second time. The callbacks should not fire right away and it
     // should request the addresses again
-    entry->askIP(resolver, callback, V4_ONLY);
-    entry->askIP(resolver, callback, V6_ONLY);
+    entry->askIP(resolver.get(), callback, V4_ONLY);
+    entry->askIP(resolver.get(), callback, V6_ONLY);
     EXPECT_EQ(2, callback->count);
     EXPECT_TRUE(resolver->asksIPs(Name(EXAMPLE_CO_UK), 2, 3));
     EXPECT_EQ(Fetchable::IN_PROGRESS, entry->getState());
diff --git a/src/lib/nsas/tests/nsas_test.h b/src/lib/nsas/tests/nsas_test.h
index 926e859..7500fc7 100644
--- a/src/lib/nsas/tests/nsas_test.h
+++ b/src/lib/nsas/tests/nsas_test.h
@@ -222,11 +222,6 @@ private:
 
 static const uint32_t HASHTABLE_DEFAULT_SIZE = 1009; ///< First prime above 1000
 
-} // namespace nsas
-} // namespace isc
-
-namespace {
-
 using namespace std;
 
 /*
@@ -245,6 +240,18 @@ class TestResolver : public isc::resolve::ResolverInterface {
     public:
         typedef pair<QuestionPtr, CallbackPtr> Request;
         vector<Request> requests;
+
+        /// \brief Destructor
+        ///
+        /// This is important.  All callbacks in the requests vector must be
+        /// called to remove them from internal loops.  Without this, destroying
+        /// the NSAS object will leave memory assigned.
+        ~TestResolver() {
+            for (size_t i = 0; i < requests.size(); ++i) {
+                requests[i].second->failure();
+            }
+        }
+
         virtual void resolve(const QuestionPtr& q, const CallbackPtr& c) {
             PresetAnswers::iterator it(answers_.find(*q));
             if (it == answers_.end()) {
@@ -420,6 +427,7 @@ protected:
     Name ns_name_;  ///< Nameserver name of ns.example.net
 };
 
-} // Empty namespace
+} // namespace nsas
+} // namespace isc
 
 #endif // __NSAS_TEST_H
diff --git a/src/lib/nsas/tests/random_number_generator_unittest.cc b/src/lib/nsas/tests/random_number_generator_unittest.cc
index c306b09..85cbcbf 100644
--- a/src/lib/nsas/tests/random_number_generator_unittest.cc
+++ b/src/lib/nsas/tests/random_number_generator_unittest.cc
@@ -59,11 +59,11 @@ private:
 // non-debug environment.
 // Note: the death test is not supported by all platforms.  We need to
 // compile tests using it selectively.
-#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST)
+#if !defined(NDEBUG)
 // Test of the constructor
 TEST_F(UniformRandomIntegerGeneratorTest, Constructor) {
     // The range must be min<=max
-    ASSERT_DEATH(UniformRandomIntegerGenerator(3, 2), "");
+    ASSERT_THROW(UniformRandomIntegerGenerator(3, 2), InvalidLimits);
 }
 #endif
 
@@ -109,30 +109,32 @@ TEST_F(WeightedRandomIntegerGeneratorTest, Constructor) {
 /// the tests will be failed since assert() is non-op in non-debug version.
 /// The "#ifndef NDEBUG" is added to make the tests be performed only in
 /// non-debug environment.
-#if !defined(NDEBUG) && defined(GTEST_HAS_DEATH_TEST)
+#if !defined(NDEBUG)
     //The probability must be >= 0
     probabilities.push_back(-0.1);
     probabilities.push_back(1.1);
-    ASSERT_DEATH(WeightedRandomIntegerGenerator gen2(probabilities), "");
+    ASSERT_THROW(WeightedRandomIntegerGenerator gen2(probabilities),
+                 InvalidProbValue);
 
     //The probability must be <= 1.0
     probabilities.clear();
     probabilities.push_back(0.1);
     probabilities.push_back(1.1);
-    ASSERT_DEATH(WeightedRandomIntegerGenerator gen3(probabilities), "");
+    ASSERT_THROW(WeightedRandomIntegerGenerator gen3(probabilities),
+                 InvalidProbValue);
 
     //The sum must be equal to 1.0
     probabilities.clear();
     probabilities.push_back(0.2);
     probabilities.push_back(0.9);
-    ASSERT_DEATH(WeightedRandomIntegerGenerator gen4(probabilities), "");
+    ASSERT_THROW(WeightedRandomIntegerGenerator gen4(probabilities), SumNotOne);
 
     //The sum must be equal to 1.0
     probabilities.clear();
     probabilities.push_back(0.3);
     probabilities.push_back(0.2);
     probabilities.push_back(0.1);
-    ASSERT_DEATH(WeightedRandomIntegerGenerator gen5(probabilities), "");
+    ASSERT_THROW(WeightedRandomIntegerGenerator gen5(probabilities), SumNotOne);
 #endif
 }
 
diff --git a/src/lib/nsas/tests/zone_entry_unittest.cc b/src/lib/nsas/tests/zone_entry_unittest.cc
index d10f12d..34f995c 100644
--- a/src/lib/nsas/tests/zone_entry_unittest.cc
+++ b/src/lib/nsas/tests/zone_entry_unittest.cc
@@ -47,7 +47,7 @@ class InheritedZoneEntry : public ZoneEntry {
             const std::string& name, const RRClass& class_code,
             boost::shared_ptr<HashTable<NameserverEntry> > nameserver_table,
             boost::shared_ptr<LruList<NameserverEntry> > nameserver_lru) :
-            ZoneEntry(resolver, name, class_code, nameserver_table,
+            ZoneEntry(resolver.get(), name, class_code, nameserver_table,
                 nameserver_lru)
         { }
         NameserverVector& nameservers() { return nameservers_; }
@@ -569,7 +569,7 @@ TEST_F(ZoneEntryTest, NameserverEntryReady) {
     // Inject the entry
     boost::shared_ptr<NameserverEntry> nse(injectEntry());
     // Fill it with data
-    nse->askIP(resolver_, nseCallback(), ANY_OK);
+    nse->askIP(resolver_.get(), nseCallback(), ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, nse->getState());
     EXPECT_TRUE(resolver_->asksIPs(ns_name_, 0, 1));
     EXPECT_NO_THROW(resolver_->answer(0, ns_name_, RRType::A(),
@@ -594,7 +594,7 @@ TEST_F(ZoneEntryTest, NameserverEntryNotAsked) {
 TEST_F(ZoneEntryTest, NameserverEntryInProgress) {
     // Prepare the nameserver entry
     boost::shared_ptr<NameserverEntry> nse(injectEntry());
-    nse->askIP(resolver_, nseCallback(), ANY_OK);
+    nse->askIP(resolver_.get(), nseCallback(), ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, nse->getState());
     EXPECT_TRUE(resolver_->asksIPs(ns_name_, 0, 1));
 
@@ -604,7 +604,7 @@ TEST_F(ZoneEntryTest, NameserverEntryInProgress) {
 /// \short Check Zone's reaction to found expired nameserver
 TEST_F(ZoneEntryTest, NameserverEntryExpired) {
     boost::shared_ptr<NameserverEntry> nse(injectEntry());
-    nse->askIP(resolver_, nseCallback(), ANY_OK);
+    nse->askIP(resolver_.get(), nseCallback(), ANY_OK);
     EXPECT_EQ(Fetchable::IN_PROGRESS, nse->getState());
     EXPECT_TRUE(resolver_->asksIPs(ns_name_, 0, 1));
     EXPECT_NO_THROW(resolver_->answer(0, ns_name_, RRType::A(),
@@ -623,7 +623,7 @@ TEST_F(ZoneEntryTest, NameserverEntryExpired) {
 /// \short Check how it reacts to an unreachable zone already in the table
 TEST_F(ZoneEntryTest, NameserverEntryUnreachable) {
     boost::shared_ptr<NameserverEntry> nse(injectEntry());
-    nse->askIP(resolver_, nseCallback(), ANY_OK);
+    nse->askIP(resolver_.get(), nseCallback(), ANY_OK);
     ASSERT_EQ(2, resolver_->requests.size());
     resolver_->requests[0].second->failure();
     resolver_->requests[1].second->failure();
diff --git a/src/lib/nsas/zone_entry.cc b/src/lib/nsas/zone_entry.cc
index 6d9c397..35cb79a 100644
--- a/src/lib/nsas/zone_entry.cc
+++ b/src/lib/nsas/zone_entry.cc
@@ -36,7 +36,7 @@ using namespace dns;
 namespace nsas {
 
 ZoneEntry::ZoneEntry(
-    boost::shared_ptr<isc::resolve::ResolverInterface> resolver,
+    isc::resolve::ResolverInterface* resolver,
     const std::string& name, const isc::dns::RRClass& class_code,
     boost::shared_ptr<HashTable<NameserverEntry> > nameserver_table,
     boost::shared_ptr<LruList<NameserverEntry> > nameserver_lru) :
diff --git a/src/lib/nsas/zone_entry.h b/src/lib/nsas/zone_entry.h
index 7d8651f..92ac75a 100644
--- a/src/lib/nsas/zone_entry.h
+++ b/src/lib/nsas/zone_entry.h
@@ -69,8 +69,7 @@ public:
      * \todo Move to cc file, include the lookup (if NSAS uses resolver for
      *     everything)
      */
-    ZoneEntry(
-        boost::shared_ptr<isc::resolve::ResolverInterface> resolver,
+    ZoneEntry(isc::resolve::ResolverInterface* resolver,
         const std::string& name, const isc::dns::RRClass& class_code,
         boost::shared_ptr<HashTable<NameserverEntry> > nameserver_table,
         boost::shared_ptr<LruList<NameserverEntry> > nameserver_lru);
@@ -158,7 +157,7 @@ private:
     void process(AddressFamily family,
         const boost::shared_ptr<NameserverEntry>& nameserver);
     // Resolver we use
-    boost::shared_ptr<isc::resolve::ResolverInterface> resolver_;
+    isc::resolve::ResolverInterface* resolver_;
     // We store the nameserver table and lru, so we can look up when there's
     // update
     boost::shared_ptr<HashTable<NameserverEntry> > nameserver_table_;
diff --git a/src/lib/resolve/tests/recursive_query_unittest_2.cc b/src/lib/resolve/tests/recursive_query_unittest_2.cc
index 7d2d150..21798b1 100644
--- a/src/lib/resolve/tests/recursive_query_unittest_2.cc
+++ b/src/lib/resolve/tests/recursive_query_unittest_2.cc
@@ -84,9 +84,12 @@ const char* WWW_EXAMPLE_ORG = "192.0.2.254";    ///< Address of www.example.org
 const bool DEBUG_PRINT = false;
 
 class MockResolver : public isc::resolve::ResolverInterface {
-    void resolve(const QuestionPtr& question,
+public:
+    virtual void resolve(const QuestionPtr& question,
                  const ResolverInterface::CallbackPtr& callback) {
     }
+
+    virtual ~MockResolver() {}
 };
 
 
@@ -116,8 +119,9 @@ public:
     QueryStatus     last_;                      ///< What was the last state
     QueryStatus     expected_;                  ///< Expected next state
     OutputBufferPtr question_buffer_;           ///< Question we expect to receive
-    isc::nsas::NameserverAddressStore* nsas_;
-    isc::cache::ResolverCache cache_;
+    boost::shared_ptr<MockResolver> resolver_;  ///< Mock resolver
+    isc::nsas::NameserverAddressStore* nsas_;   ///< Nameserver address store
+    isc::cache::ResolverCache cache_;           ///< Resolver cache
 
     // Data for TCP Server
     size_t          tcp_cumulative_;            ///< Cumulative TCP data received
@@ -143,6 +147,8 @@ public:
         last_(NONE),
         expected_(NONE),
         question_buffer_(new OutputBuffer(BUFFER_SIZE)),
+        resolver_(new MockResolver()),
+        nsas_(new isc::nsas::NameserverAddressStore(resolver_)),
         tcp_cumulative_(0),
         tcp_endpoint_(asio::ip::address::from_string(TEST_ADDRESS), TEST_PORT),
         tcp_length_(0),
@@ -155,8 +161,6 @@ public:
         udp_send_buffer_(new OutputBuffer(BUFFER_SIZE)),
         udp_socket_(service_.get_io_service(), udp::v4())
     {
-        boost::shared_ptr<MockResolver>mock_resolver(new MockResolver());
-        nsas_ = new isc::nsas::NameserverAddressStore(mock_resolver);
     }
 
     /// \brief Set Common Message Bits
@@ -632,8 +636,7 @@ TEST_F(RecursiveQueryTest2, Resolve) {
                          upstream, upstream_root);
     query.setTestServer(TEST_ADDRESS, TEST_PORT);
 
-    // Set up callback for the tor eceive notification that the query has
-    // completed.
+    // Set up callback to receive notification that the query has completed.
     isc::resolve::ResolverInterface::CallbackPtr
         resolver_callback(new ResolverCallback(service_));
 




More information about the bind10-changes mailing list