Browse Source

Merge #2836

Make the zone loading work even on top of MemorySegmentMapped. In
particular, handle the MemorySegmentGrown exception, which may cause
relocation of data.
Michal 'vorner' Vaner 12 years ago
parent
commit
e41be59e11

+ 2 - 0
src/bin/auth/tests/datasrc_clients_builder_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 <util/unittests/check_valgrind.h>
 
 #include <dns/name.h>

+ 1 - 0
src/lib/datasrc/memory/Makefile.am

@@ -17,6 +17,7 @@ libdatasrc_memory_la_SOURCES += rdata_serialization.h rdata_serialization.cc
 libdatasrc_memory_la_SOURCES += zone_data.h zone_data.cc
 libdatasrc_memory_la_SOURCES += rrset_collection.h rrset_collection.cc
 libdatasrc_memory_la_SOURCES += segment_object_holder.h
+libdatasrc_memory_la_SOURCES += segment_object_holder.cc
 libdatasrc_memory_la_SOURCES += logger.h logger.cc
 libdatasrc_memory_la_SOURCES += zone_table.h zone_table.cc
 libdatasrc_memory_la_SOURCES += zone_finder.h zone_finder.cc

+ 16 - 7
src/lib/datasrc/memory/domaintree.h

@@ -1301,15 +1301,24 @@ public:
     /// doesn't exist.
     ///
     /// This method normally involves resource allocation.  If it fails
-    /// the corresponding standard exception will be thrown.
+    /// \c std::bad_alloc will be thrown.  Also, depending on details of the
+    /// specific \c MemorySegment, it can propagate the \c MemorySegmentGrown
+    /// exception.
     ///
     /// This method does not provide the strong exception guarantee in its
-    /// strict sense; if an exception is thrown in the middle of this
-    /// method, the internal structure may change.  However, it should
-    /// still retain the same property as a mapping container before this
-    /// method is called.  For example, the result of \c find() should be
-    /// the same.  This method provides the weak exception guarantee in its
-    /// normal sense.
+    /// strict sense; there can be new empty nodes that are superdomains of
+    /// the domain to be inserted as a side effect.  However, the tree
+    /// retains internal integrity otherwise, and, in particular, the intended
+    /// insert operation is "resumable": if the \c insert() method is called
+    /// again with the same argument after resolving the cause of the
+    /// exception (possibly multiple times), it should now succeed.  Note,
+    /// however, that in case of \c MemorySegmentGrown the address of the
+    /// `DomainTree` object may have been reallocated if it was created with
+    /// the same \c MemorySegment (which will often be the case in practice).
+    /// So the caller may have to re-get the address before calling \c insert
+    /// again.  It can be done using the concept of "named addresses" of
+    /// \c MemorySegment, or the direct caller may not have to worry about it
+    /// if this condition is guaranteed at a higher level.
     ///
     /// \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

+ 9 - 0
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.

+ 40 - 0
src/lib/datasrc/memory/segment_object_holder.cc

@@ -0,0 +1,40 @@
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include "segment_object_holder.h"
+
+#include <boost/lexical_cast.hpp>
+
+#include <cassert>
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+namespace detail {
+
+std::string
+getNextHolderName() {
+    static uint64_t index = 0;
+    ++index;
+    // in practice we should be able to assume this, uint64 is large
+    // and should not overflow
+    assert(index != 0);
+    return ("Segment object holder auto name " +
+            boost::lexical_cast<std::string>(index));
+}
+
+}
+}
+}
+}

+ 71 - 11
src/lib/datasrc/memory/segment_object_holder.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013  Internet Systems Consortium, Inc. ("ISC")
 //
 // Permission to use, copy, modify, and/or distribute this software for any
 // purpose with or without fee is hereby granted, provided that the above
@@ -16,39 +16,99 @@
 #define DATASRC_MEMORY_SEGMENT_OBJECT_HOLDER_H 1
 
 #include <util/memory_segment.h>
