BIND 10 trac2836, updated. 44160bfeffd56f5ccd6c8400eb8c3e883ac10b19 [2836] Check for collision on the named address
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon May 13 12:01:16 UTC 2013
The branch, trac2836 has been updated
via 44160bfeffd56f5ccd6c8400eb8c3e883ac10b19 (commit)
via 29f6d05f66afabcd46c532622b10b370b65d3090 (commit)
from f941f042dc2c953390354434e9a4563cb67b14e0 (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 44160bfeffd56f5ccd6c8400eb8c3e883ac10b19
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon May 13 13:55:33 2013 +0200
[2836] Check for collision on the named address
commit 29f6d05f66afabcd46c532622b10b370b65d3090
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Mon May 13 13:19:54 2013 +0200
[2836] Store the address for whole lifetime
Store the named address for the whole lifetime of the zone data updater.
This should be better for performance.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/zone_data_updater.cc | 10 ----
src/lib/datasrc/memory/zone_data_updater.h | 18 +++++--
.../tests/memory/zone_data_updater_unittest.cc | 12 +++++
.../datasrc/tests/memory/zone_finder_unittest.cc | 56 ++++++++++----------
4 files changed, 54 insertions(+), 42 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_data_updater.cc b/src/lib/datasrc/memory/zone_data_updater.cc
index 77084a6..5b4f1e7 100644
--- a/src/lib/datasrc/memory/zone_data_updater.cc
+++ b/src/lib/datasrc/memory/zone_data_updater.cc
@@ -417,7 +417,6 @@ ZoneDataUpdater::add(const ConstRRsetPtr& rrset,
// Store the address, it may change during growth and the address inside
// would get updated.
- mem_sgmt_.setNamedAddress("updater_zone_data", zone_data_);
bool added = false;
do {
try {
@@ -429,18 +428,9 @@ ZoneDataUpdater::add(const ConstRRsetPtr& rrset,
zone_data_ =
static_cast<ZoneData*>(
mem_sgmt_.getNamedAddress("updater_zone_data"));
- } catch (...) {
- // In case of other exceptions, they are propagated. But clean up
- // the temporary address stored there (this is shorter than
- // RAII class in this case).
- mem_sgmt_.clearNamedAddress("updater_zone_data");
- throw;
}
// Retry if it didn't add due to the growth
} while (!added);
-
- // Clean up the named address
- mem_sgmt_.clearNamedAddress("updater_zone_data");
}
} // namespace memory
diff --git a/src/lib/datasrc/memory/zone_data_updater.h b/src/lib/datasrc/memory/zone_data_updater.h
index 2ac7c8a..4f8351e 100644
--- a/src/lib/datasrc/memory/zone_data_updater.h
+++ b/src/lib/datasrc/memory/zone_data_updater.h
@@ -57,14 +57,15 @@ public:
/// The constructor.
///
- /// \throw none
- ///
/// \param mem_sgmt The memory segment used for the zone data.
/// \param rrclass The RRclass of the zone data.
/// \param zone_name The Name of the zone under which records will be
/// added.
- // \param zone_data The ZoneData object which is populated with
- // record data.
+ /// \param zone_data The ZoneData object which is populated with
+ /// record data.
+ /// \throw InvalidOperation if there's already a zone data updater
+ /// on the given memory segment. Currently, at most one zone data
+ /// updater may exist on the same memory segment.
ZoneDataUpdater(util::MemorySegment& mem_sgmt,
isc::dns::RRClass rrclass,
const isc::dns::Name& zone_name,
@@ -74,10 +75,17 @@ public:
zone_name_(zone_name),
hash_(NULL),
zone_data_(&zone_data)
- {}
+ {
+ if (mem_sgmt_.getNamedAddress("updater_zone_data")) {
+ isc_throw(isc::InvalidOperation, "A ZoneDataUpdater already exists"
+ " on this memory segment. Destroy it first.");
+ }
+ mem_sgmt_.setNamedAddress("updater_zone_data", zone_data_);
+ }
/// The destructor.
~ZoneDataUpdater() {
+ mem_sgmt_.clearNamedAddress("updater_zone_data");
delete hash_;
}
diff --git a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
index 752df21..016df0d 100644
--- a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
@@ -92,6 +92,8 @@ protected:
~ZoneDataUpdaterTest() {
if (updater_) {
ZoneData::destroy(*mem_sgmt_, updater_->getZoneData(), zclass_);
+ // Release the updater, so it frees all memory inside the segment too
+ updater_.reset();
}
if (!mem_sgmt_->allMemoryDeallocated()) {
ADD_FAILURE() << "Memory leak detected";
@@ -102,6 +104,7 @@ protected:
void clearZoneData() {
assert(updater_);
ZoneData::destroy(*mem_sgmt_, updater_->getZoneData(), zclass_);
+ updater_.reset();
updater_.reset(new TestZoneDataUpdater(*mem_sgmt_, zclass_, zname_,
*ZoneData::create(*mem_sgmt_,
zname_)));
@@ -334,4 +337,13 @@ TEST_P(ZoneDataUpdaterTest, manySmallRRsets) {
}
}
+TEST_P(ZoneDataUpdaterTest, updaterCollision) {
+ ZoneData* zone_data = ZoneData::create(*mem_sgmt_,
+ Name("another.example.com."));
+ EXPECT_THROW(ZoneDataUpdater(*mem_sgmt_, RRClass::IN(),
+ Name("another.example.com."), *zone_data),
+ isc::InvalidOperation);
+ ZoneData::destroy(*mem_sgmt_, zone_data, RRClass::IN());
+}
+
}
diff --git a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc
index b1ebaf4..cf07107 100644
--- a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc
@@ -97,7 +97,7 @@ protected:
origin_("example.org"),
zone_data_(ZoneData::create(mem_sgmt_, origin_)),
zone_finder_(*zone_data_, class_),
- updater_(mem_sgmt_, class_, origin_, *zone_data_)
+ updater_(new ZoneDataUpdater(mem_sgmt_, class_, origin_, *zone_data_))
{
// Build test RRsets. Below, we construct an RRset for
// each textual RR(s) of zone_data, and assign it to the corresponding
@@ -195,7 +195,7 @@ protected:
}
void addToZoneData(const ConstRRsetPtr rrset) {
- updater_.add(rrset, rrset->getRRsig());
+ updater_->add(rrset, rrset->getRRsig());
}
/// \brief expensive rrset converter
@@ -223,7 +223,7 @@ protected:
MemorySegmentTest mem_sgmt_;
memory::ZoneData* zone_data_;
memory::InMemoryZoneFinder zone_finder_;
- ZoneDataUpdater updater_;
+ boost::scoped_ptr<ZoneDataUpdater> updater_;
// Placeholder for storing RRsets to be checked with rrsetsCheck()
vector<ConstRRsetPtr> actual_rrsets_;
@@ -1548,19 +1548,19 @@ TEST_F(InMemoryZoneFinderTest, findOrphanRRSIG) {
// Add A for ns.example.org, and RRSIG-only covering TXT for the same name.
// query for the TXT should result in NXRRSET.
addToZoneData(rr_ns_a_);
- updater_.add(ConstRRsetPtr(),
- textToRRset(
- "ns.example.org. 300 IN RRSIG TXT 5 3 300 20120814220826 "
- "20120715220826 1234 example.com. FAKE"));
+ updater_->add(ConstRRsetPtr(),
+ textToRRset(
+ "ns.example.org. 300 IN RRSIG TXT 5 3 300 20120814220826 "
+ "20120715220826 1234 example.com. FAKE"));
findTest(Name("ns.example.org"), RRType::TXT(),
ZoneFinder::NXRRSET, true, ConstRRsetPtr(), expected_flags);
// Add RRSIG-only covering NSEC. This shouldn't be returned when NSEC is
// requested, whether it's for NXRRSET or NXDOMAIN
- updater_.add(ConstRRsetPtr(),
- textToRRset(
- "ns.example.org. 300 IN RRSIG NSEC 5 3 300 "
- "20120814220826 20120715220826 1234 example.com. FAKE"));
+ updater_->add(ConstRRsetPtr(),
+ textToRRset(
+ "ns.example.org. 300 IN RRSIG NSEC 5 3 300 "
+ "20120814220826 20120715220826 1234 example.com. FAKE"));
// The added RRSIG for NSEC could be used for NXRRSET but shouldn't
findTest(Name("ns.example.org"), RRType::TXT(),
ZoneFinder::NXRRSET, true, ConstRRsetPtr(),
@@ -1571,29 +1571,29 @@ TEST_F(InMemoryZoneFinderTest, findOrphanRRSIG) {
expected_flags, NULL, ZoneFinder::FIND_DNSSEC);
// RRSIG-only CNAME shouldn't be accidentally confused with real CNAME.
- updater_.add(ConstRRsetPtr(),
- textToRRset(
- "nocname.example.org. 300 IN RRSIG CNAME 5 3 300 "
- "20120814220826 20120715220826 1234 example.com. FAKE"));
+ updater_->add(ConstRRsetPtr(),
+ textToRRset(
+ "nocname.example.org. 300 IN RRSIG CNAME 5 3 300 "
+ "20120814220826 20120715220826 1234 example.com. FAKE"));
findTest(Name("nocname.example.org"), RRType::A(),
ZoneFinder::NXRRSET, true, ConstRRsetPtr(), expected_flags);
// RRSIG-only for NS wouldn't invoke delegation anyway, but we check this
// case explicitly.
- updater_.add(ConstRRsetPtr(),
- textToRRset(
- "nodelegation.example.org. 300 IN RRSIG NS 5 3 300 "
- "20120814220826 20120715220826 1234 example.com. FAKE"));
+ updater_->add(ConstRRsetPtr(),
+ textToRRset(
+ "nodelegation.example.org. 300 IN RRSIG NS 5 3 300 "
+ "20120814220826 20120715220826 1234 example.com. FAKE"));
findTest(Name("nodelegation.example.org"), RRType::A(),
ZoneFinder::NXRRSET, true, ConstRRsetPtr(), expected_flags);
findTest(Name("www.nodelegation.example.org"), RRType::A(),
ZoneFinder::NXDOMAIN, true, ConstRRsetPtr(), expected_flags);
// Same for RRSIG-only for DNAME
- updater_.add(ConstRRsetPtr(),
- textToRRset(
- "nodname.example.org. 300 IN RRSIG DNAME 5 3 300 "
- "20120814220826 20120715220826 1234 example.com. FAKE"));
+ updater_->add(ConstRRsetPtr(),
+ textToRRset(
+ "nodname.example.org. 300 IN RRSIG DNAME 5 3 300 "
+ "20120814220826 20120715220826 1234 example.com. FAKE"));
findTest(Name("www.nodname.example.org"), RRType::A(),
ZoneFinder::NXDOMAIN, true, ConstRRsetPtr(), expected_flags);
// If we have a delegation NS at this node, it will be a bit trickier,
@@ -1612,6 +1612,7 @@ TEST_F(InMemoryZoneFinderTest, findOrphanRRSIG) {
TEST_F(InMemoryZoneFinderTest, NSECNonExistentTest) {
shared_ptr<ZoneTableSegment> ztable_segment(
new ZoneTableSegmentTest(class_, mem_sgmt_));
+ updater_.reset();
InMemoryClient client(ztable_segment, class_);
Name name("example.com.");
@@ -1756,10 +1757,10 @@ TEST_F(InMemoryZoneFinderNSEC3Test, RRSIGOnly) {
// add an RRSIG-only NSEC3 to the NSEC3 space, and try to find it; it
// should result in an exception.
const string n8_hash = "ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ";
- updater_.add(ConstRRsetPtr(),
- textToRRset(
- n8_hash + ".example.org. 300 IN RRSIG NSEC3 5 3 300 "
- "20120814220826 20120715220826 1234 example.com. FAKE"));
+ updater_->add(ConstRRsetPtr(),
+ textToRRset(
+ n8_hash + ".example.org. 300 IN RRSIG NSEC3 5 3 300 "
+ "20120814220826 20120715220826 1234 example.com. FAKE"));
EXPECT_THROW(zone_finder_.findNSEC3(Name("n8.example.org"), false),
DataSourceError);
}
@@ -1773,6 +1774,7 @@ TEST_F(InMemoryZoneFinderNSEC3Test, findNSEC3MissingOrigin) {
shared_ptr<ZoneTableSegment> ztable_segment(
new ZoneTableSegmentTest(class_, mem_sgmt_));
+ updater_.reset();
InMemoryClient client(ztable_segment, class_);
Name name("example.com.");
More information about the bind10-changes
mailing list