BIND 10 master, updated. 780a3b3bd3492e500154e53e6835d82eb2eb3131 [master] Merge branch 'trac2973'

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Jun 18 11:00:33 UTC 2013


The branch, master has been updated
       via  780a3b3bd3492e500154e53e6835d82eb2eb3131 (commit)
       via  40d9424ef15d3c6d484a03772fa15d57781161f8 (commit)
       via  7ffc29a5c462f9359d6059e6b14b33653f6da0ad (commit)
       via  a169232953e34690749708a29b9f6c22e94b2dbf (commit)
       via  65b0c0b8a22a6f066ed6d9cae81f1de3cbca4682 (commit)
      from  5b41c700bd362d76ce19b032c75662f64b5a9ca8 (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 780a3b3bd3492e500154e53e6835d82eb2eb3131
Merge: 5b41c70 40d9424
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Tue Jun 18 19:15:02 2013 +0900

    [master] Merge branch 'trac2973'

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

Summary of changes:
 src/lib/datasrc/memory/zone_table.cc               |    9 +-
 src/lib/datasrc/memory/zone_table.h                |   25 ++++--
 src/lib/datasrc/memory/zone_writer.cc              |    6 +-
 src/lib/datasrc/tests/memory/testdata/Makefile.am  |    1 +
 .../datasrc/tests/memory/testdata/template.zone    |    4 +
 .../tests/memory/zone_data_loader_unittest.cc      |    2 +
 .../datasrc/tests/memory/zone_table_unittest.cc    |   40 ++++-----
 .../datasrc/tests/memory/zone_writer_unittest.cc   |   87 ++++++++++++++++++++
 8 files changed, 128 insertions(+), 46 deletions(-)
 create mode 100644 src/lib/datasrc/tests/memory/testdata/template.zone

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_table.cc b/src/lib/datasrc/memory/zone_table.cc
index 32938a6..454a0aa 100644
--- a/src/lib/datasrc/memory/zone_table.cc
+++ b/src/lib/datasrc/memory/zone_table.cc
@@ -83,7 +83,7 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable, int)
 }
 
 ZoneTable::AddResult
-ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
+ZoneTable::addZone(util::MemorySegment& mem_sgmt,
                    const Name& zone_name, ZoneData* content)
 {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_ZONE).
@@ -94,13 +94,8 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
                   (content ? "empty data" : "NULL") <<
                   " is passed to Zone::addZone");
     }
-    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, zone_class);
-    holder.set(content);
 
-    const AddResult result =
-        addZoneInternal(mem_sgmt, zone_name, holder.get());
-    holder.release();
-    return (result);
+    return (addZoneInternal(mem_sgmt, zone_name, content));
 }
 
 ZoneTable::AddResult
diff --git a/src/lib/datasrc/memory/zone_table.h b/src/lib/datasrc/memory/zone_table.h
index 81f1a3c..6bad516 100644
--- a/src/lib/datasrc/memory/zone_table.h
+++ b/src/lib/datasrc/memory/zone_table.h
@@ -165,12 +165,22 @@ public:
     ///
     /// This method adds a given zone data to the internal table.
     ///
-    /// This method ensures there'll be no memory leak on exception.
-    /// But addresses allocated from \c mem_sgmt could be relocated if
-    /// \c util::MemorySegmentGrown is thrown; the caller or its upper layer
-    /// must be aware of that possibility and update any such addresses
-    /// accordingly.  On successful return, this method ensures there's no
-    /// address relocation.
+    /// On successful completion (i.e., the method returns without an
+    /// exception), the ownership of \c content will be transferred to
+    /// the \c ZoneTable: the caller should not use the \c content hereafter;
+    /// the \c ZoneTable will be responsible to destroy it when the table
+    /// itself is destroyed.
+    ///
+    /// If this method throws, the caller is responsible to take care of
+    /// the passed \c content, whether to destroy it or use for different
+    /// purposes.  Note that addresses allocated from \c mem_sgmt could be
+    /// relocated if \c util::MemorySegmentGrown is thrown; the caller or its
+    /// upper layer must be aware of that possibility and update any such
+    /// addresses accordingly.  This applies to \c content, as it's expected
+    /// to be created using \c mem_sgmt.
+    ///
+    /// On successful return, this method ensures there's no address
+    /// relocation.
     ///
     /// \throw InvalidParameter content is NULL or empty.
     /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
@@ -181,8 +191,6 @@ public:
     ///     created.  It must be the same segment that was used to create
     ///     the zone table at the time of create().
     /// \param zone_name The name of the zone to be added.
-    /// \param zone_class The RR class of the zone.  It must be the RR class
-    ///     that is supposed to be associated to the zone table.
     /// \param content This one should hold the zone content (the ZoneData).
     ///     The ownership is passed onto the zone table. Must not be null or
     ///     empty. Must correspond to the name and class and must be allocated
