[2836] Merge branch 'trac2836' of ssh://git.bind10.isc.org/var/bind10/git/bind10 into trac2836

ommit.

src/lib/datasrc/memory/rdataset.h

@@ -176,6 +176,15 @@ public:
     /// it cannot contain more than 65535 RRSIGs.  If the given RRset(s) fail
     /// to meet this condition, an \c RdataSetError exception will be thrown.
     ///
+    /// 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.
+    ///
+    /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
+    ///     relocating data.
     /// \throw isc::BadValue Given RRset(s) are invalid (see the description)
     /// \throw RdataSetError Number of RDATAs exceed the limits
     /// \throw std::bad_alloc Memory allocation fails.

src/lib/datasrc/memory/segment_object_holder.h

@@ -37,6 +37,19 @@ getNextHolderName();
 // A simple holder to create and use some objects in this implementation
 // in an exception safe manner.   It works like std::auto_ptr but much
 // more simplified.
+//
+// Note, however, that it doesn't take the pointer to hold on construction.
+// This is because the constructor itself can throw or cause address
+// reallocation inside the memory segment.  If that happens various
+// undesirable effects can happen, such as memory leak or unintentional access
+// to the pre-reallocated address.  To make it safer, we use a separate
+// \c set() method, which is exception free and doesn't cause address
+// reallocation.  So the typical usage is to first construct the holder
+// object, then the object to be held, immediately followed by a call to \c
+// set(). Subsequent access to the held address should be done via the \c get()
+// method.  get() ensures the address is always valid in the memory segment
+// even if address reallocation happens between set() and get().
+//
 // template parameter T is the type of object allocated by mem_sgmt.
 // template parameter ARG_T is the type that will be passed to destroy()
 // (deleter functor, etc).  It must be copyable.

src/lib/datasrc/memory/zone_data.h

@@ -86,6 +86,15 @@ public:
     /// The NSEC3 parameters are extracted and stored within the created
     /// \c NSEC3Data object.
     ///
+    /// 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.
+    ///
+    /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
+    ///     relocating data.
     /// \throw std::bad_alloc Memory allocation fails.
     ///
     /// \param mem_sgmt A \c MemorySegment from which memory for the new
@@ -102,6 +111,15 @@ public:
     /// The NSEC3 hash parameters are extracted and stored within the created
     /// \c NSEC3Data object.
     ///
+    /// 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.
+    ///
+    /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
+    ///     relocating data.
     /// \throw std::bad_alloc Memory allocation fails.
     ///
     /// \param mem_sgmt A \c MemorySegment from which memory for the new
@@ -375,6 +393,15 @@ public:
 public:
     /// \brief Allocate and construct \c ZoneData.
     ///
+    /// 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.
+    ///
+    /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
+    ///     relocating data.
     /// \throw std::bad_alloc Memory allocation fails.
     ///
     /// \param mem_sgmt A \c MemorySegment from which memory for the new

src/lib/datasrc/memory/zone_data_updater.cc

@@ -234,12 +234,6 @@ ZoneDataUpdater::setupNSEC3(const ConstRRsetPtr rrset) {
     NSEC3Data* nsec3_data = zone_data_->getNSEC3Data();
     if (nsec3_data == NULL) {
         nsec3_data = NSEC3Data::create(mem_sgmt_, zone_name_, nsec3_rdata);
-        // The create above might have relocated data. So get it again,
-        // just to make sure.
-        zone_data_ =
-            static_cast<ZoneData*>(mem_sgmt_.
-                                   getNamedAddress("updater_zone_data").
-                                   second);
         zone_data_->setNSEC3Data(nsec3_data);
         zone_data_->setSigned(true);
     } else {

src/lib/datasrc/memory/zone_data_updater.h

@@ -80,7 +80,13 @@ public:
             isc_throw(isc::InvalidOperation, "A ZoneDataUpdater already exists"
                       " on this memory segment. Destroy it first.");
         }
-        mem_sgmt_.setNamedAddress("updater_zone_data", zone_data_);
+        if (mem_sgmt_.setNamedAddress("updater_zone_data", zone_data_)) {
+            // It might have relocated during the set
+            zone_data_ =
+                static_cast<ZoneData*>(mem_sgmt_.getNamedAddress("updater_zone_data").
+                                       second);
+        }
+        assert(zone_data_);
     }
 
     /// The destructor.

src/lib/datasrc/memory/zone_table.h

@@ -115,6 +115,15 @@ public:
     /// from the given memory segment, constructs the object, and returns
     /// a pointer to it.
     ///
+    /// 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.
+    ///
+    /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
+    ///     relocating data.
     /// \throw std::bad_alloc Memory allocation fails.
     ///
     /// \param mem_sgmt A \c MemorySegment from which memory for the new
@@ -149,9 +158,16 @@ 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.
+    ///
+    /// \throw util::MemorySegmentGrown The memory segment has grown, possibly
+    ///     relocating data.
     /// \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

src/lib/datasrc/memory/zone_writer.cc

@@ -54,14 +54,13 @@ ZoneWriter::load() {
     }
 
     zone_data_ = load_action_(segment_.getMemorySegment());
+    segment_.resetHeader();
 
     if (!zone_data_) {
         // Bug inside load_action_.
         isc_throw(isc::InvalidOperation, "No data returned from load action");
     }
 
-    segment_.resetHeader();
-
     state_ = ZW_LOADED;
 }
 
@@ -86,11 +85,10 @@ ZoneWriter::install() {
                                                   zone_data_));
             state_ = ZW_INSTALLED;
             zone_data_ = result.zone_data;
-        } catch (const isc::util::MemorySegmentGrown&) {
-        }
-    }
+        } catch (const isc::util::MemorySegmentGrown&) {}
 
-    segment_.resetHeader();
+        segment_.resetHeader();
+    }
 }
 
 void