BIND 10 trac2836-2, updated. 575854a507052ad7de51aa259fd5d8f04156d15d [2836-2] catch and handle MemorySegmentGrown from ZoneData::create.
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon May 13 18:15:27 UTC 2013
The branch, trac2836-2 has been updated
via 575854a507052ad7de51aa259fd5d8f04156d15d (commit)
via 4774e482497ea5ea4326a9c3fe340d5cae56dd8e (commit)
via 4977a9731be4b7e2ecd11983e89259d69d22824d (commit)
via fa351ca87769fcdbe8aa934eadb96afc3e578c2b (commit)
via 2eff0d6c57c2dcbc163b1c84e55095acc5995c9c (commit)
via 423740919b7a050dc1255a0ebdc6ab6b71ea777f (commit)
via f0229b977d354a137df49137655eb3ec56d96f97 (commit)
via 2cab68004571463fdaeefe5659380fbbf9c54e01 (commit)
via 807f3b5cd6b5f7daed92413e9ab771ec500b6536 (commit)
via 772de981b14ae73731b54e6dcc5b84935584f1ce (commit)
via bd04d06c2d51af45ed818e90ef500936dafca1c4 (commit)
via 44160bfeffd56f5ccd6c8400eb8c3e883ac10b19 (commit)
via 29f6d05f66afabcd46c532622b10b370b65d3090 (commit)
via f941f042dc2c953390354434e9a4563cb67b14e0 (commit)
via 54b12f14d032796a8d6638cdbbf6ba5d63835478 (commit)
via 689aa37ea4cc087ad0affeedb2b4f8de5b29919d (commit)
from f4ca083e427788a22a4979a104b517a5a4ef3df9 (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 575854a507052ad7de51aa259fd5d8f04156d15d
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon May 13 11:14:37 2013 -0700
[2836-2] catch and handle MemorySegmentGrown from ZoneData::create.
this can happen, although it didn't with unit tests in my own environment.
commit 4774e482497ea5ea4326a9c3fe340d5cae56dd8e
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Mon May 13 11:11:46 2013 -0700
[2836-2] handle the case where NSEC3Data::create() causes segment extension.
this can cause a crash.
commit 4977a9731be4b7e2ecd11983e89259d69d22824d
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon May 13 14:06:58 2013 +0530
[2836-2] [2850] Fix MemorySegmentMapped::allMemoryDeallocated() and make it non-const
commit fa351ca87769fcdbe8aa934eadb96afc3e578c2b
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon May 13 13:47:45 2013 +0530
[2836-2] [2850] Looks like flush() finalizes base_sgmt_, so we free before that
commit 2eff0d6c57c2dcbc163b1c84e55095acc5995c9c
Author: Mukund Sivaraman <muks at isc.org>
Date: Mon May 13 13:29:05 2013 +0530
[2836-2] [2850] Pre-reserve some memory to workaround relocations in setNamedAddress()
This commit introduces a problem that allMemoryDeallocated() doesn't
work anymore, because the reserved memory is freed only when the segment
is destroyed.
One workaround is to temporarily release and re-reserve this memory in
allMemoryDeallocated(), but that would make it a non-const method. I
don't see any other clean way of doing this.
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/domaintree.h | 6 ++
src/lib/datasrc/memory/zone_data_loader.cc | 10 ++-
src/lib/datasrc/memory/zone_data_updater.cc | 15 ++--
src/lib/datasrc/memory/zone_data_updater.h | 23 ++---
src/lib/datasrc/memory/zone_table.h | 2 +
src/lib/datasrc/memory/zone_writer_local.cc | 26 ++++--
.../tests/memory/segment_object_holder_unittest.cc | 4 +
.../tests/memory/zone_data_loader_unittest.cc | 36 ++++++++
.../tests/memory/zone_data_updater_unittest.cc | 94 +++++++++++---------
.../datasrc/tests/memory/zone_finder_unittest.cc | 56 ++++++------
src/lib/util/memory_segment.h | 2 +-
src/lib/util/memory_segment_local.cc | 2 +-
src/lib/util/memory_segment_local.h | 2 +-
src/lib/util/memory_segment_mapped.cc | 66 +++++++++++++-
src/lib/util/memory_segment_mapped.h | 2 +-
.../util/tests/memory_segment_mapped_unittest.cc | 9 +-
16 files changed, 245 insertions(+), 110 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/domaintree.h b/src/lib/datasrc/memory/domaintree.h
index 6e8b062..7bd840b 100644
--- a/src/lib/datasrc/memory/domaintree.h
+++ b/src/lib/datasrc/memory/domaintree.h
@@ -1311,6 +1311,12 @@ public:
/// the same. This method provides the weak exception guarantee in its
/// normal sense.
///
+ /// In particular, this method can propagate the \c MemorySegmentGrown
+ /// exception from the \c MemorySegment object. In such case, some of
+ /// the internal nodes may have been already allocated (but hold no data).
+ /// Retrying the insert (possibly multiple times) would lead to the same
+ /// structure eventually.
+ ///
/// \param mem_sgmt A \c MemorySegment object for allocating memory of
/// a new node to be inserted. Must be the same segment as that used
/// for creating the tree itself.
diff --git a/src/lib/datasrc/memory/zone_data_loader.cc b/src/lib/datasrc/memory/zone_data_loader.cc
index 4dd008e..8bfb618 100644
--- a/src/lib/datasrc/memory/zone_data_loader.cc
+++ b/src/lib/datasrc/memory/zone_data_loader.cc
@@ -182,8 +182,14 @@ loadZoneDataInternal(util::MemorySegment& mem_sgmt,
const Name& zone_name,
boost::function<void(LoadCallback)> rrset_installer)
{
- SegmentObjectHolder<ZoneData, RRClass> holder(
- mem_sgmt, ZoneData::create(mem_sgmt, zone_name), rrclass);
+ ZoneData* zone_data = NULL;
+ while (!zone_data) {
+ try {
+ zone_data = ZoneData::create(mem_sgmt, zone_name);
+ } catch (const isc::util::MemorySegmentGrown&) {}
+ }
+ SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, zone_data,
+ rrclass);
ZoneDataLoader loader(mem_sgmt, rrclass, zone_name, *holder.get());
rrset_installer(boost::bind(&ZoneDataLoader::addFromLoad, &loader, _1));
diff --git a/src/lib/datasrc/memory/zone_data_updater.cc b/src/lib/datasrc/memory/zone_data_updater.cc
index 77084a6..4ba7c25 100644
--- a/src/lib/datasrc/memory/zone_data_updater.cc
+++ b/src/lib/datasrc/memory/zone_data_updater.cc
@@ -233,7 +233,12 @@ ZoneDataUpdater::setupNSEC3(const ConstRRsetPtr rrset) {
NSEC3Data* nsec3_data = zone_data_->getNSEC3Data();
if (nsec3_data == NULL) {
+ // NSEC3Data::create() can internally cause segment extension, making
+ // zone_data_ invalidated. This is tricky and inconvenient, but we
+ // need to deal with it.
nsec3_data = NSEC3Data::create(mem_sgmt_, zone_name_, nsec3_rdata);
+ zone_data_ = static_cast<ZoneData*>(
+ mem_sgmt_.getNamedAddress("updater_zone_data"));
zone_data_->setNSEC3Data(nsec3_data);
zone_data_->setSigned(true);
} else {
@@ -417,7 +422,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 +433,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..c13b8d3 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_;
}
@@ -192,11 +200,6 @@ private:
const isc::dns::Name& zone_name_;
RdataEncoder encoder_;
const isc::dns::NSEC3Hash* hash_;
-protected:
- /// \brief The zone data
- ///
- /// Protected, so the tests can get in. But it should not be accessed
- /// in general code.
ZoneData* zone_data_;
};
diff --git a/src/lib/datasrc/memory/zone_table.h b/src/lib/datasrc/memory/zone_table.h
index 1b369b9..b487855 100644
--- a/src/lib/datasrc/memory/zone_table.h
+++ b/src/lib/datasrc/memory/zone_table.h
@@ -144,6 +144,8 @@ public:
/// This method adds a given zone data to the internal table.
///
/// \throw std::bad_alloc Internal resource allocation fails.
+ /// \throw MemorySegmentGrown when the memory segment grown and
+ /// possibly relocated.
///
/// \param mem_sgmt The \c MemorySegment to allocate zone data to be
/// created. It must be the same segment that was used to create
diff --git a/src/lib/datasrc/memory/zone_writer_local.cc b/src/lib/datasrc/memory/zone_writer_local.cc
index 0cd9587..01827a3 100644
--- a/src/lib/datasrc/memory/zone_writer_local.cc
+++ b/src/lib/datasrc/memory/zone_writer_local.cc
@@ -64,17 +64,25 @@ ZoneWriterLocal::install() {
isc_throw(isc::InvalidOperation, "No data to install");
}
-
- ZoneTable* table(segment_->getHeader().getTable());
- if (table == NULL) {
- isc_throw(isc::Unexpected, "No zone table present");
+ // FIXME: This retry is currently untested, as there seems to be no
+ // reasonable way to create a zone table segment with non-local memory
+ // segment. Once there is, we should provide the test.
+ while (state_ != ZW_INSTALLED) {
+ try {
+ ZoneTable* table(segment_->getHeader().getTable());
+ if (table == NULL) {
+ isc_throw(isc::Unexpected, "No zone table present");
+ }
+ const ZoneTable::AddResult result(table->addZone(
+ segment_->getMemorySegment(),
+ rrclass_, origin_,
+ zone_data_));
+ state_ = ZW_INSTALLED;
+ zone_data_ = result.zone_data;
+ } catch (const isc::util::MemorySegmentGrown&) {
+ }
}
- const ZoneTable::AddResult result(table->addZone(
- segment_->getMemorySegment(),
- rrclass_, origin_, zone_data_));
- state_ = ZW_INSTALLED;
- zone_data_ = result.zone_data;
}
void
diff --git a/src/lib/datasrc/tests/memory/segment_object_holder_unittest.cc b/src/lib/datasrc/tests/memory/segment_object_holder_unittest.cc
index 1c21766..70d331e 100644
--- a/src/lib/datasrc/tests/memory/segment_object_holder_unittest.cc
+++ b/src/lib/datasrc/tests/memory/segment_object_holder_unittest.cc
@@ -88,7 +88,11 @@ allocateUntilGrows(MemorySegment& segment, size_t& current_size) {
// Check that the segment thing releases stuff even in case it throws
// SegmentGrown exception and the thing moves address
+#ifdef USE_SHARED_MEMORY
TEST(SegmentObjectHolderTest, grow) {
+#else
+TEST(SegmentObjectHolderTest, DISABLED_grow) {
+#endif
MemorySegmentMapped segment(mapped_file,
isc::util::MemorySegmentMapped::CREATE_ONLY);
// Allocate a bit of memory, to get a unique address
diff --git a/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc
index abc6f13..a9aaea3 100644
--- a/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc
@@ -16,11 +16,16 @@
#include <datasrc/memory/rdataset.h>
#include <datasrc/memory/zone_data.h>
#include <datasrc/memory/zone_data_updater.h>
+#include <datasrc/memory/segment_object_holder.h>
+#include <datasrc/zone_iterator.h>
#include <util/buffer.h>
#include <dns/name.h>
#include <dns/rrclass.h>
+#include <dns/rdataclass.h>
+#include <util/memory_segment_mapped.h>
+#include <util/memory_segment_local.h>
#include "memory_segment_test.h"
@@ -28,9 +33,13 @@
using namespace isc::dns;
using namespace isc::datasrc::memory;
+using isc::util::MemorySegmentMapped;
+using isc::datasrc::memory::detail::SegmentObjectHolder;
namespace {
+const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
+
class ZoneDataLoaderTest : public ::testing::Test {
protected:
ZoneDataLoaderTest() : zclass_(RRClass::IN()), zone_data_(NULL) {}
@@ -73,4 +82,31 @@ TEST_F(ZoneDataLoaderTest, zoneMinTTL) {
EXPECT_EQ(RRTTL(1200), RRTTL(b));
}
+// Load bunch of small zones, hoping some of the relocation will happen
+// during the memory creation, not only Rdata creation.
+TEST(ZoneDataLoaterTest, relocate) {
+ MemorySegmentMapped segment(mapped_file,
+ isc::util::MemorySegmentMapped::CREATE_ONLY,
+ 4096);
+ const size_t zone_count = 10000;
+ typedef SegmentObjectHolder<ZoneData, RRClass> Holder;
+ typedef boost::shared_ptr<Holder> HolderPtr;
+ std::vector<HolderPtr> zones;
+ for (size_t i = 0; i < zone_count; ++i) {
+ // Load some zone
+ ZoneData* data = loadZoneData(segment, RRClass::IN(),
+ Name("example.org"),
+ TEST_DATA_DIR
+ "/example.org-nsec3-signed.zone");
+ // Store it, so it is cleaned up later
+ zones.push_back(HolderPtr(new Holder(segment, data,
+ RRClass::IN())));
+
+ }
+ // Deallocate all the zones now.
+ zones.clear();
+ EXPECT_TRUE(segment.allMemoryDeallocated());
+ EXPECT_EQ(0, unlink(mapped_file));
+}
+
}
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..46a671b 100644
--- a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc
@@ -49,6 +49,7 @@ const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
// test, so we have different factories for them.
class SegmentCreator {
public:
+ virtual ~SegmentCreator() {}
typedef boost::shared_ptr<isc::util::MemorySegment> SegmentPtr;
// Create the segment.
virtual SegmentPtr create() const = 0;
@@ -67,32 +68,24 @@ getNode(isc::util::MemorySegment& mem_sgmt, const Name& name,
return (node);
}
-// Just the same as ZoneDataUpdater, but it lets get in to some guts.
-class TestZoneDataUpdater : public ZoneDataUpdater {
-public:
- TestZoneDataUpdater(isc::util::MemorySegment& mem_sgmt,
- isc::dns::RRClass rrclass,
- const isc::dns::Name& zone_name,
- ZoneData& zone_data):
- ZoneDataUpdater(mem_sgmt, rrclass, zone_name, zone_data)
- {}
- ZoneData* getZoneData() const { return (zone_data_); }
-};
-
class ZoneDataUpdaterTest : public ::testing::TestWithParam<SegmentCreator*> {
protected:
ZoneDataUpdaterTest() :
zname_("example.org"), zclass_(RRClass::IN()),
- mem_sgmt_(GetParam()->create()),
- updater_(new
- TestZoneDataUpdater(*mem_sgmt_, zclass_, zname_,
- *ZoneData::create(*mem_sgmt_, zname_)))
- {}
+ mem_sgmt_(GetParam()->create())
+ {
+ ZoneData *data = ZoneData::create(*mem_sgmt_, zname_);
+ mem_sgmt_->setNamedAddress("Test zone data", data);
+ updater_.reset(new ZoneDataUpdater(*mem_sgmt_, zclass_, zname_,
+ *data));
+ }
~ZoneDataUpdaterTest() {
- if (updater_) {
- ZoneData::destroy(*mem_sgmt_, updater_->getZoneData(), zclass_);
- }
+ assert(updater_);
+ ZoneData::destroy(*mem_sgmt_, getZoneData(), zclass_);
+ // Release the updater, so it frees all memory inside the segment too
+ updater_.reset();
+ mem_sgmt_->clearNamedAddress("Test zone data");
if (!mem_sgmt_->allMemoryDeallocated()) {
ADD_FAILURE() << "Memory leak detected";
}
@@ -101,16 +94,24 @@ protected:
void clearZoneData() {
assert(updater_);
- ZoneData::destroy(*mem_sgmt_, updater_->getZoneData(), zclass_);
- updater_.reset(new TestZoneDataUpdater(*mem_sgmt_, zclass_, zname_,
- *ZoneData::create(*mem_sgmt_,
- zname_)));
+ ZoneData::destroy(*mem_sgmt_, getZoneData(), zclass_);
+ mem_sgmt_->clearNamedAddress("Test zone data");
+ updater_.reset();
+ ZoneData *data = ZoneData::create(*mem_sgmt_, zname_);
+ mem_sgmt_->setNamedAddress("Test zone data", data);
+ updater_.reset(new ZoneDataUpdater(*mem_sgmt_, zclass_, zname_,
+ *data));
+ }
+
+ ZoneData* getZoneData() {
+ return (static_cast<ZoneData*>(
+ mem_sgmt_->getNamedAddress("Test zone data")));
}
const Name zname_;
const RRClass zclass_;
boost::shared_ptr<isc::util::MemorySegment> mem_sgmt_;
- boost::scoped_ptr<TestZoneDataUpdater> updater_;
+ boost::scoped_ptr<ZoneDataUpdater> updater_;
};
class TestSegmentCreator : public SegmentCreator {
@@ -160,6 +161,7 @@ private:
const size_t initial_size_;
};
+#ifdef USE_SHARED_MEMORY
// There should be no initialization fiasco there. We only set int value inside
// and don't use it until the create() is called.
MappedSegmentCreator small_creator(4092), default_creator;
@@ -167,6 +169,7 @@ MappedSegmentCreator small_creator(4092), default_creator;
INSTANTIATE_TEST_CASE_P(MappedSegment, ZoneDataUpdaterTest, ::testing::Values(
static_cast<SegmentCreator*>(&small_creator),
static_cast<SegmentCreator*>(&default_creator)));
+#endif
TEST_P(ZoneDataUpdaterTest, bothNull) {
// At least either covered RRset or RRSIG must be non NULL.
@@ -180,8 +183,7 @@ TEST_P(ZoneDataUpdaterTest, zoneMinTTL) {
"example.org. 3600 IN SOA . . 0 0 0 0 1200",
zclass_, zname_),
ConstRRsetPtr());
- isc::util::InputBuffer b(updater_->getZoneData()->getMinTTLData(),
- sizeof(uint32_t));
+ isc::util::InputBuffer b(getZoneData()->getMinTTLData(), sizeof(uint32_t));
EXPECT_EQ(RRTTL(1200), RRTTL(b));
}
@@ -192,7 +194,7 @@ TEST_P(ZoneDataUpdaterTest, rrsigOnly) {
"www.example.org. 3600 IN RRSIG A 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
ZoneNode* node = getNode(*mem_sgmt_, Name("www.example.org"),
- updater_->getZoneData());
+ getZoneData());
const RdataSet* rdset = node->getData();
ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
rdset = RdataSet::find(rdset, RRType::A(), true);
@@ -210,8 +212,7 @@ TEST_P(ZoneDataUpdaterTest, rrsigOnly) {
updater_->add(ConstRRsetPtr(), textToRRset(
"*.wild.example.org. 3600 IN RRSIG A 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- node = getNode(*mem_sgmt_, Name("wild.example.org"),
- updater_->getZoneData());
+ node = getNode(*mem_sgmt_, Name("wild.example.org"), getZoneData());
EXPECT_TRUE(node->getFlag(ZoneData::WILDCARD_NODE));
// Simply adding RRSIG covering (delegating NS) shouldn't enable callback
@@ -219,16 +220,14 @@ TEST_P(ZoneDataUpdaterTest, rrsigOnly) {
updater_->add(ConstRRsetPtr(), textToRRset(
"child.example.org. 3600 IN RRSIG NS 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- node = getNode(*mem_sgmt_, Name("child.example.org"),
- updater_->getZoneData());
+ node = getNode(*mem_sgmt_, Name("child.example.org"), getZoneData());
EXPECT_FALSE(node->getFlag(ZoneNode::FLAG_CALLBACK));
// Same for DNAME
updater_->add(ConstRRsetPtr(), textToRRset(
"dname.example.org. 3600 IN RRSIG DNAME 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- node = getNode(*mem_sgmt_, Name("dname.example.org"),
- updater_->getZoneData());
+ node = getNode(*mem_sgmt_, Name("dname.example.org"), getZoneData());
EXPECT_FALSE(node->getFlag(ZoneNode::FLAG_CALLBACK));
// Likewise, RRSIG for NSEC3PARAM alone shouldn't make the zone
@@ -236,13 +235,13 @@ TEST_P(ZoneDataUpdaterTest, rrsigOnly) {
updater_->add(ConstRRsetPtr(), textToRRset(
"example.org. 3600 IN RRSIG NSEC3PARAM 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- EXPECT_FALSE(updater_->getZoneData()->isNSEC3Signed());
+ EXPECT_FALSE(getZoneData()->isNSEC3Signed());
// And same for (RRSIG for) NSEC and "is signed".
updater_->add(ConstRRsetPtr(), textToRRset(
"example.org. 3600 IN RRSIG NSEC 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- EXPECT_FALSE(updater_->getZoneData()->isSigned());
+ EXPECT_FALSE(getZoneData()->isSigned());
}
// Commonly used checks for rrsigForNSEC3Only
@@ -275,13 +274,12 @@ TEST_P(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
textToRRset(
"example.org. 3600 IN RRSIG NSEC3PARAM 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- EXPECT_TRUE(updater_->getZoneData()->isNSEC3Signed());
+ EXPECT_TRUE(getZoneData()->isNSEC3Signed());
updater_->add(ConstRRsetPtr(),
textToRRset(
"09GM.example.org. 3600 IN RRSIG NSEC3 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- checkNSEC3Rdata(*mem_sgmt_, Name("09GM.example.org"),
- updater_->getZoneData());
+ checkNSEC3Rdata(*mem_sgmt_, Name("09GM.example.org"), getZoneData());
// Clear the current content of zone, then add NSEC3
clearZoneData();
@@ -294,8 +292,7 @@ TEST_P(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
textToRRset(
"09GM.example.org. 3600 IN RRSIG NSEC3 5 3 3600 "
"20150420235959 20051021000000 1 example.org. FAKE"));
- checkNSEC3Rdata(*mem_sgmt_, Name("09GM.example.org"),
- updater_->getZoneData());
+ checkNSEC3Rdata(*mem_sgmt_, Name("09GM.example.org"), getZoneData());
// If we add only RRSIG without any NSEC3 related data beforehand,
// it will be rejected; it's a limitation of the current implementation.
@@ -311,7 +308,8 @@ TEST_P(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
// Generate many small RRsets. This tests that the underlying memory segment
// can grow during the execution and that the updater handles that well.
//
-// Some of the grows will happen inserting the RRSIG, some with the TXT.
+// Some of the grows will happen inserting the RRSIG, some with the TXT. Or,
+// at least we hope so.
TEST_P(ZoneDataUpdaterTest, manySmallRRsets) {
for (size_t i = 0; i < 32768; ++i) {
const std::string name(boost::lexical_cast<std::string>(i) +
@@ -323,8 +321,7 @@ TEST_P(ZoneDataUpdaterTest, manySmallRRsets) {
"example.org. FAKE"));
ZoneNode* node = getNode(*mem_sgmt_,
Name(boost::lexical_cast<std::string>(i) +
- ".example.org"),
- updater_->getZoneData());
+ ".example.org"), getZoneData());
const RdataSet* rdset = node->getData();
ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
rdset = RdataSet::find(rdset, RRType::TXT(), true);
@@ -334,4 +331,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.");
diff --git a/src/lib/util/memory_segment.h b/src/lib/util/memory_segment.h
index e62c9df..66e7329 100644
--- a/src/lib/util/memory_segment.h
+++ b/src/lib/util/memory_segment.h
@@ -153,7 +153,7 @@ public:
/// \return Returns <code>true</code> if all allocated memory (including
/// names associated by memory addresses by \c setNamedAddress()) was
/// deallocated, <code>false</code> otherwise.
- virtual bool allMemoryDeallocated() const = 0;
+ virtual bool allMemoryDeallocated() = 0;
/// \brief Associate specified address in the segment with a given name.
///
diff --git a/src/lib/util/memory_segment_local.cc b/src/lib/util/memory_segment_local.cc
index 81548fd..4d35f38 100644
--- a/src/lib/util/memory_segment_local.cc
+++ b/src/lib/util/memory_segment_local.cc
@@ -47,7 +47,7 @@ MemorySegmentLocal::deallocate(void* ptr, size_t size) {
}
bool
-MemorySegmentLocal::allMemoryDeallocated() const {
+MemorySegmentLocal::allMemoryDeallocated() {
return (allocated_size_ == 0 && named_addrs_.empty());
}
diff --git a/src/lib/util/memory_segment_local.h b/src/lib/util/memory_segment_local.h
index 1db55a0..5007c22 100644
--- a/src/lib/util/memory_segment_local.h
+++ b/src/lib/util/memory_segment_local.h
@@ -64,7 +64,7 @@ public:
///
/// \return Returns <code>true</code> if all allocated memory was
/// deallocated, <code>false</code> otherwise.
- virtual bool allMemoryDeallocated() const;
+ virtual bool allMemoryDeallocated();
/// \brief Local segment version of getNamedAddress.
///
diff --git a/src/lib/util/memory_segment_mapped.cc b/src/lib/util/memory_segment_mapped.cc
index b74fbd4..a32d215 100644
--- a/src/lib/util/memory_segment_mapped.cc
+++ b/src/lib/util/memory_segment_mapped.cc
@@ -44,6 +44,15 @@ using boost::interprocess::offset_ptr;
namespace isc {
namespace util {
+
+namespace { // unnamed namespace
+
+const char* const RESERVED_NAMED_ADDRESS_STORAGE_NAME =
+ "_RESERVED_NAMED_ADDRESS_STORAGE";
+
+} // end of unnamed namespace
+
+
// Definition of class static constant so it can be referenced by address
// or reference.
const size_t MemorySegmentMapped::INITIAL_SIZE;
@@ -98,6 +107,7 @@ struct MemorySegmentMapped::Impl {
// confirm there's no other user and there won't either.
lock_.reset(new boost::interprocess::file_lock(filename.c_str()));
checkWriter();
+ reserveMemory();
}
// Constructor for open-or-write (and read-write) mode
@@ -108,6 +118,7 @@ struct MemorySegmentMapped::Impl {
lock_(new boost::interprocess::file_lock(filename.c_str()))
{
checkWriter();
+ reserveMemory();
}
// Constructor for existing segment, either read-only or read-write
@@ -123,6 +134,36 @@ struct MemorySegmentMapped::Impl {
} else {
checkWriter();
}
+ reserveMemory();
+ }
+
+ void reserveMemory() {
+ if (!read_only_) {
+ // Reserve a named address for use during
+ // setNamedAddress(). Though this will almost always succeed
+ // during construction, it may fail later during a call from
+ // allMemoryDeallocated() when the segment has been in use
+ // for a while.
+ while (true) {
+ const offset_ptr<void>* reserved_storage =
+ base_sgmt_->find_or_construct<offset_ptr<void> >(
+ RESERVED_NAMED_ADDRESS_STORAGE_NAME, std::nothrow)();
+
+ if (reserved_storage) {
+ break;
+ }
+
+ growSegment();
+ }
+ }
+ }
+
+ void freeReservedMemory() {
+ if (!read_only_) {
+ const bool deleted = base_sgmt_->destroy<offset_ptr<void> >
+ (RESERVED_NAMED_ADDRESS_STORAGE_NAME);
+ assert(deleted);
+ }
}
// Internal helper to grow the underlying mapped segment.
@@ -233,6 +274,7 @@ MemorySegmentMapped::MemorySegmentMapped(const std::string& filename,
MemorySegmentMapped::~MemorySegmentMapped() {
if (impl_->base_sgmt_ && !impl_->read_only_) {
+ impl_->freeReservedMemory();
impl_->base_sgmt_->flush(); // note: this is exception free
}
delete impl_;
@@ -281,8 +323,12 @@ MemorySegmentMapped::deallocate(void* ptr, size_t) {
}
bool
-MemorySegmentMapped::allMemoryDeallocated() const {
- return (impl_->base_sgmt_->all_memory_deallocated());
+MemorySegmentMapped::allMemoryDeallocated() {
+ impl_->freeReservedMemory();
+ const bool result = impl_->base_sgmt_->all_memory_deallocated();
+ impl_->reserveMemory();
+
+ return (result);
}
void*
@@ -305,13 +351,27 @@ MemorySegmentMapped::setNamedAddressImpl(const char* name, void* addr) {
isc_throw(MemorySegmentError, "address is out of segment: " << addr);
}
+ // Temporarily save the passed addr into pre-allocated offset_ptr in
+ // case there are any relocations caused by allocations.
+ offset_ptr<void>* reserved_storage =
+ impl_->base_sgmt_->find<offset_ptr<void> >(
+ RESERVED_NAMED_ADDRESS_STORAGE_NAME).first;
+ assert(reserved_storage);
+ *reserved_storage = addr;
+
bool grown = false;
while (true) {
offset_ptr<void>* storage =
impl_->base_sgmt_->find_or_construct<offset_ptr<void> >(
name, std::nothrow)();
if (storage) {
- *storage = addr;
+ // Move the address from saved offset_ptr into the
+ // newly-allocated storage.
+ reserved_storage =
+ impl_->base_sgmt_->find<offset_ptr<void> >(
+ RESERVED_NAMED_ADDRESS_STORAGE_NAME).first;
+ assert(reserved_storage);
+ *storage = *reserved_storage;
return (grown);
}
diff --git a/src/lib/util/memory_segment_mapped.h b/src/lib/util/memory_segment_mapped.h
index c6c79a1..3ff77d2 100644
--- a/src/lib/util/memory_segment_mapped.h
+++ b/src/lib/util/memory_segment_mapped.h
@@ -179,7 +179,7 @@ public:
/// read-only mode; in that case MemorySegmentError will be thrown.
virtual void deallocate(void* ptr, size_t size);
- virtual bool allMemoryDeallocated() const;
+ virtual bool allMemoryDeallocated();
/// \brief Mapped segment version of setNamedAddress.
///
diff --git a/src/lib/util/tests/memory_segment_mapped_unittest.cc b/src/lib/util/tests/memory_segment_mapped_unittest.cc
index bd37a73..11022fa 100644
--- a/src/lib/util/tests/memory_segment_mapped_unittest.cc
+++ b/src/lib/util/tests/memory_segment_mapped_unittest.cc
@@ -461,7 +461,14 @@ TEST_F(MemorySegmentMappedTest, shrink) {
EXPECT_EQ(shrinked_size, segment_->getSize());
// Check that the segment is still usable after shrink.
- void* p = segment_->allocate(sizeof(uint32_t));
+ void *p = NULL;
+ while (!p) {
+ try {
+ p = segment_->allocate(sizeof(uint32_t));
+ } catch (const MemorySegmentGrown&) {
+ // Do nothing. Just try again.
+ }
+ }
segment_->deallocate(p, sizeof(uint32_t));
}
More information about the bind10-changes
mailing list