@@ -194,7 +202,6 @@ public:
     ///     inside the result unless it's empty; if the zone was previously
     ///     added by \c addEmptyZone(), the data returned is NULL.
     AddResult addZone(util::MemorySegment& mem_sgmt,
-                      dns::RRClass zone_class,
                       const dns::Name& zone_name,
                       ZoneData* content);
 
diff --git a/src/lib/datasrc/memory/zone_writer.cc b/src/lib/datasrc/memory/zone_writer.cc
index 3d4469c..ea70bb9 100644
--- a/src/lib/datasrc/memory/zone_writer.cc
+++ b/src/lib/datasrc/memory/zone_writer.cc
@@ -134,9 +134,6 @@ ZoneWriter::install() {
     // zone data or we've allowed load error to create an empty zone.
     assert(impl_->data_holder_.get() || impl_->catch_load_error_);
 
-    // 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 (impl_->state_ != Impl::ZW_INSTALLED) {
         try {
             ZoneTableHeader& header = impl_->segment_.getHeader();
@@ -152,8 +149,7 @@ ZoneWriter::install() {
             const ZoneTable::AddResult result(
                 impl_->data_holder_->get() ?
                 table->addZone(impl_->segment_.getMemorySegment(),
-                               impl_->rrclass_, impl_->origin_,
-                               impl_->data_holder_->get()) :
+                               impl_->origin_, impl_->data_holder_->get()) :
                 table->addEmptyZone(impl_->segment_.getMemorySegment(),
                                     impl_->origin_));
             impl_->data_holder_->set(result.zone_data);
diff --git a/src/lib/datasrc/tests/memory/testdata/Makefile.am b/src/lib/datasrc/tests/memory/testdata/Makefile.am
index b076837..da27482 100644
--- a/src/lib/datasrc/tests/memory/testdata/Makefile.am
+++ b/src/lib/datasrc/tests/memory/testdata/Makefile.am
@@ -29,6 +29,7 @@ EXTRA_DIST += example.org-rrsigs.zone
 EXTRA_DIST += example.org-wildcard-dname.zone
 EXTRA_DIST += example.org-wildcard-ns.zone
 EXTRA_DIST += example.org-wildcard-nsec3.zone
+EXTRA_DIST += template.zone
 EXTRA_DIST += rrset-collection.zone
 
 EXTRA_DIST += 2503-test.zone
diff --git a/src/lib/datasrc/tests/memory/testdata/template.zone b/src/lib/datasrc/tests/memory/testdata/template.zone
new file mode 100644
index 0000000..d8ae6d8
--- /dev/null
+++ b/src/lib/datasrc/tests/memory/testdata/template.zone
@@ -0,0 +1,4 @@
+; a zone file that can be used with any origin.
+
+@ 3600 IN SOA . . 1 0 0 0 0
+@ 3600 IN NS ns1
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 ad20d43..0e5677c 100644
--- a/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc
@@ -12,6 +12,8 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <config.h>
+
 #include <datasrc/memory/zone_data_loader.h>
 #include <datasrc/memory/rdataset.h>
 #include <datasrc/memory/zone_data.h>
diff --git a/src/lib/datasrc/tests/memory/zone_table_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
index 328274f..7357322 100644
--- a/src/lib/datasrc/tests/memory/zone_table_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
@@ -74,7 +74,7 @@ TEST_F(ZoneTableTest, addZone) {
     EXPECT_EQ(0, zone_table->getZoneCount());
 
     // It doesn't accept NULL as zone data
-    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1, NULL),
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zname1, NULL),
                  isc::InvalidParameter);
     EXPECT_EQ(0, zone_table->getZoneCount()); // count is still 0
 
