BIND 10 trac641, updated. ddb07c03439bba577baf4f0c1462cbb8749d2d0e [trac641] First part of tackling memory leaks
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Mar 15 16:10:00 UTC 2011
The branch, trac641 has been updated
via ddb07c03439bba577baf4f0c1462cbb8749d2d0e (commit)
from fdfe3422ffc3d7f6eb44114d02a0a3759d4dee7c (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 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 | 4 +-
src/lib/nsas/nameserver_address_store.h | 2 +-
src/lib/nsas/nameserver_entry.cc | 6 +--
src/lib/nsas/nameserver_entry.h | 4 +-
.../tests/nameserver_address_store_unittest.cc | 2 +-
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 | 8 +----
src/lib/nsas/tests/zone_entry_unittest.cc | 10 +++---
src/lib/nsas/zone_entry.cc | 2 +-
src/lib/nsas/zone_entry.h | 5 +--
11 files changed, 40 insertions(+), 45 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/nsas/nameserver_address_store.cc b/src/lib/nsas/nameserver_address_store.cc
index 4efe491..9a8149d 100644
--- a/src/lib/nsas/nameserver_address_store.cc
+++ b/src/lib/nsas/nameserver_address_store.cc
@@ -53,7 +53,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 {
@@ -66,7 +66,7 @@ namespace {
*/
boost::shared_ptr<ZoneEntry>
newZone(
- const boost::shared_ptr<isc::resolve::ResolverInterface>* resolver,
+ isc::resolve::ResolverInterface** const resolver,
const string* zone, const RRClass* class_code,
const boost::shared_ptr<HashTable<NameserverEntry> >* ns_hash,
const boost::shared_ptr<LruList<NameserverEntry> >* ns_lru)
diff --git a/src/lib/nsas/nameserver_address_store.h b/src/lib/nsas/nameserver_address_store.h
index 9804a55..18ae32e 100644
--- a/src/lib/nsas/nameserver_address_store.h
+++ b/src/lib/nsas/nameserver_address_store.h
@@ -115,7 +115,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 f6c2e8c..b54b7c3 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/tests/nameserver_address_store_unittest.cc b/src/lib/nsas/tests/nameserver_address_store_unittest.cc
index 95b46a8..f07ffe6 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> >())));
}
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..9262c8c 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;
/*
@@ -420,6 +415,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/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 36e01d2..4023a5e 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 a1f12bc..a721efa 100644
--- a/src/lib/nsas/zone_entry.h
+++ b/src/lib/nsas/zone_entry.h
@@ -68,8 +68,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);
@@ -153,7 +152,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_;
More information about the bind10-changes
mailing list