+#include <string>
+#include <cassert>
 
 namespace isc {
 namespace datasrc {
 namespace memory {
 namespace detail {
 
+// Internal function to get next yet unused name of segment holder.
+// We need the names of holders to be unique per segment at any given
+// momemnt. This just keeps incrementing number after a prefix with
+// each call, it should be enough (we assert it does not wrap around,
+// but 64bits should be enough).
+//
+// Also, it is not thread safe.
+std::string
+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.
 template <typename T, typename ARG_T>
 class SegmentObjectHolder {
 public:
-    SegmentObjectHolder(util::MemorySegment& mem_sgmt, T* obj, ARG_T arg) :
-        mem_sgmt_(mem_sgmt), obj_(obj), arg_(arg)
-    {}
+    SegmentObjectHolder(util::MemorySegment& mem_sgmt, ARG_T arg) :
+        mem_sgmt_(mem_sgmt), arg_(arg),
+        holder_name_(getNextHolderName()), holding_(true)
+    {
+        if (mem_sgmt_.setNamedAddress(holder_name_.c_str(), NULL)) {
+            // OK. We've grown. The caller might need to be informed, so
+            // we throw. But then, we don't get our destructor, so we
+            // release the memory right away.
+            mem_sgmt_.clearNamedAddress(holder_name_.c_str());
+            isc_throw(isc::util::MemorySegmentGrown,
+                      "Segment grown when allocating holder");
+        }
+    }
     ~SegmentObjectHolder() {
-        if (obj_ != NULL) {
-            T::destroy(mem_sgmt_, obj_, arg_);
+        if (holding_) {
+            // Use release, as it removes the stored address from segment
+            T* obj = release();
+            if (obj) { // May be NULL if set wasn't called
+                T::destroy(mem_sgmt_, obj, arg_);
+            }
+        }
+    }
+    void set(T* obj) {
+        const bool grown = mem_sgmt_.setNamedAddress(holder_name_.c_str(),
+                                                     obj);
+        // We reserve the space in the constructor, should not grow now
+        assert(!grown);
+    }
+    T* get() {
+        if (holding_) {
+            const util::MemorySegment::NamedAddressResult result =
+                mem_sgmt_.getNamedAddress(holder_name_.c_str());
+            assert(result.first);
+            return (static_cast<T*>(result.second));
+        } else {
+            return (NULL);
         }
     }
-    T* get() { return (obj_); }
     T* release() {
-        T* ret = obj_;
-        obj_ = NULL;
-        return (ret);
+        if (holding_) {
+            T* obj = get();
+            mem_sgmt_.clearNamedAddress(holder_name_.c_str());
+            holding_ = false;
+            return (obj);
+        } else {
+            return (NULL);
+        }
     }
 private:
     util::MemorySegment& mem_sgmt_;
-    T* obj_;
     ARG_T arg_;
+    const std::string holder_name_;
+    bool holding_;
 };
 
 } // detail

+ 4 - 4
src/lib/datasrc/memory/zone_data.cc

@@ -91,8 +91,8 @@ NSEC3Data::create(util::MemorySegment& mem_sgmt,
     // (with an assertion check for that).
     typedef boost::function<void(RdataSet*)> RdataSetDeleterType;
     detail::SegmentObjectHolder<ZoneTree, RdataSetDeleterType> holder(
-        mem_sgmt, ZoneTree::create(mem_sgmt, true),
-        boost::bind(nullDeleter, _1));
+        mem_sgmt, boost::bind(nullDeleter, _1));
+    holder.set(ZoneTree::create(mem_sgmt, true));
 
     ZoneTree* tree = holder.get();
     const ZoneTree::Result result =
@@ -165,8 +165,8 @@ ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) {
     // NSEC3Data::create().
     typedef boost::function<void(RdataSet*)> RdataSetDeleterType;
     detail::SegmentObjectHolder<ZoneTree, RdataSetDeleterType> holder(
-        mem_sgmt, ZoneTree::create(mem_sgmt, true),
-        boost::bind(nullDeleter, _1));
+        mem_sgmt, boost::bind(nullDeleter, _1));
+    holder.set(ZoneTree::create(mem_sgmt, true));
 
     ZoneTree* tree = holder.get();
     ZoneNode* origin_node = NULL;

+ 27 - 0
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

+ 41 - 29
src/lib/datasrc/memory/zone_data_loader.cc

@@ -131,7 +131,7 @@ ZoneDataLoader::flushNodeRRsets() {
     }
 
     // Normally rrsigsets map should be empty at this point, but it's still
-    // possible that an RRSIG that don't has covered RRset is added; they
+    // possible that an RRSIG that doesn't have covered RRset is added; they
     // still remain in the map.  We add them to the zone separately.
     BOOST_FOREACH(NodeRRsetsVal val, node_rrsigsets_) {
         updater_.add(ConstRRsetPtr(), val.second);
@@ -182,35 +182,47 @@ 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);
-
-    ZoneDataLoader loader(mem_sgmt, rrclass, zone_name, *holder.get());
-    rrset_installer(boost::bind(&ZoneDataLoader::addFromLoad, &loader, _1));
-    // Add any last RRsets that were left
-    loader.flushNodeRRsets();
-
-    const ZoneNode* origin_node = holder.get()->getOriginNode();
-    const RdataSet* rdataset = origin_node->getData();
-    // If the zone is NSEC3-signed, check if it has NSEC3PARAM
-    if (holder.get()->isNSEC3Signed()) {
-        if (RdataSet::find(rdataset, RRType::NSEC3PARAM()) == NULL) {
-            LOG_WARN(logger, DATASRC_MEMORY_MEM_NO_NSEC3PARAM).
-                arg(zone_name).arg(rrclass);
+    while (true) { // Try as long as it takes to load and grow the segment
+        bool created = false;
+        try {
+            SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, rrclass);
+            holder.set(ZoneData::create(mem_sgmt, zone_name));
+
+            // Nothing from this point on should throw MemorySegmentGrown.
+            // It is handled inside here.
+            created = true;
+
+            ZoneDataLoader loader(mem_sgmt, rrclass, zone_name, *holder.get());
+            rrset_installer(boost::bind(&ZoneDataLoader::addFromLoad, &loader,
+                                        _1));
+            // Add any last RRsets that were left
+            loader.flushNodeRRsets();
+
+            const ZoneNode* origin_node = holder.get()->getOriginNode();
+            const RdataSet* rdataset = origin_node->getData();
+            // If the zone is NSEC3-signed, check if it has NSEC3PARAM
+            if (holder.get()->isNSEC3Signed()) {
+                if (RdataSet::find(rdataset, RRType::NSEC3PARAM()) == NULL) {
+                    LOG_WARN(logger, DATASRC_MEMORY_MEM_NO_NSEC3PARAM).
+                        arg(zone_name).arg(rrclass);
+                }
+            }
+
+            RRsetCollection collection(*(holder.get()), rrclass);
+            const dns::ZoneCheckerCallbacks
+                callbacks(boost::bind(&logError, &zone_name, &rrclass, _1),
+                          boost::bind(&logWarning, &zone_name, &rrclass, _1));
+            if (!dns::checkZone(zone_name, rrclass, collection, callbacks)) {
+                isc_throw(ZoneValidationError,
+                          "Errors found when validating zone: "
+                          << zone_name << "/" << rrclass);
+            }
+
+            return (holder.release());
+        } catch (const util::MemorySegmentGrown&) {
+            assert(!created);
         }
     }
-
-    RRsetCollection collection(*(holder.get()), rrclass);
-    const dns::ZoneCheckerCallbacks
-        callbacks(boost::bind(&logError, &zone_name, &rrclass, _1),
-                  boost::bind(&logWarning, &zone_name, &rrclass, _1));
-    if (!dns::checkZone(zone_name, rrclass, collection, callbacks)) {
-        isc_throw(ZoneValidationError,
-                  "Errors found when validating zone: "
-                  << zone_name << "/" << rrclass);
-    }
-
-    return (holder.release());
 }
 
 // A wrapper for dns::MasterLoader used by loadZoneData() below.  Essentially
@@ -256,7 +268,7 @@ loadZoneData(util::MemorySegment& mem_sgmt,
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_LOAD_FROM_FILE).
         arg(zone_name).arg(rrclass).arg(zone_file);
 
-     return (loadZoneDataInternal(mem_sgmt, rrclass, zone_name,
+    return (loadZoneDataInternal(mem_sgmt, rrclass, zone_name,
                                  boost::bind(masterLoaderWrapper,
                                              zone_file.c_str(),
                                              zone_name, rrclass,

+ 1 - 1
src/lib/datasrc/memory/zone_data_loader.h

@@ -57,7 +57,7 @@ ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
 /// \c iterator.
 ///
 /// Throws \c ZoneDataUpdater::AddError if invalid or inconsistent data
-/// is present in the \c zone_file. Throws \c isc::Unexpected if empty
+/// is present in the \c iterator. Throws \c isc::Unexpected if empty
 /// RRsets are passed by the zone iterator. Throws \c EmptyZone if an
 /// empty zone would be created due to the \c loadZoneData().
 ///

+ 49 - 25
src/lib/datasrc/memory/zone_data_updater.cc

@@ -49,14 +49,14 @@ ZoneDataUpdater::addWildcards(const Name& name) {
             // Ensure a separate level exists for the "wildcarding"
             // name, and mark the node as "wild".
             ZoneNode* node;
-            zone_data_.insertName(mem_sgmt_, wname.split(1), &node);
+            zone_data_->insertName(mem_sgmt_, wname.split(1), &node);
             node->setFlag(ZoneData::WILDCARD_NODE);
 
             // Ensure a separate level exists for the wildcard name.
             // Note: for 'name' itself we do this later anyway, but the
             // overhead should be marginal because wildcard names should
             // be rare.
-            zone_data_.insertName(mem_sgmt_, wname, &node);
+            zone_data_->insertName(mem_sgmt_, wname, &node);
         }
     }
 }
@@ -210,7 +210,7 @@ ZoneDataUpdater::validate(const isc::dns::ConstRRsetPtr rrset) const {
 const NSEC3Hash*
 ZoneDataUpdater::getNSEC3Hash() {
     if (hash_ == NULL) {
-        NSEC3Data* nsec3_data = zone_data_.getNSEC3Data();
+        NSEC3Data* nsec3_data = zone_data_->getNSEC3Data();
         // This should never be NULL in this codepath.
         assert(nsec3_data != NULL);
 
@@ -231,11 +231,11 @@ ZoneDataUpdater::setupNSEC3(const ConstRRsetPtr rrset) {
         dynamic_cast<const T&>(
             rrset->getRdataIterator()->getCurrent());
 
-    NSEC3Data* nsec3_data = zone_data_.getNSEC3Data();
+    NSEC3Data* nsec3_data = zone_data_->getNSEC3Data();
     if (nsec3_data == NULL) {
         nsec3_data = NSEC3Data::create(mem_sgmt_, zone_name_, nsec3_rdata);
-        zone_data_.setNSEC3Data(nsec3_data);
-        zone_data_.setSigned(true);
+        zone_data_->setNSEC3Data(nsec3_data);
+        zone_data_->setSigned(true);
     } else {
         const NSEC3Hash* hash = getNSEC3Hash();
         if (!hash->match(nsec3_rdata)) {
@@ -247,14 +247,14 @@ ZoneDataUpdater::setupNSEC3(const ConstRRsetPtr rrset) {
 }
 
 void
-ZoneDataUpdater::addNSEC3(const Name& name, const ConstRRsetPtr rrset,
-                          const ConstRRsetPtr rrsig)
+ZoneDataUpdater::addNSEC3(const Name& name, const ConstRRsetPtr& rrset,
+                          const ConstRRsetPtr& rrsig)
 {
     if (rrset) {
         setupNSEC3<generic::NSEC3>(rrset);
     }
 
-    NSEC3Data* nsec3_data = zone_data_.getNSEC3Data();
+    NSEC3Data* nsec3_data = zone_data_->getNSEC3Data();
     if (nsec3_data == NULL) {
         // This is some tricky case: an RRSIG for NSEC3 is given without the
         // covered NSEC3, and we don't even know any NSEC3 related data.
@@ -283,14 +283,14 @@ ZoneDataUpdater::addNSEC3(const Name& name, const ConstRRsetPtr rrset,
 
 void
 ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
-                             const ConstRRsetPtr rrset,
-                             const ConstRRsetPtr rrsig)
+                             const ConstRRsetPtr& rrset,
+                             const ConstRRsetPtr& rrsig)
 {
     if (rrtype == RRType::NSEC3()) {
         addNSEC3(name, rrset, rrsig);
     } else {
         ZoneNode* node;
-        zone_data_.insertName(mem_sgmt_, name, &node);
+        zone_data_->insertName(mem_sgmt_, name, &node);
 
         RdataSet* rdataset_head = node->getData();
 
@@ -334,7 +334,7 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
         // Ok, we just put it in.
 
         // Convenient (and more efficient) shortcut to check RRsets at origin
-        const bool is_origin = (node == zone_data_.getOriginNode());
+        const bool is_origin = (node == zone_data_->getOriginNode());
 
         // If this RRset creates a zone cut at this node, mark the node
         // indicating the need for callback in find().  Note that we do this
@@ -356,7 +356,7 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
             // (conceptually "signed" is a broader notion but our
             // current zone finder implementation regards "signed" as
             // "NSEC signed")
-            zone_data_.setSigned(true);
+            zone_data_->setSigned(true);
         }
 
         // If we are adding a new SOA at the origin, update zone's min TTL.
@@ -365,7 +365,7 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
         // this should be only once in normal cases) update the TTL.
         if (rrset && rrtype == RRType::SOA() && is_origin) {
             // Our own validation ensures the RRset is not empty.
-            zone_data_.setMinTTL(
+            zone_data_->setMinTTL(
                 dynamic_cast<const generic::SOA&>(
                     rrset->getRdataIterator()->getCurrent()).getMinimum());
         }
@@ -373,6 +373,24 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype,
 }
 
 void
+ZoneDataUpdater::addInternal(const isc::dns::Name& name,
+                     const isc::dns::RRType& rrtype,
+                     const isc::dns::ConstRRsetPtr& rrset,
+                     const isc::dns::ConstRRsetPtr& rrsig)
+{
+    // Add wildcards possibly contained in the owner name to the domain
+    // tree.  This can only happen for the normal (non-NSEC3) tree.
+    // Note: this can throw an exception, breaking strong exception
+    // guarantee.  (see also the note for the call to contextCheck()
+    // above).
+    if (rrtype != RRType::NSEC3()) {
+        addWildcards(name);
+    }
+
+    addRdataSet(name, rrtype, rrset, rrsig);
+}
+
+void
 ZoneDataUpdater::add(const ConstRRsetPtr& rrset,
                      const ConstRRsetPtr& sig_rrset)
 {
@@ -397,16 +415,22 @@ ZoneDataUpdater::add(const ConstRRsetPtr& rrset,
         arg(rrset ? rrtype.toText() : "RRSIG(" + rrtype.toText() + ")").
         arg(zone_name_);
 
-    // Add wildcards possibly contained in the owner name to the domain
-    // tree.  This can only happen for the normal (non-NSEC3) tree.
-    // Note: this can throw an exception, breaking strong exception
-    // guarantee.  (see also the note for the call to contextCheck()
-    // above).
-    if (rrtype != RRType::NSEC3()) {
-        addWildcards(name);
-    }
-
-    addRdataSet(name, rrtype, rrset, sig_rrset);
+    // Store the address, it may change during growth and the address inside
+    // would get updated.
+    bool added = false;
+    do {
+        try {
+            addInternal(name, rrtype, rrset, sig_rrset);
+            added = true;
+        } catch (const isc::util::MemorySegmentGrown&) {
+            // The segment has grown. So, we update the base pointer (because
+            // the data may have been remapped somewhere else in the process).
+            zone_data_ =
+                static_cast<ZoneData*>(
+                    mem_sgmt_.getNamedAddress("updater_zone_data").second);
+        }
+        // Retry if it didn't add due to the growth
+    } while (!added);
 }
 
 } // namespace memory

+ 31 - 12
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,
@@ -72,12 +73,25 @@ public:
        mem_sgmt_(mem_sgmt),
        rrclass_(rrclass),
        zone_name_(zone_name),
-       zone_data_(zone_data),
-       hash_(NULL)
-    {}
+       hash_(NULL),
+       zone_data_(&zone_data)
+    {
+        if (mem_sgmt_.getNamedAddress("updater_zone_data").first) {
+            isc_throw(isc::InvalidOperation, "A ZoneDataUpdater already exists"
+                      " on this memory segment. Destroy it first.");
+        }
+        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.
     ~ZoneDataUpdater() {
+        mem_sgmt_.clearNamedAddress("updater_zone_data");
         delete hash_;
     }
 
@@ -159,6 +173,11 @@ private:
     // contained in 'name' (e.g., '*.foo.example' in 'bar.*.foo.example').
     void addWildcards(const isc::dns::Name& name);
 
+    void addInternal(const isc::dns::Name& name,
+                     const isc::dns::RRType& rrtype,
+                     const isc::dns::ConstRRsetPtr& rrset,
+                     const isc::dns::ConstRRsetPtr& rrsig);
+
     // Does some checks in context of the data that are already in the
     // zone.  Currently checks for forbidden combinations of RRsets in
     // the same domain (CNAME+anything, DNAME+NS).  If such condition is
@@ -175,19 +194,19 @@ private:
     template <typename T>
     void setupNSEC3(const isc::dns::ConstRRsetPtr rrset);
     void addNSEC3(const isc::dns::Name& name,
-                  const isc::dns::ConstRRsetPtr rrset,
-                  const isc::dns::ConstRRsetPtr rrsig);
+                  const isc::dns::ConstRRsetPtr& rrset,
+                  const isc::dns::ConstRRsetPtr& rrsig);
     void addRdataSet(const isc::dns::Name& name,
                      const isc::dns::RRType& rrtype,
-                     const isc::dns::ConstRRsetPtr rrset,
-                     const isc::dns::ConstRRsetPtr rrsig);
+                     const isc::dns::ConstRRsetPtr& rrset,
+                     const isc::dns::ConstRRsetPtr& rrsig);
 
     util::MemorySegment& mem_sgmt_;
     const isc::dns::RRClass rrclass_;
     const isc::dns::Name& zone_name_;
-    ZoneData& zone_data_;
     RdataEncoder encoder_;
     const isc::dns::NSEC3Hash* hash_;
+    ZoneData* zone_data_;
 };
 
 } // namespace memory

+ 4 - 4
src/lib/datasrc/memory/zone_table.cc

@@ -50,8 +50,8 @@ typedef boost::function<void(ZoneData*)> ZoneDataDeleterType;
 ZoneTable*
 ZoneTable::create(util::MemorySegment& mem_sgmt, const RRClass& zone_class) {
     SegmentObjectHolder<ZoneTableTree, ZoneDataDeleterType> holder(
-        mem_sgmt, ZoneTableTree::create(mem_sgmt),
-        boost::bind(deleteZoneData, &mem_sgmt, _1, zone_class));
+        mem_sgmt, boost::bind(deleteZoneData, &mem_sgmt, _1, zone_class));
+    holder.set(ZoneTableTree::create(mem_sgmt));
     void* p = mem_sgmt.allocate(sizeof(ZoneTable));
     ZoneTable* zone_table = new(p) ZoneTable(zone_class, holder.get());
     holder.release();
@@ -77,8 +77,8 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     if (content == NULL) {
         isc_throw(isc::BadValue, "Zone content must not be NULL");
     }
-    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, content,
-                                                  zone_class);
+    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, zone_class);
+    holder.set(content);
     // Get the node where we put the zone
     ZoneTableNode* node(NULL);
     switch (zones_->insert(mem_sgmt, zone_name, &node)) {

+ 18 - 0
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,6 +158,15 @@ 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.
     ///
     /// \param mem_sgmt The \c MemorySegment to allocate zone data to be

+ 19 - 13
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;
 }
 
@@ -71,18 +70,25 @@ ZoneWriter::install() {
         isc_throw(isc::InvalidOperation, "No data to install");
     }
 
-    ZoneTable* table(segment_.getHeader().getTable());
-    if (!table) {
-        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&) {}
+
+        segment_.resetHeader();
     }
-    const ZoneTable::AddResult result(table->addZone(
-                                          segment_.getMemorySegment(),
-                                          rrclass_, origin_, zone_data_));
-
-    state_ = ZW_INSTALLED;
-    zone_data_ = result.zone_data;
-
-    segment_.resetHeader();
 }
 
 void

+ 2 - 0
src/lib/datasrc/tests/client_list_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/client_list.h>
 #include <datasrc/client.h>
 #include <datasrc/cache_config.h>

+ 29 - 45
src/lib/datasrc/tests/memory/rdataset_unittest.cc

@@ -191,20 +191,16 @@ TEST_F(RdataSetTest, mergeCreate) {
             SCOPED_TRACE("creating merge case " + lexical_cast<string>(i) +
                          ", " + lexical_cast<string>(j));
             // Create old rdataset
-            SegmentObjectHolder<RdataSet, RRClass> holder1(
-                mem_sgmt_,
-                RdataSet::create(mem_sgmt_, encoder_,
+            SegmentObjectHolder<RdataSet, RRClass> holder1(mem_sgmt_, rrclass);
+            holder1.set(RdataSet::create(mem_sgmt_, encoder_,
                                  (i & 1) != 0 ? a_rrsets[0] : null_rrset,
-                                 (i & 2) != 0 ? rrsig_rrsets[0] : null_rrset),
-                rrclass);
+                                 (i & 2) != 0 ? rrsig_rrsets[0] : null_rrset));
             // Create merged rdataset, based on the old one and RRsets
-            SegmentObjectHolder<RdataSet, RRClass> holder2(
-                mem_sgmt_,
-                RdataSet::create(mem_sgmt_, encoder_,
+            SegmentObjectHolder<RdataSet, RRClass> holder2(mem_sgmt_, rrclass);
+            holder2.set(RdataSet::create(mem_sgmt_, encoder_,
                                  (j & 1) != 0 ? a_rrsets[1] : null_rrset,
                                  (j & 2) != 0 ? rrsig_rrsets[1] : null_rrset,
-                                 holder1.get()),
-                rrclass);
+                                 holder1.get()));
 
             // Set up the expected data for the case.
             vector<string> expected_rdata;
@@ -241,16 +237,14 @@ TEST_F(RdataSetTest, duplicate) {
 
     // After suppressing duplicates, it should be the same as the default
     // RdataSet.  Check that.
-    SegmentObjectHolder<RdataSet, RRClass> holder1(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, dup_rrset, dup_rrsig), rrclass);
+    SegmentObjectHolder<RdataSet, RRClass> holder1(mem_sgmt_, rrclass);
+    holder1.set(RdataSet::create(mem_sgmt_, encoder_, dup_rrset, dup_rrsig));
     checkRdataSet(*holder1.get(), def_rdata_txt_, def_rrsig_txt_);
 
     // Confirm the same thing for the merge mode.
-    SegmentObjectHolder<RdataSet, RRClass> holder2(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, a_rrset_, rrsig_rrset_,
-                         holder1.get()), rrclass);
+    SegmentObjectHolder<RdataSet, RRClass> holder2(mem_sgmt_, rrclass);
+    holder2.set(RdataSet::create(mem_sgmt_, encoder_, a_rrset_, rrsig_rrset_,
+                                 holder1.get()));
     checkRdataSet(*holder2.get(), def_rdata_txt_, def_rrsig_txt_);
 }
 
@@ -275,24 +269,21 @@ TEST_F(RdataSetTest, getNext) {
 
 TEST_F(RdataSetTest, find) {
     // Create some RdataSets and make a chain of them.
-    SegmentObjectHolder<RdataSet, RRClass> holder1(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, a_rrset_, ConstRRsetPtr()),
-        RRClass::IN());
+    SegmentObjectHolder<RdataSet, RRClass> holder1(mem_sgmt_, RRClass::IN());
+    holder1.set(RdataSet::create(mem_sgmt_, encoder_, a_rrset_,
+                                 ConstRRsetPtr()));
     ConstRRsetPtr aaaa_rrset =
         textToRRset("www.example.com. 1076895760 IN AAAA 2001:db8::1");
-    SegmentObjectHolder<RdataSet, RRClass> holder2(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, aaaa_rrset, ConstRRsetPtr()),
-        RRClass::IN());
+    SegmentObjectHolder<RdataSet, RRClass> holder2(mem_sgmt_, RRClass::IN());
+    holder2.set(RdataSet::create(mem_sgmt_, encoder_, aaaa_rrset,
+                                 ConstRRsetPtr()));
     ConstRRsetPtr sigonly_rrset =
         textToRRset("www.example.com. 1076895760 IN RRSIG "
                     "TXT 5 2 3600 20120814220826 20120715220826 "
                     "1234 example.com. FAKE");
-    SegmentObjectHolder<RdataSet, RRClass> holder3(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(), sigonly_rrset),
-        RRClass::IN());
+    SegmentObjectHolder<RdataSet, RRClass> holder3(mem_sgmt_, RRClass::IN());
+    holder3.set(RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(),
+                                 sigonly_rrset));
 
     RdataSet* rdataset_a = holder1.get();
     RdataSet* rdataset_aaaa = holder2.get();
@@ -376,10 +367,8 @@ TEST_F(RdataSetTest, createManyRRs) {
 
 TEST_F(RdataSetTest, mergeCreateManyRRs) {
     ConstRRsetPtr rrset = textToRRset("example.com. 3600 IN TXT some-text");
-    SegmentObjectHolder<RdataSet, RRClass> holder(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, rrset, ConstRRsetPtr()),
-        RRClass::IN());
+    SegmentObjectHolder<RdataSet, RRClass> holder(mem_sgmt_, RRClass::IN());
+    holder.set(RdataSet::create(mem_sgmt_, encoder_, rrset, ConstRRsetPtr()));
 
     checkCreateManyRRs(boost::bind(&RdataSet::create, _1, _2, _3, _4,
                                    holder.get()), rrset->getRdataCount());
@@ -474,10 +463,8 @@ TEST_F(RdataSetTest, mergeCreateManyRRSIGs) {
     ConstRRsetPtr rrsig = textToRRset(
         "example.com. 3600 IN RRSIG A 5 2 3600 20120814220826 20120715220826 "
         "1234 example.com. FAKEFAKE");
-    SegmentObjectHolder<RdataSet, RRClass> holder(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(), rrsig),
-        rrclass);
+    SegmentObjectHolder<RdataSet, RRClass> holder(mem_sgmt_, rrclass);
+    holder.set(RdataSet::create(mem_sgmt_, encoder_, ConstRRsetPtr(), rrsig));
 
     checkCreateManyRRSIGs(boost::bind(&RdataSet::create, _1, _2, _3, _4,
                                       holder.get()), rrsig->getRdataCount());
@@ -543,12 +530,10 @@ TEST_F(RdataSetTest, badCreate) {
 TEST_F(RdataSetTest, badMergeCreate) {
     // The 'old RdataSet' for merge.  Its content doesn't matter much; the test
     // should trigger exception before examining it except for the last checks.
-    SegmentObjectHolder<RdataSet, RRClass> holder(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_,
+    SegmentObjectHolder<RdataSet, RRClass> holder(mem_sgmt_, RRClass::IN());
+    holder.set(RdataSet::create(mem_sgmt_, encoder_,
                          textToRRset("www.example.com. 0 IN AAAA 2001:db8::1"),
-                         ConstRRsetPtr()),
-        RRClass::IN());
+                         ConstRRsetPtr()));
 
     checkBadCreate(boost::bind(&RdataSet::create, _1, _2, _3, _4,
                                holder.get()));
@@ -585,9 +570,8 @@ TEST_F(RdataSetTest, varyingTTL) {
     RdataSet::destroy(mem_sgmt_, rdataset, rrclass);
 
     // RRSIG's TTL is smaller
-    SegmentObjectHolder<RdataSet, RRClass> holder1(
-        mem_sgmt_,
-        RdataSet::create(mem_sgmt_, encoder_, aaaa_large, sig_small), rrclass);
+    SegmentObjectHolder<RdataSet, RRClass> holder1(mem_sgmt_, rrclass);
+    holder1.set(RdataSet::create(mem_sgmt_, encoder_, aaaa_large, sig_small));
     EXPECT_EQ(RRTTL(10), restoreTTL(holder1.get()->getTTLData()));
 
     // Merging another RRset (w/o sig) that has larger TTL

+ 11 - 7
src/lib/datasrc/tests/memory/rrset_collection_unittest.cc

@@ -24,6 +24,8 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/scoped_ptr.hpp>
+
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using namespace std;
@@ -43,22 +45,24 @@ public:
         rrclass("IN"),
         origin("example.org"),
         zone_file(TEST_DATA_DIR "/rrset-collection.zone"),
-        zone_data_holder(mem_sgmt,
-                         loadZoneData(mem_sgmt, rrclass, origin, zone_file),
-                         rrclass),
-        collection(*zone_data_holder.get(), rrclass)
-    {}
+        zone_data_holder(mem_sgmt, rrclass)
+    {
+        zone_data_holder.set(loadZoneData(mem_sgmt, rrclass, origin,
+                                          zone_file));
+        collection.reset(new RRsetCollection(*zone_data_holder.get(),
+                                             rrclass));
+    }
 
     const RRClass rrclass;
     const Name origin;
     std::string zone_file;
     test::MemorySegmentMock mem_sgmt;
     SegmentObjectHolder<ZoneData, RRClass> zone_data_holder;
-    RRsetCollection collection;
+    boost::scoped_ptr<RRsetCollection> collection;
 };
 
 TEST_F(RRsetCollectionTest, find) {
-    const RRsetCollection& ccln = collection;
+    const RRsetCollection& ccln = *collection;
     ConstRRsetPtr rrset = ccln.find(Name("www.example.org"), rrclass,
                                     RRType::A());
     EXPECT_TRUE(rrset);

+ 77 - 1
src/lib/datasrc/tests/memory/segment_object_holder_unittest.cc

@@ -12,10 +12,17 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <config.h>
+
 #include <util/memory_segment_local.h>
+#include <util/memory_segment_mapped.h>
 
 #include <datasrc/memory/segment_object_holder.h>
 
+#ifdef USE_SHARED_MEMORY
+#include <boost/interprocess/managed_mapped_file.hpp>
+#endif
+
 #include <gtest/gtest.h>
 
 using namespace isc::util;
@@ -24,12 +31,14 @@ using namespace isc::datasrc::memory::detail;
 
 namespace {
 const int TEST_ARG_VAL = 42;    // arbitrary chosen magic number
+const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
 
 class TestObject {
 public:
     static void destroy(MemorySegment& sgmt, TestObject* obj, int arg) {
         sgmt.deallocate(obj, sizeof(*obj));
         EXPECT_EQ(TEST_ARG_VAL, arg);
+        EXPECT_TRUE(obj);
     }
 };
 
@@ -42,7 +51,8 @@ useHolder(MemorySegment& sgmt, TestObject* obj, bool release) {
     // deallocate().
 
     typedef SegmentObjectHolder<TestObject, int> HolderType;
-    HolderType holder(sgmt, obj, TEST_ARG_VAL);
+    HolderType holder(sgmt, TEST_ARG_VAL);
+    holder.set(obj);
     EXPECT_EQ(obj, holder.get());
     if (release) {
         EXPECT_EQ(obj, holder.release());
@@ -64,4 +74,70 @@ TEST(SegmentObjectHolderTest, foo) {
     useHolder(sgmt, obj, false);
     EXPECT_TRUE(sgmt.allMemoryDeallocated());
 }
+
+// Test nothing bad happens if the holder is not set before it is destroyed
+TEST(SegmentObjectHolderTest, destroyNotSet) {
+    MemorySegmentLocal sgmt;
+    {
+        typedef SegmentObjectHolder<TestObject, int> HolderType;
+        HolderType holder(sgmt, TEST_ARG_VAL);
+    }
+    EXPECT_TRUE(sgmt.allMemoryDeallocated());
+}
+
+#ifdef USE_SHARED_MEMORY
+// Keep allocating bigger and bigger chunks of data until the allocation
+// fails with growing the segment.
+void
+allocateUntilGrows(MemorySegment& segment, size_t& current_size) {
+    // Create an object that will not be explicitly deallocated.
+    // It must be deallocated by the segment holder and even in case
+    // the position moved.
+    void *object_memory = segment.allocate(sizeof(TestObject));
+    TestObject* object = new(object_memory) TestObject;
+    SegmentObjectHolder<TestObject, int> holder(segment, TEST_ARG_VAL);
+    holder.set(object);
+    while (true) {
+        void* data = segment.allocate(current_size);
+        segment.deallocate(data, current_size);
+        current_size *= 2;
+    }
+}
+
+// Check that the segment thing releases stuff even in case it throws
+// SegmentGrown exception and the thing moves address
+TEST(SegmentObjectHolderTest, grow) {
+    MemorySegmentMapped segment(mapped_file,
+                                isc::util::MemorySegmentMapped::CREATE_ONLY);
+    // Allocate a bit of memory, to get a unique address
+    void* mark = segment.allocate(1);
+    segment.setNamedAddress("mark", mark);
+
+    // We'd like to cause 'mark' will be mapped at a different address on
+    // MemorySegmentGrown; there doesn't seem to be a reliable and safe way
+    // to cause this situation, but opening another mapped region seems to
+    // often work in practice.  We use Boost managed_mapped_file directly
+    // to ignore the imposed file lock with MemorySegmentMapped.
+    using boost::interprocess::managed_mapped_file;
+    using boost::interprocess::open_only;
+    managed_mapped_file mapped_sgmt(open_only, mapped_file);
+
+    // Try allocating bigger and bigger chunks of data until the segment
+    // actually relocates
+    size_t alloc_size = 1024;
+    EXPECT_THROW(allocateUntilGrows(segment, alloc_size), MemorySegmentGrown);
+    // Confirm it's now mapped at a different address.
+    EXPECT_NE(mark, segment.getNamedAddress("mark").second)
+        << "portability assumption for the test doesn't hold; "
+        "disable the test by setting env variable GTEST_FILTER to "
+        "'-SegmentObjectHolderTest.grow'";
+    mark = segment.getNamedAddress("mark").second;
+    segment.clearNamedAddress("mark");
+    segment.deallocate(mark, 1);
+    EXPECT_TRUE(segment.allMemoryDeallocated());
+    // Remove the file
+    EXPECT_EQ(0, unlink(mapped_file));
+}
+#endif
+
 }

+ 36 - 0
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 <datasrc/tests/memory/memory_segment_mock.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, RRClass::IN())));
+        zones.back()->set(data);
+
+    }
+    // Deallocate all the zones now.
+    zones.clear();
+    EXPECT_TRUE(segment.allMemoryDeallocated());
+    EXPECT_EQ(0, unlink(mapped_file));
+}
+
 }

+ 166 - 39
src/lib/datasrc/tests/memory/zone_data_updater_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_updater.h>
 #include <datasrc/memory/rdataset.h>
 #include <datasrc/memory/zone_data.h>
@@ -25,11 +27,14 @@
 #include <dns/rrset.h>
 #include <dns/rrttl.h>
 
+#include <util/memory_segment_local.h>
+#include <util/memory_segment_mapped.h>
 #include <datasrc/tests/memory/memory_segment_mock.h>
 
 #include <gtest/gtest.h>
 
 #include <boost/scoped_ptr.hpp>
+#include <boost/lexical_cast.hpp>
 
 #include <cassert>
 
@@ -39,71 +44,158 @@ using namespace isc::datasrc::memory;
 
 namespace {
 
-class ZoneDataUpdaterTest : public ::testing::Test {
+const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
+
+// An abstract factory class for the segments. We want fresh segment for each
+// 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;
+    // Clean-up after the test. Most of them will be just NOP (the default),
+    // but the file-mapped one needs to remove the file.
+    virtual void cleanup() const {}
+};
+
+ZoneNode*
+getNode(isc::util::MemorySegment& mem_sgmt, const Name& name,
+        ZoneData* zone_data)
+{
+    ZoneNode* node = NULL;
+    zone_data->insertName(mem_sgmt, name, &node);
+    EXPECT_NE(static_cast<ZoneNode*>(NULL), node);
+    return (node);
+}
+
+class ZoneDataUpdaterTest : public ::testing::TestWithParam<SegmentCreator*> {
 protected:
     ZoneDataUpdaterTest() :
         zname_("example.org"), zclass_(RRClass::IN()),
-        zone_data_(ZoneData::create(mem_sgmt_, zname_)),
-        updater_(new ZoneDataUpdater(mem_sgmt_, zclass_, zname_, *zone_data_))
-    {}
+        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 (zone_data_ != NULL) {
-            ZoneData::destroy(mem_sgmt_, zone_data_, zclass_);
-        }
-        if (!mem_sgmt_.allMemoryDeallocated()) {
+        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";
         }
+        GetParam()->cleanup();
     }
 
     void clearZoneData() {
-        assert(zone_data_ != NULL);
-        ZoneData::destroy(mem_sgmt_, zone_data_, zclass_);
-        zone_data_ = ZoneData::create(mem_sgmt_, zname_);
-        updater_.reset(new ZoneDataUpdater(mem_sgmt_, zclass_, zname_,
-                                           *zone_data_));
+        assert(updater_);
+        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").second));
     }
 
     const Name zname_;
     const RRClass zclass_;
-    test::MemorySegmentMock mem_sgmt_;
-    ZoneData* zone_data_;
+    boost::shared_ptr<isc::util::MemorySegment> mem_sgmt_;
     boost::scoped_ptr<ZoneDataUpdater> updater_;
 };
 
-TEST_F(ZoneDataUpdaterTest, bothNull) {
+class TestSegmentCreator : public SegmentCreator {
+public:
+    virtual SegmentPtr create() const {
+        return (SegmentPtr(new test::MemorySegmentMock));
+    }
+};
+
+TestSegmentCreator test_segment_creator;
+
+INSTANTIATE_TEST_CASE_P(TestSegment, ZoneDataUpdaterTest,
+                        ::testing::Values(static_cast<SegmentCreator*>(
+                            &test_segment_creator)));
+
+class MemorySegmentCreator : public SegmentCreator {
+public:
+    virtual SegmentPtr create() const {
+        // We are not really supposed to create the segment directly in real
+        // code, but it should be OK inside tests.
+        return (SegmentPtr(new isc::util::MemorySegmentLocal));
+    }
+};
+
+MemorySegmentCreator memory_segment_creator;
+
+INSTANTIATE_TEST_CASE_P(LocalSegment, ZoneDataUpdaterTest,
+                        ::testing::Values(static_cast<SegmentCreator*>(
+                            &memory_segment_creator)));
+
+class MappedSegmentCreator : public SegmentCreator {
+public:
+    MappedSegmentCreator(size_t initial_size =
+                         isc::util::MemorySegmentMapped::INITIAL_SIZE) :
+        initial_size_(initial_size)
+    {}
+    virtual SegmentPtr create() const {
+        return (SegmentPtr(new isc::util::MemorySegmentMapped(
+                               mapped_file,
+                               isc::util::MemorySegmentMapped::CREATE_ONLY,
+                               initial_size_)));
+    }
+    virtual void cleanup() const {
+        EXPECT_EQ(0, unlink(mapped_file));
+    }
+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;
+
+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.
     EXPECT_THROW(updater_->add(ConstRRsetPtr(), ConstRRsetPtr()),
                  ZoneDataUpdater::NullRRset);
 }
 
-ZoneNode*
-getNode(isc::util::MemorySegment& mem_sgmt, const Name& name,
-        ZoneData* zone_data)
-{
-    ZoneNode* node = NULL;
-    zone_data->insertName(mem_sgmt, name, &node);
-    EXPECT_NE(static_cast<ZoneNode*>(NULL), node);
-    return (node);
-}
-
-TEST_F(ZoneDataUpdaterTest, zoneMinTTL) {
+TEST_P(ZoneDataUpdaterTest, zoneMinTTL) {
     // If we add SOA, zone's min TTL will be updated.
     updater_->add(textToRRset(
                       "example.org. 3600 IN SOA . . 0 0 0 0 1200",
                       zclass_, zname_),
                   ConstRRsetPtr());
-    isc::util::InputBuffer b(zone_data_->getMinTTLData(), sizeof(uint32_t));
+    isc::util::InputBuffer b(getZoneData()->getMinTTLData(), sizeof(uint32_t));
     EXPECT_EQ(RRTTL(1200), RRTTL(b));
 }
 
-TEST_F(ZoneDataUpdaterTest, rrsigOnly) {
+TEST_P(ZoneDataUpdaterTest, rrsigOnly) {
     // RRSIG that doesn't have covered RRset can be added.  The resulting
     // rdataset won't have "normal" RDATA but sig RDATA.
     updater_->add(ConstRRsetPtr(), textToRRset(
                       "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"), zone_data_);
+    ZoneNode* node = getNode(*mem_sgmt_, Name("www.example.org"),
+                             getZoneData());
     const RdataSet* rdset = node->getData();
     ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
     rdset = RdataSet::find(rdset, RRType::A(), true);
@@ -121,7 +213,7 @@ TEST_F(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"), zone_data_);
+    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
@@ -129,14 +221,14 @@ TEST_F(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"), zone_data_);
+    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"), zone_data_);
+    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
@@ -144,13 +236,13 @@ TEST_F(ZoneDataUpdaterTest, rrsigOnly) {
     updater_->add(ConstRRsetPtr(), textToRRset(
                       "example.org. 3600 IN RRSIG NSEC3PARAM 5 3 3600 "
                       "20150420235959 20051021000000 1 example.org. FAKE"));
-    EXPECT_FALSE(zone_data_->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(zone_data_->isSigned());
+    EXPECT_FALSE(getZoneData()->isSigned());
 }
 
 // Commonly used checks for rrsigForNSEC3Only
@@ -168,7 +260,7 @@ checkNSEC3Rdata(isc::util::MemorySegment& mem_sgmt, const Name& name,
     EXPECT_EQ(1, rdset->getSigRdataCount());
 }
 
-TEST_F(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
+TEST_P(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
     // Adding only RRSIG covering NSEC3 is tricky.  It should go to the
     // separate NSEC3 tree, but the separate space is only created when
     // NSEC3 or NSEC3PARAM is added.  So, in many cases RRSIG-only is allowed,
@@ -183,12 +275,12 @@ TEST_F(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
                   textToRRset(
                       "example.org. 3600 IN RRSIG NSEC3PARAM 5 3 3600 "
                       "20150420235959 20051021000000 1 example.org. FAKE"));
-    EXPECT_TRUE(zone_data_->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"), zone_data_);
+    checkNSEC3Rdata(*mem_sgmt_, Name("09GM.example.org"), getZoneData());
 
     // Clear the current content of zone, then add NSEC3
     clearZoneData();
@@ -201,7 +293,7 @@ TEST_F(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"), zone_data_);
+    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.
@@ -214,4 +306,39 @@ TEST_F(ZoneDataUpdaterTest, rrsigForNSEC3Only) {
                  isc::NotImplemented);
 }
 
+// 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. 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) +
+                               ".example.org.");
+        updater_->add(textToRRset(name + " 3600 IN TXT " +
+                                  std::string(30, 'X')),
+                      textToRRset(name + " 3600 IN RRSIG TXT 5 3 3600 "
+                                  "20150420235959 20051021000000 1 "
+                                  "example.org. FAKE"));
+        ZoneNode* node = getNode(*mem_sgmt_,
+                                 Name(boost::lexical_cast<std::string>(i) +
+                                      ".example.org"), getZoneData());
+        const RdataSet* rdset = node->getData();
+        ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
+        rdset = RdataSet::find(rdset, RRType::TXT(), true);
+        ASSERT_NE(static_cast<RdataSet*>(NULL), rdset);
+        EXPECT_EQ(1, rdset->getRdataCount());
+        EXPECT_EQ(1, rdset->getSigRdataCount());
+    }
+}
+
+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());
+}
+
 }

+ 29 - 27
src/lib/datasrc/tests/memory/zone_finder_unittest.cc

@@ -98,7 +98,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
@@ -196,7 +196,7 @@ protected:
     }
 
     void addToZoneData(const ConstRRsetPtr rrset) {
-        updater_.add(rrset, rrset->getRRsig());
+        updater_->add(rrset, rrset->getRRsig());
     }
 
     /// \brief expensive rrset converter
@@ -224,7 +224,7 @@ protected:
     MemorySegmentMock 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_;
@@ -1549,19 +1549,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(),
@@ -1572,29 +1572,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,
@@ -1614,6 +1614,7 @@ TEST_F(InMemoryZoneFinderTest, NSECNonExistentTest) {
     const Name name("example.com.");
     shared_ptr<ZoneTableSegment> ztable_segment(
          new ZoneTableSegmentMock(class_, mem_sgmt_));
+    updater_.reset();
     loadZoneIntoTable(*ztable_segment, name, class_,
                       TEST_DATA_DIR "/2504-test.zone");
     InMemoryClient client(ztable_segment, class_);
@@ -1758,10 +1759,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);
 }
@@ -1776,6 +1777,7 @@ TEST_F(InMemoryZoneFinderNSEC3Test, findNSEC3MissingOrigin) {
      const Name name("example.com.");
      shared_ptr<ZoneTableSegment> ztable_segment(
           new ZoneTableSegmentMock(class_, mem_sgmt_));
+     updater_.reset();
      loadZoneIntoTable(*ztable_segment, name, class_,
                        TEST_DATA_DIR "/2503-test.zone");
      InMemoryClient client(ztable_segment, class_);

+ 20 - 11
src/lib/datasrc/tests/memory/zone_table_unittest.cc

@@ -79,7 +79,8 @@ TEST_F(ZoneTableTest, addZone) {
     EXPECT_EQ(0, zone_table->getZoneCount()); // count is still 0
 
     SegmentObjectHolder<ZoneData, RRClass> holder1(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+        mem_sgmt_, zclass_);
+    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_,
@@ -93,7 +94,8 @@ TEST_F(ZoneTableTest, addZone) {
 
     // Duplicate add doesn't replace the existing data.
     SegmentObjectHolder<ZoneData, RRClass> holder2(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+        mem_sgmt_, zclass_);
+    holder2.set(ZoneData::create(mem_sgmt_, zname1));
     const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
                                                            zname1,
                                                            holder2.release()));
@@ -107,8 +109,8 @@ TEST_F(ZoneTableTest, addZone) {
     EXPECT_EQ(1, zone_table->getZoneCount()); // count doesn't change.
 
     SegmentObjectHolder<ZoneData, RRClass> holder3(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("EXAMPLE.COM")),
-                                    zclass_);
+        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_,
                                                            Name("EXAMPLE.COM"),
@@ -117,13 +119,15 @@ TEST_F(ZoneTableTest, addZone) {
     ZoneData::destroy(mem_sgmt_, result3.zone_data, zclass_);
     // Add some more different ones.  Should just succeed.
     SegmentObjectHolder<ZoneData, RRClass> holder4(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
+        mem_sgmt_, zclass_);
+    holder4.set(ZoneData::create(mem_sgmt_, zname2));
     EXPECT_EQ(result::SUCCESS,
               zone_table->addZone(mem_sgmt_, zclass_, zname2,
                                   holder4.release()).code);
     EXPECT_EQ(2, zone_table->getZoneCount());
     SegmentObjectHolder<ZoneData, RRClass> holder5(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
+        mem_sgmt_, zclass_);
+    holder5.set(ZoneData::create(mem_sgmt_, zname3));
     EXPECT_EQ(result::SUCCESS,
               zone_table->addZone(mem_sgmt_, zclass_, zname3,
                                   holder5.release()).code);
@@ -133,7 +137,8 @@ TEST_F(ZoneTableTest, addZone) {
     // tree.  It still shouldn't cause memory leak (which would be detected
     // in TearDown()).
     SegmentObjectHolder<ZoneData, RRClass> holder6(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("example.org")), zclass_);
+        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()),
@@ -142,17 +147,20 @@ TEST_F(ZoneTableTest, addZone) {
 
 TEST_F(ZoneTableTest, findZone) {
     SegmentObjectHolder<ZoneData, RRClass> holder1(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+        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,
                                                    holder1.release()).code);
     SegmentObjectHolder<ZoneData, RRClass> holder2(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
+        mem_sgmt_, zclass_);
+    holder2.set(ZoneData::create(mem_sgmt_, zname2));
     EXPECT_EQ(result::SUCCESS,
               zone_table->addZone(mem_sgmt_, zclass_, zname2,
                                   holder2.release()).code);
     SegmentObjectHolder<ZoneData, RRClass> holder3(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
+        mem_sgmt_, zclass_);
+    holder3.set(ZoneData::create(mem_sgmt_, zname3));
     EXPECT_EQ(result::SUCCESS,
               zone_table->addZone(mem_sgmt_, zclass_, zname3,
                                   holder3.release()).code);
@@ -177,7 +185,8 @@ TEST_F(ZoneTableTest, findZone) {
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
     SegmentObjectHolder<ZoneData, RRClass> holder4(
-        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("com")), zclass_);
+        mem_sgmt_, zclass_);
+    holder4.set(ZoneData::create(mem_sgmt_, Name("com")));
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
                                                    Name("com"),
                                                    holder4.release()).code);

+ 21 - 9
src/lib/util/memory_segment_mapped.cc

@@ -171,6 +171,7 @@ struct MemorySegmentMapped::Impl {
     void growSegment() {
         // We first need to unmap it before calling grow().
         const size_t prev_size = base_sgmt_->get_size();
+        base_sgmt_->flush();
         base_sgmt_.reset();
 
         // Double the segment size.  In theory, this process could repeat
@@ -181,16 +182,21 @@ struct MemorySegmentMapped::Impl {
         const size_t new_size = prev_size * 2;
         assert(new_size > prev_size);
 
-        if (!BaseSegment::grow(filename_.c_str(), new_size - prev_size)) {
-            throw std::bad_alloc();
-        }
+        const bool grown = BaseSegment::grow(filename_.c_str(),
+                                             new_size - prev_size);
 
+        // Remap the file, whether or not grow() succeeded.  this should
+        // normally succeed(*), but it's not 100% guaranteed.  We abort
+        // if it fails (see the method description in the header file).
+        // (*) Although it's not formally documented, the implementation
+        // of grow() seems to provide strong guarantee, i.e, if it fails
+        // the underlying file can be used with the previous size.
         try {
-            // Remap the grown file; this should succeed, but it's not 100%
-            // guaranteed.  If it fails we treat it as if we fail to create
-            // the new segment.
             base_sgmt_.reset(new BaseSegment(open_only, filename_.c_str()));
-        } catch (const boost::interprocess::interprocess_exception& ex) {
+        } catch (...) {
+            abort();
+        }
+        if (!grown) {
             throw std::bad_alloc();
         }
     }
@@ -402,14 +408,20 @@ MemorySegmentMapped::shrinkToFit() {
         return;
     }
 
-    // First, (unmap and) close the underlying file.
+    // First, unmap the underlying file.
+    impl_->base_sgmt_->flush();
     impl_->base_sgmt_.reset();
 
     BaseSegment::shrink_to_fit(impl_->filename_.c_str());
     try {
         // Remap the shrunk file; this should succeed, but it's not 100%
         // guaranteed.  If it fails we treat it as if we fail to create
-        // the new segment.
+        // the new segment.  Note that this is different from the case where
+        // reset() after grow() fails.  While the same argument can apply
+        // in theory, it should be less likely that other methods will be
+        // called after shrinkToFit() (and the destructor can still be called
+        // safely), so we give the application an opportunity to handle the
+        // case as gracefully as possible.
         impl_->base_sgmt_.reset(
             new BaseSegment(open_only, impl_->filename_.c_str()));
     } catch (const boost::interprocess::interprocess_exception& ex) {

+ 8 - 1
src/lib/util/memory_segment_mapped.h

@@ -150,7 +150,14 @@ public:
 
     /// \brief Allocate/acquire a segment of memory.
     ///
-    /// This version can throw \c MemorySegmentGrown.
+    /// This version can throw \c MemorySegmentGrown.  Furthermore, there is
+    /// a very small chance that the object loses its integrity and can't be
+    /// usable in the case where \c MemorySegmentGrown would be thrown.
+    /// In this case, throwing a different exception wouldn't help, because
+    /// an application trying to provide exception safety might then call
+    /// deallocate() or named address APIs on this object, which would simply
+    /// cause a crash.  So, while suboptimal, this method just aborts the
+    /// program in this case, indicating there's no hope to shutdown cleanly.
     ///
     /// This method cannot be called if the segment object is created in the
     /// read-only mode; in that case MemorySegmentError will be thrown.

+ 3 - 2
src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -238,11 +238,12 @@ TEST_F(MemorySegmentMappedTest, allocate) {
 
 TEST_F(MemorySegmentMappedTest, badAllocate) {
     // Make the mapped file non-writable; managed_mapped_file::grow() will
-    // fail, resulting in std::bad_alloc
+    // fail, resulting in abort.
     const int ret = chmod(mapped_file, 0444);
     ASSERT_EQ(0, ret);
 
-    EXPECT_THROW(segment_->allocate(DEFAULT_INITIAL_SIZE * 2), std::bad_alloc);
+    EXPECT_DEATH_IF_SUPPORTED(
+        {segment_->allocate(DEFAULT_INITIAL_SIZE * 2);}, "");
 }
 
 // XXX: this test can cause too strong side effect (creating a very large