BIND 10 trac2850_4, updated. 33f75a53137bb954ed660f41043c6887c94e398f [2850] ZoneWriter updates

BIND 10 source code commits bind10-changes at lists.isc.org
Fri May 24 13:52:14 UTC 2013


The branch, trac2850_4 has been updated
       via  33f75a53137bb954ed660f41043c6887c94e398f (commit)
       via  e4a817403b39c77a0b3a7c015e1f77f241c21c3a (commit)
      from  85fcde63692e73b4ac783d2300851b94578575c9 (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 33f75a53137bb954ed660f41043c6887c94e398f
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Fri May 24 15:30:03 2013 +0530

    [2850] ZoneWriter updates

commit e4a817403b39c77a0b3a7c015e1f77f241c21c3a
Author: Mukund Sivaraman <muks at isc.org>
Date:   Fri May 24 15:27:30 2013 +0530

    [2850] Add a comment

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

Summary of changes:
 src/lib/datasrc/memory/zone_writer.cc |   95 +++++++++++++++++++++++----------
 src/lib/datasrc/memory/zone_writer.h  |   25 ++++-----
 src/lib/util/memory_segment_mapped.cc |    2 +
 3 files changed, 78 insertions(+), 44 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_writer.cc b/src/lib/datasrc/memory/zone_writer.cc
index 867901f..d76fe3f 100644
--- a/src/lib/datasrc/memory/zone_writer.cc
+++ b/src/lib/datasrc/memory/zone_writer.cc
@@ -15,6 +15,9 @@
 #include <datasrc/memory/zone_writer.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/memory/segment_object_holder.h>
+
+#include <dns/rrclass.h>
 
 #include <memory>
 
@@ -24,65 +27,100 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
+ZoneTableSegment&
+checkZoneTableSegment(ZoneTableSegment& segment) {
+    if (!segment.isWritable()) {
+        isc_throw(isc::InvalidOperation,
+                  "Attempt to construct ZoneWriter for a read-only segment");
+    }
+    return (segment);
+}
+
+struct ZoneWriter::Impl {
+    Impl(ZoneTableSegment& segment, const LoadAction& load_action,
+         const dns::Name& origin, const dns::RRClass& rrclass) :
+        // We validate segment first so we can use it to initialize
+        // data_holder_ safely.
+        segment_(checkZoneTableSegment(segment)),
+        load_action_(load_action),
+        origin_(origin),
+        rrclass_(rrclass),
+        state_(ZW_UNUSED),
+        data_holder_(segment.getMemorySegment(), rrclass_)
+    {}
+
+    ZoneTableSegment& segment_;
+    const LoadAction load_action_;
+    const dns::Name origin_;
+    const dns::RRClass rrclass_;
+    enum State {
+        ZW_UNUSED,
+        ZW_LOADED,
+        ZW_INSTALLED,
+        ZW_CLEANED
+    };
+    State state_;
+    detail::SegmentObjectHolder<ZoneData, dns::RRClass> data_holder_;
+};
+
 ZoneWriter::ZoneWriter(ZoneTableSegment& segment,
                        const LoadAction& load_action,
                        const dns::Name& origin,
                        const dns::RRClass& rrclass) :
-    segment_(segment),
-    load_action_(load_action),
-    origin_(origin),
-    rrclass_(rrclass),
-    zone_data_(NULL),
-    state_(ZW_UNUSED)
+    impl_(new Impl(segment, load_action, origin, rrclass))
 {
-    if (!segment.isWritable()) {
-        isc_throw(isc::InvalidOperation,
-                  "Attempt to construct ZoneWriter for a read-only segment");
-    }
 }
 
 ZoneWriter::~ZoneWriter() {
     // Clean up everything there might be left if someone forgot, just
     // in case.
     cleanup();
+    delete impl_;
 }
 
 void
 ZoneWriter::load() {
-    if (state_ != ZW_UNUSED) {
+    if (impl_->state_ != Impl::ZW_UNUSED) {
         isc_throw(isc::InvalidOperation, "Trying to load twice");
     }
 
-    zone_data_ = load_action_(segment_.getMemorySegment());
-    if (!zone_data_) {
+    ZoneData* zone_data =
+        impl_->load_action_(impl_->segment_.getMemorySegment());
+    if (!zone_data) {
         // Bug inside load_action_.
         isc_throw(isc::InvalidOperation, "No data returned from load action");
     }
+    impl_->data_holder_.set(zone_data);
 
-    state_ = ZW_LOADED;
+    impl_->state_ = Impl::ZW_LOADED;
 }
 
 void
 ZoneWriter::install() {
-    if (state_ != ZW_LOADED) {
+    if (impl_->state_ != Impl::ZW_LOADED) {
         isc_throw(isc::InvalidOperation, "No data to install");
     }
 
     // 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) {
+    while (impl_->state_ != Impl::ZW_INSTALLED) {
         try {
-            ZoneTable* table(segment_.getHeader().getTable());
-            if (table == NULL) {
+            ZoneTable* table(impl_->segment_.getHeader().getTable());
+            if (!table) {
                 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;
+            // We still need to hold the zone data until we return from
+            // addZone in case it throws, but we then need to immediately
+            // release it as the ownership is transferred to the zone table.
+            // we release this by (re)set it to the old data; that way we can
+            // use the holder for the final cleanup.
+            const ZoneTable::AddResult result(
+                table->addZone(impl_->segment_.getMemorySegment(),
+                               impl_->rrclass_, impl_->origin_,
+                               impl_->data_holder_.get()));
+            impl_->data_holder_.set(result.zone_data);
+            impl_->state_ = Impl::ZW_INSTALLED;
         } catch (const isc::util::MemorySegmentGrown&) {}
     }
 }
@@ -91,10 +129,11 @@ void
 ZoneWriter::cleanup() {
     // We eat the data (if any) now.
 
-    if (zone_data_ != NULL) {
-        ZoneData::destroy(segment_.getMemorySegment(), zone_data_, rrclass_);
-        zone_data_ = NULL;
-        state_ = ZW_CLEANED;
+    ZoneData* zone_data = impl_->data_holder_.release();
+    if (zone_data) {
+        ZoneData::destroy(impl_->segment_.getMemorySegment(), zone_data,
+                          impl_->rrclass_);
+        impl_->state_ = Impl::ZW_CLEANED;
     }
 }
 
diff --git a/src/lib/datasrc/memory/zone_writer.h b/src/lib/datasrc/memory/zone_writer.h
index 1c4e944..226c3ce 100644
--- a/src/lib/datasrc/memory/zone_writer.h
+++ b/src/lib/datasrc/memory/zone_writer.h
@@ -15,15 +15,16 @@
 #ifndef MEM_ZONE_WRITER_H
 #define MEM_ZONE_WRITER_H
 
-#include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/load_action.h>
 
-#include <dns/rrclass.h>
-#include <dns/name.h>
+#include <boost/noncopyable.hpp>
+
+#include <dns/dns_fwd.h>
 
 namespace isc {
 namespace datasrc {
 namespace memory {
+class ZoneTableSegment;
 
 /// \brief Does an update to a zone.
 ///
@@ -38,7 +39,7 @@ namespace memory {
 /// This class provides strong exception guarantee for each public
 /// method. That is, when any of the methods throws, the entire state
 /// stays the same as before the call.
-class ZoneWriter {
+class ZoneWriter : boost::noncopyable {
 public:
     /// \brief Constructor
     ///
@@ -100,18 +101,10 @@ public:
     void cleanup();
 
 private:
-    ZoneTableSegment& segment_;
-    const LoadAction load_action_;
-    const dns::Name origin_;
-    const dns::RRClass rrclass_;
-    ZoneData* zone_data_;
-    enum State {
-        ZW_UNUSED,
-        ZW_LOADED,
-        ZW_INSTALLED,
-        ZW_CLEANED
-    };
-    State state_;
+    // We hide details as this class will be used by various applications
+    // and we use some internal data structures in the implementation.
+    struct Impl;
+    Impl* impl_;
 };
 
 }
diff --git a/src/lib/util/memory_segment_mapped.cc b/src/lib/util/memory_segment_mapped.cc
index 55ba395..f36ad71 100644
--- a/src/lib/util/memory_segment_mapped.cc
+++ b/src/lib/util/memory_segment_mapped.cc
@@ -331,6 +331,8 @@ MemorySegmentMapped::allMemoryDeallocated() const {
     try {
         impl_->freeReservedMemory();
         const bool result = impl_->base_sgmt_->all_memory_deallocated();
+        // reserveMemory() should succeed now as the memory was already
+        // allocated.
         impl_->reserveMemory();
         return (result);
     } catch (...) {



More information about the bind10-changes mailing list