@@ -82,8 +82,7 @@ TEST_F(ZoneTableTest, addZone) {
     SegmentObjectHolder<ZoneData, RRClass> holder_empty(
         mem_sgmt_, zclass_);
     holder_empty.set(ZoneData::create(mem_sgmt_));
-    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1,
-                                     holder_empty.get()),
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zname1, holder_empty.get()),
                  isc::InvalidParameter);
 
     SegmentObjectHolder<ZoneData, RRClass> holder1(
@@ -91,8 +90,7 @@ TEST_F(ZoneTableTest, addZone) {
     holder1.set(ZoneData::create(mem_sgmt_, zname1));
     const ZoneData* data1(holder1.get());
     // Normal successful case.
-    const ZoneTable::AddResult result1(zone_table->addZone(mem_sgmt_, zclass_,
-                                                           zname1,
+    const ZoneTable::AddResult result1(zone_table->addZone(mem_sgmt_, zname1,
                                                            holder1.release()));
     EXPECT_EQ(result::SUCCESS, result1.code);
     EXPECT_EQ(static_cast<const ZoneData*>(NULL), result1.zone_data);
@@ -103,8 +101,7 @@ TEST_F(ZoneTableTest, addZone) {
     // Duplicate add replaces the existing data wit the newly added one.
     SegmentObjectHolder<ZoneData, RRClass> holder2(mem_sgmt_, zclass_);
     holder2.set(ZoneData::create(mem_sgmt_, zname1));
-    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
-                                                           zname1,
+    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zname1,
                                                            holder2.release()));
     EXPECT_EQ(result::EXIST, result2.code);
     // The old one gets out
@@ -119,7 +116,7 @@ TEST_F(ZoneTableTest, addZone) {
         mem_sgmt_, zclass_);
     holder3.set(ZoneData::create(mem_sgmt_, Name("EXAMPLE.COM")));
     // names are compared in a case insensitive manner.
-    const ZoneTable::AddResult result3(zone_table->addZone(mem_sgmt_, zclass_,
+    const ZoneTable::AddResult result3(zone_table->addZone(mem_sgmt_,
                                                            Name("EXAMPLE.COM"),
                                                            holder3.release()));
     EXPECT_EQ(result::EXIST, result3.code);
@@ -129,26 +126,23 @@ TEST_F(ZoneTableTest, addZone) {
         mem_sgmt_, zclass_);
     holder4.set(ZoneData::create(mem_sgmt_, zname2));
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2,
-                                  holder4.release()).code);
+              zone_table->addZone(mem_sgmt_, zname2, holder4.release()).code);
     EXPECT_EQ(2, zone_table->getZoneCount());
     SegmentObjectHolder<ZoneData, RRClass> holder5(
         mem_sgmt_, zclass_);
     holder5.set(ZoneData::create(mem_sgmt_, zname3));
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3,
-                                  holder5.release()).code);
+              zone_table->addZone(mem_sgmt_, zname3, holder5.release()).code);
     EXPECT_EQ(3, zone_table->getZoneCount());
 
     // Have the memory segment throw an exception in extending the internal
-    // tree.  It still shouldn't cause memory leak (which would be detected
-    // in TearDown()).
+    // tree.  We'll destroy it after that via SegmentObjectHolder.
     SegmentObjectHolder<ZoneData, RRClass> holder6(
         mem_sgmt_, zclass_);
     holder6.set(ZoneData::create(mem_sgmt_, Name("example.org")));
     mem_sgmt_.setThrowCount(1);
-    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org"),
-                                     holder6.release()),
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, Name("example.org"),
+                                     holder6.get()),
                  std::bad_alloc);
 }
 
@@ -175,8 +169,7 @@ TEST_F(ZoneTableTest, addEmptyZone) {
     // internal to the ZoneTable implementation.
     SegmentObjectHolder<ZoneData, RRClass> holder2(mem_sgmt_, zclass_);
     holder2.set(ZoneData::create(mem_sgmt_, zname1));
-    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
-                                                           zname1,
+    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zname1,
                                                            holder2.release()));
     EXPECT_EQ(result::EXIST, result2.code);
     EXPECT_EQ(static_cast<const ZoneData*>(NULL), result2.zone_data);
@@ -197,20 +190,18 @@ TEST_F(ZoneTableTest, findZone) {
         mem_sgmt_, zclass_);
     holder1.set(ZoneData::create(mem_sgmt_, zname1));
     ZoneData* zone_data = holder1.get();
-    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_, zname1,
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zname1,
                                                    holder1.release()).code);
     SegmentObjectHolder<ZoneData, RRClass> holder2(
         mem_sgmt_, zclass_);
     holder2.set(ZoneData::create(mem_sgmt_, zname2));
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2,
-                                  holder2.release()).code);
+              zone_table->addZone(mem_sgmt_, zname2, holder2.release()).code);
     SegmentObjectHolder<ZoneData, RRClass> holder3(
         mem_sgmt_, zclass_);
     holder3.set(ZoneData::create(mem_sgmt_, zname3));
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3,
-                                  holder3.release()).code);
+              zone_table->addZone(mem_sgmt_, zname3, holder3.release()).code);
 
     const ZoneTable::FindResult find_result1 =
         zone_table->findZone(Name("example.com"));
@@ -234,8 +225,7 @@ TEST_F(ZoneTableTest, findZone) {
     SegmentObjectHolder<ZoneData, RRClass> holder4(
         mem_sgmt_, zclass_);
     holder4.set(ZoneData::create(mem_sgmt_, Name("com")));
-    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
-                                                   Name("com"),
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, Name("com"),
                                                    holder4.release()).code);
     EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
diff --git a/src/lib/datasrc/tests/memory/zone_writer_unittest.cc b/src/lib/datasrc/tests/memory/zone_writer_unittest.cc
index 97a396f..e1fe672 100644
--- a/src/lib/datasrc/tests/memory/zone_writer_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_writer_unittest.cc
@@ -12,10 +12,20 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <config.h>
+
 #include <datasrc/memory/zone_writer.h>
 #include <datasrc/memory/zone_table_segment_local.h>
 #include <datasrc/memory/zone_data.h>
+#include <datasrc/memory/zone_data_loader.h>
+#include <datasrc/memory/load_action.h>
+#include <datasrc/memory/zone_table.h>
 #include <datasrc/exceptions.h>
+#include <datasrc/result.h>
+
+#include <util/memory_segment_mapped.h>
+
+#include <cc/data.h>
 
 #include <dns/rrclass.h>
 #include <dns/name.h>
@@ -27,8 +37,10 @@
 
 #include <boost/scoped_ptr.hpp>
 #include <boost/bind.hpp>
+#include <boost/format.hpp>
 
 #include <string>
+#include <unistd.h>
 
 using boost::scoped_ptr;
 using boost::bind;
@@ -325,4 +337,79 @@ TEST_F(ZoneWriterTest, autoCleanUp) {
     EXPECT_NO_THROW(writer_->load());
 }
 
+// Used in the manyWrites test, encapsulating loadZoneData() to avoid
+// its signature ambiguity.
+ZoneData*
+loadZoneDataWrapper(isc::util::MemorySegment& segment, const RRClass& rrclass,
+                    const Name& name, const std::string& filename)
+{
+    return (loadZoneData(segment, rrclass, name, filename));
+}
+
+// Check the behavior of creating many small zones.  The main purpose of
+// test is to trigger MemorySegmentGrown exception in ZoneWriter::install.
+// There's no easy (if any) way to cause that reliably as it's highly
+// dependent on details of the underlying boost implementation and probably
+// also on the system behavior, but we'll try some promising scenario (it
+// in fact triggered the intended result at least on one environment).
+TEST_F(ZoneWriterTest, manyWrites) {
+#ifdef USE_SHARED_MEMORY
+    // First, make a fresh mapped file of a small size (so it'll be more likely
+    // to grow in the test.
+    const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
+    unlink(mapped_file);
+    boost::scoped_ptr<isc::util::MemorySegmentMapped> segment(
+        new isc::util::MemorySegmentMapped(
+            mapped_file, isc::util::MemorySegmentMapped::CREATE_ONLY, 4096));
+    segment.reset();
+
+    // Then prepare a ZoneTableSegment of the 'mapped' type specifying the
+    // file we just created.
+    boost::scoped_ptr<ZoneTableSegment> zt_segment(
+        ZoneTableSegment::create(RRClass::IN(), "mapped"));
+    const isc::data::ConstElementPtr params =
+        isc::data::Element::fromJSON(
+            "{\"mapped-file\": \"" + std::string(mapped_file) + "\"}");
+    zt_segment->reset(ZoneTableSegment::READ_WRITE, params);
+#else
+    // Do the same test for the local segment, although there shouldn't be
+    // anything tricky in that case.
+    boost::scoped_ptr<ZoneTableSegment> zt_segment(
+        ZoneTableSegment::create(RRClass::IN(), "local"));
+#endif
+
+    // Now, create many small zones in the zone table with a ZoneWriter.
+    // We use larger origin names so it'll (hopefully) require the memory
+    // segment to grow while adding the name into the internal table.
+    const size_t zone_count = 10000; // arbitrary choice
+    for (size_t i = 0; i < zone_count; ++i) {
+        const Name origin(
+            boost::str(boost::format("%063u.%063u.%063u.example.org")
+                       % i % i % i));
+        const LoadAction action = boost::bind(loadZoneDataWrapper, _1,
+                                              RRClass::IN(), origin,
+                                              TEST_DATA_DIR
+                                              "/template.zone");
+        ZoneWriter writer(*zt_segment, action, origin, RRClass::IN(), false);
+        writer.load();
+        writer.install();
+        writer.cleanup();
+
+        // Confirm it's been successfully added and can be actually found.
+        const ZoneTable::FindResult result =
+            zt_segment->getHeader().getTable()->findZone(origin);
+        EXPECT_EQ(isc::datasrc::result::SUCCESS, result.code);
+        EXPECT_NE(static_cast<const ZoneData*>(NULL), result.zone_data) <<
+            "unexpected find result: " + origin.toText();
+    }
+
+    // Make sure to close the segment before (possibly) removing the mapped
+    // file.
+    zt_segment.reset();
+
+#ifdef USE_SHARED_MEMORY
+    unlink(mapped_file);
+#endif
+}
+
 }



More information about the bind10-changes mailing list