Merge #2209

Introduction of the ConfigurableClientList::getCachedZoneWriter() method.

src/bin/auth/command.cc

@@ -200,7 +200,7 @@ public:
         }
 
         switch (list->reload(origin)) {
-            case ConfigurableClientList::ZONE_RELOADED:
+            case ConfigurableClientList::ZONE_SUCCESS:
                 // Everything worked fine.
                 LOG_DEBUG(auth_logger, DBG_AUTH_OPS, AUTH_LOAD_ZONE)
                     .arg(zone_class).arg(origin);

src/bin/auth/tests/auth_srv_unittest.cc

@@ -15,7 +15,6 @@
 #include <config.h>
 
 #include <util/io/sockaddr_util.h>
-#include <util/memory_segment_local.h>
 
 #include <dns/message.h>
 #include <dns/messagerenderer.h>
@@ -74,6 +73,7 @@ using namespace isc::asiodns;
 using namespace isc::asiolink;
 using namespace isc::testutils;
 using namespace isc::server_common::portconfig;
+using isc::datasrc::memory::ZoneTableSegment;
 using isc::UnitTestUtil;
 using boost::scoped_ptr;
 
@@ -1401,7 +1401,9 @@ public:
              real_list, ThrowWhen throw_when, bool isc_exception,
              ConstRRsetPtr fake_rrset = ConstRRsetPtr()) :
         ConfigurableClientList(RRClass::IN()),
-        real_(real_list)
+        real_(real_list),
+        config_(Element::fromJSON("{}")),
+        ztable_segment_(ZoneTableSegment::create(*config_, RRClass::IN()))
     {
         BOOST_FOREACH(const DataSourceInfo& info, real_->getDataSources()) {
              const isc::datasrc::DataSourceClientPtr
@@ -1413,13 +1415,14 @@ public:
              data_sources_.push_back(
                  DataSourceInfo(client.get(),
                                 isc::datasrc::DataSourceClientContainerPtr(),
-                                false, RRClass::IN(), mem_sgmt_));
+                                false, RRClass::IN(), ztable_segment_));
         }
     }
 private:
     const boost::shared_ptr<isc::datasrc::ConfigurableClientList> real_;
+    const ConstElementPtr config_;
+    boost::shared_ptr<ZoneTableSegment> ztable_segment_;
     vector<isc::datasrc::DataSourceClientPtr> clients_;
-    MemorySegmentLocal mem_sgmt_;
 };
 
 } // end anonymous namespace for throwing proxy classes

src/lib/datasrc/client_list.cc

@@ -12,17 +12,21 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/memory_segment_local.h>
 
 #include "client_list.h"
 #include "client.h"
 #include "factory.h"
 #include "memory/memory_client.h"
+#include "memory/zone_table_segment.h"
+#include "memory/zone_writer.h"
+#include "memory/zone_data_loader.h"
 #include "logger.h"
 #include <dns/masterload.h>
+#include <util/memory_segment_local.h>
 
 #include <memory>
 #include <boost/foreach.hpp>
+#include <boost/bind.hpp>
 
 using namespace isc::data;
 using namespace isc::dns;
@@ -32,6 +36,7 @@ using boost::lexical_cast;
 using boost::shared_ptr;
 using boost::dynamic_pointer_cast;
 using isc::datasrc::memory::InMemoryClient;
+using isc::datasrc::memory::ZoneTableSegment;
 
 namespace isc {
 namespace datasrc {
@@ -39,21 +44,24 @@ namespace datasrc {
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
     DataSourceClient* data_src_client,
     const DataSourceClientContainerPtr& container, bool has_cache,
-    const RRClass& rrclass, MemorySegment& mem_sgmt) :
+    const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment) :
     data_src_client_(data_src_client),
     container_(container)
 {
     if (has_cache) {
-        cache_.reset(new InMemoryClient(mem_sgmt, rrclass));
+        cache_.reset(new InMemoryClient(segment, rrclass));
+        ztable_segment_ = segment;
     }
 }
 
 ConfigurableClientList::DataSourceInfo::DataSourceInfo(
-    const RRClass& rrclass, MemorySegment& mem_sgmt, bool has_cache) :
+    const RRClass& rrclass, const shared_ptr<ZoneTableSegment>& segment,
+    bool has_cache) :
     data_src_client_(NULL)
 {
     if (has_cache) {
-        cache_.reset(new InMemoryClient(mem_sgmt, rrclass));
+        cache_.reset(new InMemoryClient(segment, rrclass));
+        ztable_segment_ = segment;
     }
 }
 
@@ -64,21 +72,10 @@ ConfigurableClientList::DataSourceInfo::getCacheClient() const {
 
 ConfigurableClientList::ConfigurableClientList(const RRClass& rrclass) :
     rrclass_(rrclass),
-    mem_sgmt_(new util::MemorySegmentLocal),
     configuration_(new isc::data::ListElement),
     allow_cache_(false)
 {}
 
-ConfigurableClientList::~ConfigurableClientList() {
-    // Explicitly clear the contained data source clients, and check memory
-    // leak.  assert() (with abort on failure) may be too harsh, but
-    // it's probably better to find more leaks initially.  Once it's stabilized
-    // we should probably revisit it.
-
-    data_sources_.clear();
-    assert(mem_sgmt_->allMemoryDeallocated());
-}
-
 void
 ConfigurableClientList::configure(const ConstElementPtr& config,
                                   bool allow_cache)
@@ -90,6 +87,8 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
     size_t i(0); // Outside of the try to be able to access it in the catch
     try {
         vector<DataSourceInfo> new_data_sources;
+        shared_ptr<ZoneTableSegment> ztable_segment(
+            ZoneTableSegment::create(*config, rrclass_));
         for (; i < config->size(); ++i) {
             // Extract the parameters
             const ConstElementPtr dconf(config->get(i));
@@ -126,7 +125,8 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                     isc_throw(ConfigurationError, "The cache must be enabled "
                               "for the MasterFiles type");
                 }
-                new_data_sources.push_back(DataSourceInfo(rrclass_, *mem_sgmt_,
+                new_data_sources.push_back(DataSourceInfo(rrclass_,
+                                                          ztable_segment,
                                                           true));
             } else {
                 // Ask the factory to create the data source for us
@@ -135,7 +135,7 @@ ConfigurableClientList::configure(const ConstElementPtr& config,
                 // And put it into the vector
                 new_data_sources.push_back(DataSourceInfo(ds.first, ds.second,
                                                           want_cache, rrclass_,
-                                                          *mem_sgmt_));
+                                                          ztable_segment));
             }
 
             if (want_cache) {
@@ -337,33 +337,93 @@ ConfigurableClientList::findInternal(MutableResult& candidate,
     // and the need_updater parameter is true, get the zone there.
 }
 
+// We still provide this method for backward compatibility. But to not have
+// duplicate code, it is a thin wrapper around getCachedZoneWriter only.
 ConfigurableClientList::ReloadResult
 ConfigurableClientList::reload(const Name& name) {
+    const ZoneWriterPair result(getCachedZoneWriter(name));
+    if (result.first != ZONE_SUCCESS) {
+        return (result.first);
+    }
+
+    assert(result.second);
+    result.second->load();
+    result.second->install();
+    result.second->cleanup();
+
+    return (ZONE_SUCCESS);
+}
+
+namespace {
+
+// We would like to use boost::bind for this. However, the loadZoneData takes
+// a reference, while we have a shared pointer to the iterator -- and we need
+// to keep it alive as long as the ZoneWriter is alive. Therefore we can't
+// really just dereference it and pass it, since it would get destroyed once
+// the getCachedZoneWriter would end. This class holds the shared pointer
+// alive, otherwise is mostly simple.
+//
+// It might be doable with nested boost::bind, but it would probably look
+// more awkward and complicated than this.
+class IteratorLoader {
+public:
+    IteratorLoader(const RRClass& rrclass, const Name& name,
+                   const ZoneIteratorPtr& iterator) :
+        rrclass_(rrclass),
+        name_(name),
+        iterator_(iterator)
+    {}
+    memory::ZoneData* operator()(util::MemorySegment& segment) {
+        return (memory::loadZoneData(segment, rrclass_, name_, *iterator_));
+    }
+private:
+    const RRClass rrclass_;
+    const Name name_;
+    ZoneIteratorPtr iterator_;
+};
+
+// We can't use the loadZoneData function directly in boost::bind, since
+// it is overloaded and the compiler can't choose the correct version
+// reliably and fails. So we simply wrap it into an unique name.
+memory::ZoneData*
+loadZoneDataFromFile(util::MemorySegment& segment, const RRClass& rrclass,
+                     const Name& name, const string& filename)
+{
+    return (memory::loadZoneData(segment, rrclass, name, filename));
+}
+
+}
+
+ConfigurableClientList::ZoneWriterPair
+ConfigurableClientList::getCachedZoneWriter(const Name& name) {
     if (!allow_cache_) {
-        return (CACHE_DISABLED);
+        return (ZoneWriterPair(CACHE_DISABLED, ZoneWriterPtr()));
     }
     // Try to find the correct zone.
     MutableResult result;
     findInternal(result, name, true, true);
     if (!result.finder) {
-        return (ZONE_NOT_FOUND);
+        return (ZoneWriterPair(ZONE_NOT_FOUND, ZoneWriterPtr()));
     }
-    // Try to convert the finder to in-memory one. If it is the cache,
-    // it should work.
-    // It is of a different type or there's no cache.
+    // Try to get the in-memory cache for the zone. If there's none,
+    // we can't provide the result.
     if (!result.info->cache_) {
-        return (ZONE_NOT_CACHED);
+        return (ZoneWriterPair(ZONE_NOT_CACHED, ZoneWriterPtr()));
     }
+    memory::LoadAction load_action;
     DataSourceClient* client(result.info->data_src_client_);
-    if (client) {
-        // Now do the final reload. If it does not exist in client,
+    if (client != NULL) {
+        // Now finally provide the writer.
+        // If it does not exist in client,
         // DataSourceError is thrown, which is exactly the result what we
         // want, so no need to handle it.
         ZoneIteratorPtr iterator(client->getIterator(name));
         if (!iterator) {
             isc_throw(isc::Unexpected, "Null iterator from " << name);
         }
-        result.info->cache_->load(name, *iterator);
+        // And wrap the iterator into the correct functor (which
+        // keeps it alive as long as it is needed).
+        load_action = IteratorLoader(rrclass_, name, iterator);
     } else {
         // The MasterFiles special case
         const string filename(result.info->cache_->getFileName(name));
@@ -371,9 +431,14 @@ ConfigurableClientList::reload(const Name& name) {
             isc_throw(isc::Unexpected, "Confused about missing both filename "
                       "and data source");
         }
-        result.info->cache_->load(name, filename);
+        // boost::bind is enough here.
+        load_action = boost::bind(loadZoneDataFromFile, _1, rrclass_, name,
+                                  filename);
     }
-    return (ZONE_RELOADED);
+    return (ZoneWriterPair(ZONE_SUCCESS,
+                           ZoneWriterPtr(
+                               result.info->ztable_segment_->
+                               getZoneWriter(load_action, name, rrclass_))));
 }
 
 // NOTE: This function is not tested, it would be complicated. However, the

src/lib/datasrc/client_list.h

@@ -21,6 +21,7 @@
 #include <dns/rrclass.h>
 #include <cc/data.h>
 #include <exceptions/exceptions.h>
+#include "memory/zone_table_segment.h"
 
 #include <vector>
 #include <boost/shared_ptr.hpp>
@@ -42,6 +43,7 @@ typedef boost::shared_ptr<DataSourceClientContainer>
 // and hide real definitions except for itself and tests.
 namespace memory {
 class InMemoryClient;
+class ZoneWriter;
 }
 
 /// \brief The list of data source clients.
@@ -219,9 +221,6 @@ public:
     /// \param rrclass For which class the list should work.
     ConfigurableClientList(const isc::dns::RRClass& rrclass);
 
-    /// \brief Destructor
-    virtual ~ConfigurableClientList();
-
     /// \brief Exception thrown when there's an error in configuration.
     class ConfigurationError : public Exception {
     public:
@@ -271,7 +270,8 @@ public:
         CACHE_DISABLED,     ///< The cache is not enabled in this list.
         ZONE_NOT_CACHED,    ///< Zone is served directly, not from cache.
         ZONE_NOT_FOUND,     ///< Zone does not exist or not cached.
-        ZONE_RELOADED       ///< The zone was successfully reloaded.
+        ZONE_SUCCESS        ///< The zone was successfully reloaded or
+                            ///  the writer provided.
     };
 
     /// \brief Reloads a cached zone.
@@ -288,6 +288,36 @@ public:
     ///      the original data source no longer contains the cached zone.
     ReloadResult reload(const dns::Name& zone);
 
+private:
+    /// \brief Convenience type shortcut
+    typedef boost::shared_ptr<memory::ZoneWriter> ZoneWriterPtr;
+public:
+
+    /// \brief Return value of getCachedZoneWriter()
+    ///
+    /// A pair containing status and the zone writer, for the
+    /// getCachedZoneWriter() method.
+    typedef std::pair<ReloadResult, ZoneWriterPtr> ZoneWriterPair;
+
+    /// \brief Return a zone writer that can be used to reload a zone.
+    ///
+    /// This looks up a cached copy of zone and returns the ZoneWriter
+    /// that can be used to reload the content of the zone. This can
+    /// be used instead of reload() -- reload() works synchronously, which
+    /// is not what is needed every time.
+    ///
+    /// \param zone The origin of the zone to reload.
+    /// \return The result has two parts. The first one is a status describing
+    ///     if it worked or not (and in case it didn't, also why). If the
+    ///     status is ZONE_SUCCESS, the second part contains a shared pointer
+    ///     to the writer. If the status is anything else, the second part is
+    ///     NULL.
+    /// \throw DataSourceError or anything else that the data source
+    ///      containing the zone might throw is propagated.
+    /// \throw DataSourceError if something unexpected happens, like when
+    ///      the original data source no longer contains the cached zone.
+    ZoneWriterPair getCachedZoneWriter(const dns::Name& zone);
+
     /// \brief Implementation of the ClientList::find.
     virtual FindResult find(const dns::Name& zone,
                             bool want_exact_match = false,
@@ -299,12 +329,16 @@ public:
     struct DataSourceInfo {
         // Plays a role of default constructor too (for vector)
         DataSourceInfo(const dns::RRClass& rrclass,
-                       util::MemorySegment& mem_sgmt,
+                       const boost::shared_ptr
+                           <isc::datasrc::memory::ZoneTableSegment>&
+                               ztable_segment,
                        bool has_cache = false);
         DataSourceInfo(DataSourceClient* data_src_client,
                        const DataSourceClientContainerPtr& container,
                        bool has_cache, const dns::RRClass& rrclass,
-                       util::MemorySegment& mem_sgmt);
+                       const boost::shared_ptr
+                           <isc::datasrc::memory::ZoneTableSegment>&
+                               ztable_segment);
         DataSourceClient* data_src_client_;
         DataSourceClientContainerPtr container_;
 
@@ -315,6 +349,7 @@ public:
         // No other applications or tests may use it.
         const DataSourceClient* getCacheClient() const;
         boost::shared_ptr<memory::InMemoryClient> cache_;
+        boost::shared_ptr<memory::ZoneTableSegment> ztable_segment_;
     };
 
     /// \brief The collection of data sources.
@@ -369,12 +404,6 @@ private:
                       bool want_exact_match, bool want_finder) const;
     const isc::dns::RRClass rrclass_;
 
-    /// \brief Memory segment for in-memory cache.
-    ///
-    /// Note that this must be placed before data_sources_ so it won't be
-    /// destroyed before the built objects in the destructor.
-    boost::scoped_ptr<util::MemorySegment> mem_sgmt_;
-
     /// \brief Currently active configuration.
     isc::data::ConstElementPtr configuration_;
 

src/lib/datasrc/memory/memory_client.cc

@@ -22,6 +22,7 @@
 #include <datasrc/memory/treenode_rrset.h>
 #include <datasrc/memory/zone_finder.h>
 #include <datasrc/memory/zone_data_loader.h>
+#include <datasrc/memory/zone_table_segment.h>
 
 #include <util/memory_segment_local.h>
 
@@ -42,12 +43,14 @@ using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using namespace isc::datasrc::memory;
+using namespace isc::util;
 
 namespace isc {
 namespace datasrc {
 namespace memory {
 
 using detail::SegmentObjectHolder;
+using boost::shared_ptr;
 
 namespace { // unnamed namespace
 
@@ -64,25 +67,19 @@ public:
 
 } // end of unnamed namespace
 
-InMemoryClient::InMemoryClient(util::MemorySegment& mem_sgmt,
+InMemoryClient::InMemoryClient(shared_ptr<ZoneTableSegment> ztable_segment,
                                RRClass rrclass) :
-    mem_sgmt_(mem_sgmt),
+    ztable_segment_(ztable_segment),
     rrclass_(rrclass),
-    zone_count_(0)
-{
-    SegmentObjectHolder<ZoneTable, RRClass> holder(
-        mem_sgmt_, ZoneTable::create(mem_sgmt_, rrclass), rrclass_);
-
-    file_name_tree_ = FileNameTree::create(mem_sgmt_, false);
-
-    zone_table_ = holder.release();
-}
+    zone_count_(0),
+    file_name_tree_(FileNameTree::create(
+        ztable_segment_->getMemorySegment(), false))
+{}
 
 InMemoryClient::~InMemoryClient() {
+    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
     FileNameDeleter deleter;
-    FileNameTree::destroy(mem_sgmt_, file_name_tree_, deleter);
-
-    ZoneTable::destroy(mem_sgmt_, zone_table_, rrclass_);
+    FileNameTree::destroy(mem_sgmt, file_name_tree_, deleter);
 }
 
 result::Result
@@ -90,8 +87,9 @@ InMemoryClient::loadInternal(const isc::dns::Name& zone_name,
                              const std::string& filename,
                              ZoneData* zone_data)
 {
+    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
     SegmentObjectHolder<ZoneData, RRClass> holder(
-        mem_sgmt_, zone_data, rrclass_);
+        mem_sgmt, zone_data, rrclass_);
 
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_ADD_ZONE).
         arg(zone_name).arg(rrclass_);
@@ -99,7 +97,7 @@ InMemoryClient::loadInternal(const isc::dns::Name& zone_name,
     // Set the filename in file_name_tree_ now, so that getFileName()
     // can use it (during zone reloading).
     FileNameNode* node(NULL);
-    switch (file_name_tree_->insert(mem_sgmt_, zone_name, &node)) {
+    switch (file_name_tree_->insert(mem_sgmt, zone_name, &node)) {
     case FileNameTree::SUCCESS:
     case FileNameTree::ALREADYEXISTS:
         // These are OK
@@ -114,9 +112,10 @@ InMemoryClient::loadInternal(const isc::dns::Name& zone_name,
     const std::string* tstr = node->setData(new std::string(filename));
     delete tstr;
 
-    const ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
-                                                           zone_name,
-                                                           holder.release()));
+    ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
+    const ZoneTable::AddResult result(zone_table->addZone(mem_sgmt, rrclass_,
+                                                          zone_name,
+                                                          holder.release()));
     if (result.code == result::SUCCESS) {
         // Only increment the zone count if the zone doesn't already
         // exist.
@@ -124,7 +123,7 @@ InMemoryClient::loadInternal(const isc::dns::Name& zone_name,
     }
     // Destroy the old instance of the zone if there was any
     if (result.zone_data != NULL) {
-        ZoneData::destroy(mem_sgmt_, result.zone_data, rrclass_);
+        ZoneData::destroy(mem_sgmt, result.zone_data, rrclass_);
     }
 
     return (result.code);
@@ -145,7 +144,8 @@ InMemoryClient::findZone(const isc::dns::Name& zone_name) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA,
               DATASRC_MEMORY_MEM_FIND_ZONE).arg(zone_name);
 
-    ZoneTable::FindResult result(zone_table_->findZone(zone_name));
+    const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
+    const ZoneTable::FindResult result(zone_table->findZone(zone_name));
 
     ZoneFinderPtr finder;
     if (result.code != result::NOTFOUND) {
@@ -157,7 +157,8 @@ InMemoryClient::findZone(const isc::dns::Name& zone_name) const {
 
 const ZoneData*
 InMemoryClient::findZoneData(const isc::dns::Name& zone_name) {
-    ZoneTable::FindResult result(zone_table_->findZone(zone_name));
+    const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
+    const ZoneTable::FindResult result(zone_table->findZone(zone_name));
     return (result.zone_data);
 }
 
@@ -168,14 +169,16 @@ InMemoryClient::load(const isc::dns::Name& zone_name,
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_LOAD).arg(zone_name).
         arg(filename);
 
-    ZoneData* zone_data = loadZoneData(mem_sgmt_, rrclass_, zone_name,
+    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
+    ZoneData* zone_data = loadZoneData(mem_sgmt, rrclass_, zone_name,
                                        filename);
     return (loadInternal(zone_name, filename, zone_data));
 }
 
 result::Result
 InMemoryClient::load(const isc::dns::Name& zone_name, ZoneIterator& iterator) {
-    ZoneData* zone_data = loadZoneData(mem_sgmt_, rrclass_, zone_name,
+    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
+    ZoneData* zone_data = loadZoneData(mem_sgmt, rrclass_, zone_name,
                                        iterator);
     return (loadInternal(zone_name, string(), zone_data));
 }
@@ -207,7 +210,7 @@ private:
     bool separate_rrs_;
     bool ready_;
 public:
-    MemoryIterator(const RRClass rrclass,
+    MemoryIterator(const RRClass& rrclass,
                    const ZoneTree& tree, const Name& origin,
                    bool separate_rrs) :
         rrclass_(rrclass),
@@ -306,7 +309,8 @@ public:
 
 ZoneIteratorPtr
 InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
-    ZoneTable::FindResult result(zone_table_->findZone(name));
+    const ZoneTable* zone_table = ztable_segment_->getHeader().getTable();
+    const ZoneTable::FindResult result(zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
         isc_throw(DataSourceError, "No such zone: " + name.toText());
     }

src/lib/datasrc/memory/memory_client.h

@@ -22,6 +22,8 @@
 #include <datasrc/memory/zone_table.h>
 #include <datasrc/memory/zone_data.h>
 
+#include <boost/shared_ptr.hpp>
+
 #include <string>
 
 namespace isc {
@@ -34,6 +36,8 @@ class RRsetList;
 namespace datasrc {
 namespace memory {
 
+class ZoneTableSegment;
+
 /// \brief A data source client that holds all necessary data in memory.
 ///
 /// The \c InMemoryClient class provides an access to a conceptual data
@@ -60,7 +64,7 @@ public:
     /// This constructor internally involves resource allocation, and if
     /// it fails, a corresponding standard exception will be thrown.
     /// It never throws an exception otherwise.
-    InMemoryClient(util::MemorySegment& mem_sgmt,
+    InMemoryClient(boost::shared_ptr<ZoneTableSegment> ztable_segment,
                    isc::dns::RRClass rrclass);
 
     /// The destructor.
@@ -186,10 +190,9 @@ private:
                                 const std::string& filename,
                                 ZoneData* zone_data);
 
-    util::MemorySegment& mem_sgmt_;
+    boost::shared_ptr<ZoneTableSegment> ztable_segment_;
     const isc::dns::RRClass rrclass_;
     unsigned int zone_count_;
-    ZoneTable* zone_table_;
     FileNameTree* file_name_tree_;
 };
 

src/lib/datasrc/memory/zone_data_loader.cc

@@ -165,7 +165,7 @@ ZoneDataLoader::getCurrentName() const {
 
 ZoneData*
 loadZoneDataInternal(util::MemorySegment& mem_sgmt,
-                     const isc::dns::RRClass rrclass,
+                     const isc::dns::RRClass& rrclass,
                      const Name& zone_name,
                      boost::function<void(LoadCallback)> rrset_installer)
 {
@@ -223,7 +223,7 @@ generateRRsetFromIterator(ZoneIterator* iterator, LoadCallback callback) {
 
 ZoneData*
 loadZoneData(util::MemorySegment& mem_sgmt,
-             const isc::dns::RRClass rrclass,
+             const isc::dns::RRClass& rrclass,
              const isc::dns::Name& zone_name,
              const std::string& zone_file)
 {
@@ -236,7 +236,7 @@ loadZoneData(util::MemorySegment& mem_sgmt,
 
 ZoneData*
 loadZoneData(util::MemorySegment& mem_sgmt,
-             const isc::dns::RRClass rrclass,
+             const isc::dns::RRClass& rrclass,
              const isc::dns::Name& zone_name,
              ZoneIterator& iterator)
 {

src/lib/datasrc/memory/zone_data_loader.h

@@ -48,7 +48,7 @@ struct EmptyZone : public InvalidParameter {
 /// \param zone_name The name of the zone that is being loaded.
 /// \param zone_file Filename which contains the zone data for \c zone_name.
 ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
-                       const isc::dns::RRClass rrclass,
+                       const isc::dns::RRClass& rrclass,
                        const isc::dns::Name& zone_name,
                        const std::string& zone_file);
 
@@ -65,7 +65,7 @@ ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
 /// \param zone_name The name of the zone that is being loaded.
 /// \param iterator Iterator that returns RRsets to load into the zone.
 ZoneData* loadZoneData(util::MemorySegment& mem_sgmt,
-                       const isc::dns::RRClass rrclass,
+                       const isc::dns::RRClass& rrclass,
                        const isc::dns::Name& zone_name,
                        ZoneIterator& iterator);
 

src/lib/datasrc/memory/zone_table.cc

@@ -47,23 +47,22 @@ typedef boost::function<void(ZoneData*)> ZoneDataDeleterType;
 }
 
 ZoneTable*
-ZoneTable::create(util::MemorySegment& mem_sgmt, RRClass zone_class) {
+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));
     void* p = mem_sgmt.allocate(sizeof(ZoneTable));
-    ZoneTable* zone_table = new(p) ZoneTable(holder.get());
+    ZoneTable* zone_table = new(p) ZoneTable(zone_class, holder.get());
     holder.release();
     return (zone_table);
 }
 
 void
-ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
-                   RRClass zone_class)
+ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable)
 {
     ZoneTableTree::destroy(mem_sgmt, ztable->zones_.get(),
                            boost::bind(deleteZoneData, &mem_sgmt, _1,
-                                       zone_class));
+                                       ztable->rrclass_));
     mem_sgmt.deallocate(ztable, sizeof(ZoneTable));
 }
 

src/lib/datasrc/memory/zone_table.h

@@ -102,7 +102,9 @@ private:
     /// This constructor internally involves resource allocation, and if
     /// it fails, a corresponding standard exception will be thrown.
     /// It never throws an exception otherwise.
-    ZoneTable(ZoneTableTree* zones) : zones_(zones)
+    ZoneTable(const dns::RRClass& rrclass, ZoneTableTree* zones) :
+        rrclass_(rrclass),
+        zones_(zones)
     {}
 
 public:
@@ -119,7 +121,7 @@ public:
     /// \param zone_class The RR class of the zone.  It must be the RR class
     /// that is supposed to be associated to the zone table.
     static ZoneTable* create(util::MemorySegment& mem_sgmt,
-                             dns::RRClass zone_class);
+                             const dns::RRClass& zone_class);
 
     /// \brief Destruct and deallocate \c ZoneTable
     ///
@@ -135,8 +137,7 @@ public:
     /// \param ztable A non NULL pointer to a valid \c ZoneTable object
     /// that was originally created by the \c create() method (the behavior
     /// is undefined if this condition isn't met).
-    static void destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
-                        dns::RRClass zone_class);
+    static void destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable);
 
     /// Add a new zone to the \c ZoneTable.
     ///
@@ -185,6 +186,7 @@ public:
     FindResult findZone(const isc::dns::Name& name) const;
 
 private:
+    const dns::RRClass rrclass_;
     boost::interprocess::offset_ptr<ZoneTableTree> zones_;
 };
 }

src/lib/datasrc/memory/zone_table_segment.cc

@@ -15,17 +15,20 @@
 #include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_table_segment_local.h>
 
+using namespace isc::dns;
+
 namespace isc {
 namespace datasrc {
 namespace memory {
 
 ZoneTableSegment*
-ZoneTableSegment::create(const isc::data::Element&) {
+ZoneTableSegment::create(const isc::data::Element&, const RRClass& rrclass) {
     /// FIXME: For now, we always return ZoneTableSegmentLocal. This
     /// should be updated eventually to parse the passed Element
     /// argument and construct a corresponding ZoneTableSegment
     /// implementation.
-    return (new ZoneTableSegmentLocal);
+
+    return (new ZoneTableSegmentLocal(rrclass));
 }
 
 void

src/lib/datasrc/memory/zone_table_segment.h

@@ -15,6 +15,7 @@
 #ifndef ZONE_TABLE_SEGMENT_H
 #define ZONE_TABLE_SEGMENT_H
 
+#include <dns/rrclass.h>
 #include <datasrc/memory/zone_table.h>
 #include "load_action.h"
 #include <cc/data.h>
@@ -42,38 +43,21 @@ class ZoneWriter;
 /// map from domain names to zone locators) in memory.
 struct ZoneTableHeader {
 public:
+    ZoneTableHeader(ZoneTable* zone_table) :
+        table_(zone_table)
+    {}
+
     /// \brief Returns a pointer to the underlying zone table.
     ZoneTable* getTable() {
-        return (table.get());
+        return (table_.get());
     }
 
     /// \brief const version of \c getTable().
     const ZoneTable* getTable() const {
-        return (table.get());
+        return (table_.get());
     }
-
-    /// \brief Method to set the internal table
-    ///
-    /// The interface is tentative, we don't know if this is the correct place
-    /// and way to set the data. But for now, we need something to be there
-    /// at least for the tests. So we have this. For this reason, there are
-    /// no tests for this method directly. Do not use in actual
-    /// implementation.
-    ///
-    /// It can be used only once, to initially set it. It can't replace the
-    /// one already there.
-    ///
-    /// \param table Pointer to the table to use.
-    /// \throw isc::Unexpected if called the second time.
-    void setTable(ZoneTable* table) {
-        if (this->table.get() != NULL) {
-            isc_throw(isc::Unexpected, "Replacing table");
-        }
-        this->table = table;
-    }
-
 private:
-    boost::interprocess::offset_ptr<ZoneTable> table;
+    boost::interprocess::offset_ptr<ZoneTable> table_;
 };
 
 /// \brief Manages a ZoneTableHeader, an entry point into a table of
@@ -91,7 +75,7 @@ protected:
     /// An instance implementing this interface is expected to be
     /// created by the factory method (\c create()), so this constructor
     /// is protected.
-    ZoneTableSegment()
+    ZoneTableSegment(isc::dns::RRClass)
     {}
 public:
     /// \brief Destructor
@@ -119,7 +103,20 @@ public:
     /// \param config The configuration based on which a derived object
     ///               is returned.
     /// \return Returns a ZoneTableSegment object
-    static ZoneTableSegment* create(const isc::data::Element& config);
+    static ZoneTableSegment* create(const isc::data::Element& config,
+                                    const isc::dns::RRClass& rrclass);
+
+    /// \brief Temporary/Testing version of create.
+    ///
+    /// This exists as a temporary solution during the migration phase
+    /// towards using the ZoneTableSegment. It doesn't take a config,
+    /// but a memory segment instead. If you can, you should use the
+    /// other version, this one will be gone soon.
+    ///
+    /// \param segment The memory segment to use.
+    /// \return Returns a new ZoneTableSegment object.
+    /// \todo Remove this method.
+    static ZoneTableSegment* create(isc::util::MemorySegment& segment);
 
     /// \brief Destroy a ZoneTableSegment
     ///

src/lib/datasrc/memory/zone_table_segment_local.cc

@@ -15,12 +15,29 @@
 #include <datasrc/memory/zone_table_segment_local.h>
 #include "zone_writer_local.h"
 
+using namespace isc::dns;
 using namespace isc::util;
 
 namespace isc {
 namespace datasrc {
 namespace memory {
 
+ZoneTableSegmentLocal::ZoneTableSegmentLocal(const RRClass& rrclass) :
+    ZoneTableSegment(rrclass),
+    header_(ZoneTable::create(mem_sgmt_, rrclass))
+{
+}
+
+ZoneTableSegmentLocal::~ZoneTableSegmentLocal() {
+    // Explicitly clear the contained data, and check memory
+    // leak.  assert() (with abort on failure) may be too harsh, but
+    // it's probably better to find more leaks initially.  Once it's stabilized
+    // we should probably revisit it.
+
+    ZoneTable::destroy(mem_sgmt_, header_.getTable());
+    assert(mem_sgmt_.allMemoryDeallocated());
+}
+
 // After more methods' definitions are added here, it would be a good
 // idea to move getHeader() and getMemorySegment() definitions to the
 // header file.

src/lib/datasrc/memory/zone_table_segment_local.h

@@ -37,11 +37,10 @@ protected:
     /// Instances are expected to be created by the factory method
     /// (\c ZoneTableSegment::create()), so this constructor is
     /// protected.
-    ZoneTableSegmentLocal()
-    {}
+    ZoneTableSegmentLocal(const isc::dns::RRClass& rrclass);
 public:
     /// \brief Destructor
-    virtual ~ZoneTableSegmentLocal() {}
+    virtual ~ZoneTableSegmentLocal();
 
     /// \brief Return the ZoneTableHeader for the local zone table
     /// segment implementation.
@@ -59,8 +58,8 @@ public:
                                       const dns::Name& origin,
                                       const dns::RRClass& rrclass);
 private:
-    ZoneTableHeader header_;
     isc::util::MemorySegmentLocal mem_sgmt_;
+    ZoneTableHeader header_;
 };
 
 } // namespace memory

src/lib/datasrc/tests/client_list_unittest.cc

@@ -12,14 +12,14 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <util/memory_segment_local.h>
-
 #include <datasrc/client_list.h>
 #include <datasrc/client.h>
 #include <datasrc/iterator.h>
 #include <datasrc/data_source.h>
 #include <datasrc/memory/memory_client.h>
+#include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_finder.h>
+#include <datasrc/memory/zone_writer.h>
 
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
@@ -32,6 +32,7 @@
 
 using namespace isc::datasrc;
 using isc::datasrc::memory::InMemoryClient;
+using isc::datasrc::memory::ZoneTableSegment;
 using isc::datasrc::memory::InMemoryZoneFinder;
 using namespace isc::data;
 using namespace isc::dns;
@@ -255,7 +256,9 @@ public:
             "   \"type\": \"test_type\","
             "   \"params\": [\"example.org\", \"example.com\", "
             "                \"noiter.org\", \"null.org\"]"
-            "}]"))
+            "}]")),
+        config_(Element::fromJSON("{}")),
+        ztable_segment_(ZoneTableSegment::create(*config_, rrclass_))
     {
         for (size_t i(0); i < ds_count; ++ i) {
             shared_ptr<MockDataSourceClient>
@@ -263,7 +266,7 @@ public:
             ds_.push_back(ds);
             ds_info_.push_back(ConfigurableClientList::DataSourceInfo(
                                    ds.get(), DataSourceClientContainerPtr(),
-                                   false, rrclass_, mem_sgmt_));
+                                   false, rrclass_, ztable_segment_));
         }
     }
 
@@ -283,13 +286,14 @@ public:
 
         // Create cache from the temporary data source, and push it to the
         // client list.
-        const shared_ptr<InMemoryClient> cache(new InMemoryClient(mem_sgmt_,
-                                                                  rrclass_));
+        const shared_ptr<InMemoryClient> cache(
+            new InMemoryClient(ztable_segment_, rrclass_));
         cache->load(zone, *mock_client.getIterator(zone, false));
 
         ConfigurableClientList::DataSourceInfo& dsrc_info =
                 list_->getDataSources()[index];
         dsrc_info.cache_ = cache;
+        dsrc_info.ztable_segment_ = ztable_segment_;
     }
     // Check the positive result is as we expect it.
     void positiveResult(const ClientList::FindResult& result,
@@ -362,12 +366,12 @@ public:
                   shared_ptr<InMemoryClient>());
     }
     const RRClass rrclass_;
-    isc::util::MemorySegmentLocal mem_sgmt_;
     shared_ptr<TestedList> list_;
     const ClientList::FindResult negative_result_;
     vector<shared_ptr<MockDataSourceClient> > ds_;
     vector<ConfigurableClientList::DataSourceInfo> ds_info_;
-    const ConstElementPtr config_elem_, config_elem_zones_;
+    const ConstElementPtr config_elem_, config_elem_zones_, config_;
+    shared_ptr<ZoneTableSegment> ztable_segment_;
 };
 
 // Test the test itself
@@ -844,116 +848,169 @@ TEST_F(ListTest, BadMasterFile) {
                    true);
 }
 
+// This allows us to test two versions of the reloading code
+// (One by calling reload(), one by obtaining a ZoneWriter and
+// playing with that). Once we deprecate reload(), we should revert this
+// change and not use typed tests any more.
+template<class UpdateType>
+class ReloadTest : public ListTest {
+public:
+    ConfigurableClientList::ReloadResult doReload(const Name& origin);
+};
+
+// Version with calling reload()
+class ReloadUpdateType {};
+template<>
+ConfigurableClientList::ReloadResult
+ReloadTest<ReloadUpdateType>::doReload(const Name& origin) {
+    return (list_->reload(origin));
+};
+
+// Version with the ZoneWriter
+class WriterUpdateType {};
+template<>
+ConfigurableClientList::ReloadResult
+ReloadTest<WriterUpdateType>::doReload(const Name& origin) {
+    ConfigurableClientList::ZoneWriterPair
+        result(list_->getCachedZoneWriter(origin));
+    if (result.first == ConfigurableClientList::ZONE_SUCCESS) {
+        // Can't use ASSERT_NE here, it would want to return(), which
+        // it can't in non-void function.
+        if (result.second) {
+            result.second->load();
+            result.second->install();
+            result.second->cleanup();
+        } else {
+            ADD_FAILURE() << "getCachedZoneWriter returned ZONE_SUCCESS, "
+                "but the writer is NULL";
+        }
+    } else {
+        EXPECT_EQ(static_cast<memory::ZoneWriter*>(NULL),
+                  result.second.get());
+    }
+    return (result.first);
+}
+
+// Typedefs for the GTEST guts to make it work
+typedef ::testing::Types<ReloadUpdateType, WriterUpdateType> UpdateTypes;
+TYPED_TEST_CASE(ReloadTest, UpdateTypes);
+
 // Test we can reload a zone
-TEST_F(ListTest, reloadSuccess) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadSuccess) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("example.org");
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The cache currently contains a tweaked version of zone, which doesn't
     // have apex NS.  So the lookup should result in NXRRSET.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
     // Now reload the full zone. It should be there now.
-    EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED, list_->reload(name));
+    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, this->doReload(name));
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
 }
 
 // The cache is not enabled. The load should be rejected.
-TEST_F(ListTest, reloadNotEnabled) {
-    list_->configure(config_elem_zones_, false);
+TYPED_TEST(ReloadTest, reloadNotEnabled) {
+    this->list_->configure(this->config_elem_zones_, false);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the thing.
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // See the reloadSuccess test.  This should result in NXRRSET.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
     // Now reload. It should reject it.
-    EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, list_->reload(name));
+    EXPECT_EQ(ConfigurableClientList::CACHE_DISABLED, this->doReload(name));
     // Nothing changed here
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(name).finder_->find(name, RRType::NS())->code);
+              this->list_->find(name).finder_->find(name, RRType::NS())->code);
 }
 
 // Test several cases when the zone does not exist
-TEST_F(ListTest, reloadNoSuchZone) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadNoSuchZone) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("example.org");
     // We put the cache in even when not enabled. This won't confuse the
     // reload method, as that one looks at the real state of things, not
     // at the configuration.
-    prepareCache(0, Name("example.com"));
+    this->prepareCache(0, Name("example.com"));
     // Not in the data sources
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
-              list_->reload(Name("example.cz")));
+              this->doReload(Name("exmaple.cz")));
     // Not cached
-    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, list_->reload(name));
+    EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND, this->doReload(name));
     // Partial match
     EXPECT_EQ(ConfigurableClientList::ZONE_NOT_FOUND,
-              list_->reload(Name("sub.example.com")));
+              this->doReload(Name("sub.example.com")));
     // Nothing changed here - these zones don't exist
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              list_->find(name).dsrc_client_);
+              this->list_->find(name).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              list_->find(Name("example.cz")).dsrc_client_);
+              this->list_->find(Name("example.cz")).dsrc_client_);
     EXPECT_EQ(static_cast<isc::datasrc::DataSourceClient*>(NULL),
-              list_->find(Name("sub.example.com"), true).dsrc_client_);
+              this->list_->find(Name("sub.example.com"), true).dsrc_client_);
     // Not reloaded, so NS shouldn't be visible yet.
     EXPECT_EQ(ZoneFinder::NXRRSET,
-              list_->find(Name("example.com")).finder_->
+              this->list_->find(Name("example.com")).finder_->
               find(Name("example.com"), RRType::NS())->code);
 }
 
 // Check we gracefuly throw an exception when a zone disappeared in
 // the underlying data source when we want to reload it
-TEST_F(ListTest, reloadZoneGone) {
-    list_->configure(config_elem_, true);
+TYPED_TEST(ReloadTest, reloadZoneGone) {
+    this->list_->configure(this->config_elem_, true);
     const Name name("example.org");
     // We put in a cache for non-existant zone. This emulates being loaded
     // and then the zone disappearing. We prefill the cache, so we can check
     // it.
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The (cached) zone contains zone's SOA
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
     // The zone is not there, so abort the reload.
-    EXPECT_THROW(list_->reload(name), DataSourceError);
+    EXPECT_THROW(this->doReload(name), DataSourceError);
     // The (cached) zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
 }
 
 // The underlying data source throws. Check we don't modify the state.
-TEST_F(ListTest, reloadZoneThrow) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadZoneThrow) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("noiter.org");
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
     // The iterator throws, so abort the reload.
-    EXPECT_THROW(list_->reload(name), isc::NotImplemented);
+    EXPECT_THROW(this->doReload(name), isc::NotImplemented);
     // The zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
 }
 
-TEST_F(ListTest, reloadNullIterator) {
-    list_->configure(config_elem_zones_, true);
+TYPED_TEST(ReloadTest, reloadNullIterator) {
+    this->list_->configure(this->config_elem_zones_, true);
     const Name name("null.org");
-    prepareCache(0, name);
+    this->prepareCache(0, name);
     // The zone contains stuff now
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
     // The iterator throws, so abort the reload.
-    EXPECT_THROW(list_->reload(name), isc::Unexpected);
+    EXPECT_THROW(this->doReload(name), isc::Unexpected);
     // The zone is not hurt.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(name).finder_->find(name, RRType::SOA())->code);
+              this->list_->find(name).finder_->find(name,
+                                                    RRType::SOA())->code);
 }
 
 // Test we can reload the master files too (special-cased)
-TEST_F(ListTest, reloadMasterFile) {
+TYPED_TEST(ReloadTest, reloadMasterFile) {
     const char* const install_cmd = INSTALL_PROG " -c " TEST_DATA_DIR
         "/root.zone " TEST_DATA_BUILDDIR "/root.zone.copied";
     if (system(install_cmd) != 0) {
@@ -971,21 +1028,21 @@ TEST_F(ListTest, reloadMasterFile) {
         "       \".\": \"" TEST_DATA_BUILDDIR "/root.zone.copied\""
         "   }"
         "}]"));
-    list_->configure(elem, true);
+    this->list_->configure(elem, true);
     // Add a record that is not in the zone
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
-                                                   RRType::TXT())->code);
+              this->list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
+                                                         RRType::TXT())->code);
     ofstream f;
     f.open(TEST_DATA_BUILDDIR "/root.zone.copied", ios::out | ios::app);
     f << "nosuchdomain.\t\t3600\tIN\tTXT\ttest" << std::endl;
     f.close();
     // Do the reload.
-    EXPECT_EQ(ConfigurableClientList::ZONE_RELOADED, list_->reload(Name(".")));
+    EXPECT_EQ(ConfigurableClientList::ZONE_SUCCESS, this->doReload(Name(".")));
     // It is here now.
     EXPECT_EQ(ZoneFinder::SUCCESS,
-              list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
-                                                   RRType::TXT())->code);
+              this->list_->find(Name(".")).finder_->find(Name("nosuchdomain"),
+                                                         RRType::TXT())->code);
 }
 
 }

src/lib/datasrc/tests/memory/Makefile.am

@@ -32,6 +32,7 @@ run_unittests_SOURCES += ../../tests/faked_nsec3.h ../../tests/faked_nsec3.cc
 run_unittests_SOURCES += memory_segment_test.h
 run_unittests_SOURCES += segment_object_holder_unittest.cc
 run_unittests_SOURCES += memory_client_unittest.cc
+run_unittests_SOURCES += zone_table_segment_test.h
 run_unittests_SOURCES += zone_table_segment_unittest.cc
 run_unittests_SOURCES += zone_writer_unittest.cc
 

src/lib/datasrc/tests/memory/memory_client_unittest.cc

@@ -36,16 +36,22 @@
 #include <testutils/dnsmessage_test.h>
 
 #include "memory_segment_test.h"
+#include "zone_table_segment_test.h"
 
 #include <gtest/gtest.h>
 
+#include <boost/lexical_cast.hpp>
+#include <boost/shared_ptr.hpp>
+
 #include <new>                  // for bad_alloc
 
+using namespace isc::data;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 using namespace isc::datasrc;
 using namespace isc::datasrc::memory;
 using namespace isc::testutils;
+using boost::shared_ptr;
 using std::vector;
 
 namespace {
@@ -156,20 +162,22 @@ public:
 class MemoryClientTest : public ::testing::Test {
 protected:
     MemoryClientTest() : zclass_(RRClass::IN()),
-                         client_(new InMemoryClient(mem_sgmt_, zclass_))
+                         ztable_segment_(new test::ZoneTableSegmentTest(
+                             zclass_, mem_sgmt_)),
+                         client_(new InMemoryClient(ztable_segment_, zclass_))
     {}
     ~MemoryClientTest() {
-        if (client_ != NULL) {
-            delete client_;
-        }
+        delete client_;
     }
     void TearDown() {
         delete client_;
         client_ = NULL;
+        ztable_segment_.reset();
         EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated()); // catch any leak here.
     }
     const RRClass zclass_;
     test::MemorySegmentTest mem_sgmt_;
+    shared_ptr<ZoneTableSegment> ztable_segment_;
     InMemoryClient* client_;
 };
 
@@ -289,14 +297,19 @@ TEST_F(MemoryClientTest, loadMemoryAllocationFailures) {
     // Just to check that things get cleaned up
 
     for (int i = 1; i < 16; i++) {
-        SCOPED_TRACE("For throw count = " + i);
+        SCOPED_TRACE("For throw count = " +
+                     boost::lexical_cast<std::string>(i));
         mem_sgmt_.setThrowCount(i);
         EXPECT_THROW({
+            shared_ptr<ZoneTableSegment> ztable_segment(
+                new test::ZoneTableSegmentTest(
+                    zclass_, mem_sgmt_));
+
             // Include the InMemoryClient construction too here. Now,
             // even allocations done from InMemoryClient constructor
             // fail (due to MemorySegmentTest throwing) and we check for
             // leaks when this happens.
-            InMemoryClient client2(mem_sgmt_, zclass_);
+            InMemoryClient client2(ztable_segment, zclass_);
             client2.load(Name("example.org"),
                          TEST_DATA_DIR "/example.org.zone");
         }, std::bad_alloc);

src/lib/datasrc/tests/memory/zone_table_segment_test.h

@@ -0,0 +1,116 @@
+// Copyright (C) 2012  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.
+
+#ifndef DATASRC_MEMORY_ZONE_TABLE_SEGMENT_TEST_H
+#define DATASRC_MEMORY_ZONE_TABLE_SEGMENT_TEST_H 1
+
+#include <datasrc/memory/zone_table_segment.h>
+#include <datasrc/memory/zone_table.h>
+#include <datasrc/memory/zone_data.h>
+#include <datasrc/memory/zone_writer_local.h>
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+namespace test {
+
+// A special ZoneTableSegment that can be used for tests.  It can be
+// passed a MemorySegment that can be used later to test if all memory
+// was de-allocated on it.
+class ZoneTableSegmentTest : public ZoneTableSegment {
+public:
+    ZoneTableSegmentTest(isc::dns::RRClass rrclass,
+                         isc::util::MemorySegment& mem_sgmt) :
+        ZoneTableSegment(rrclass),
+        mem_sgmt_(mem_sgmt),
+        header_(ZoneTable::create(mem_sgmt_, rrclass))
+    {}
+
+    virtual ~ZoneTableSegmentTest() {
+        ZoneTable::destroy(mem_sgmt_, header_.getTable());
+    }
+
+    virtual ZoneTableHeader& getHeader() {
+        return (header_);
+    }
+
+    virtual const ZoneTableHeader& getHeader() const {
+        return (header_);
+    }
+
+    virtual isc::util::MemorySegment& getMemorySegment() {
+        return (mem_sgmt_);
+    }
+
+    virtual ZoneWriter* getZoneWriter(const LoadAction& load_action,
+                                      const dns::Name& name,
+                                      const dns::RRClass& rrclass)
+    {
+        return (new Writer(this, load_action, name, rrclass));
+    }
+
+private:
+    isc::util::MemorySegment& mem_sgmt_;
+    ZoneTableHeader header_;
+
+    // A writer for this segment. The implementation is similar
+    // to ZoneWriterLocal, but all the error handling is stripped
+    // for simplicity. Also, we do everything inside the
+    // install(), for the same reason. We just need something
+    // inside the tests, not a full-blown implementation
+    // for background loading.
+    class Writer : public ZoneWriter {
+    public:
+        Writer(ZoneTableSegmentTest* segment, const LoadAction& load_action,
+               const dns::Name& name, const dns::RRClass& rrclass) :
+            segment_(segment),
+            load_action_(load_action),
+            name_(name),
+            rrclass_(rrclass)
+        {}
+
+        void load() {}
+
+        void install() {
+            ZoneTable* table(segment_->getHeader().getTable());
+            const ZoneTable::AddResult
+                result(table->addZone(segment_->getMemorySegment(), rrclass_,
+                                      name_,
+                                      load_action_(segment_->
+                                                   getMemorySegment())));
+            if (result.zone_data != NULL) {
+                ZoneData::destroy(segment_->getMemorySegment(),
+                                  result.zone_data, rrclass_);
+            }
+        }
+
+        virtual void cleanup() {}
+    private:
+        ZoneTableSegmentTest* segment_;
+        LoadAction load_action_;
+        dns::Name name_;
+        dns::RRClass rrclass_;
+    };
+};
+
+} // namespace test
+} // namespace memory
+} // namespace datasrc
+} // namespace isc
+
+#endif // DATASRC_MEMORY_ZONE_TABLE_SEGMENT_TEST_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/datasrc/tests/memory/zone_table_segment_unittest.cc

@@ -12,78 +12,68 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/memory/zone_writer_local.h>
-#include <gtest/gtest.h>
+#include <datasrc/memory/zone_table_segment_local.h>
+#include <util/memory_segment_local.h>
 
+#include <gtest/gtest.h>
 #include <boost/scoped_ptr.hpp>
 
-using boost::scoped_ptr;
-using isc::dns::Name;
-using isc::dns::RRClass;
+using namespace isc::dns;
 using namespace isc::datasrc::memory;
 using namespace isc::data;
 using namespace isc::util;
 using namespace std;
+using boost::scoped_ptr;
 
 namespace {
 
 class ZoneTableSegmentTest : public ::testing::Test {
 protected:
     ZoneTableSegmentTest() :
-        config_(Element::fromJSON("{}")),
-        segment_(ZoneTableSegment::create((*config_.get())))
+        ztable_segment_(ZoneTableSegment::create(isc::data::NullElement(),
+                                                 RRClass::IN()))
     {}
 
-    ~ZoneTableSegmentTest() {
-        if (segment_ != NULL) {
-            ZoneTableSegment::destroy(segment_);
-        }
-    }
-
     void TearDown() {
-        // Catch any future leaks here.
-        const MemorySegment& mem_sgmt = segment_->getMemorySegment();
-        EXPECT_TRUE(mem_sgmt.allMemoryDeallocated());
-
-        ZoneTableSegment::destroy(segment_);
-        segment_ = NULL;
+        ZoneTableSegment::destroy(ztable_segment_);
+        ztable_segment_ = NULL;
     }
 
-    const ElementPtr config_;
-    ZoneTableSegment* segment_;
+    ZoneTableSegment* ztable_segment_;
 };
 
 
 TEST_F(ZoneTableSegmentTest, create) {
     // By default, a local zone table segment is created.
-    EXPECT_NE(static_cast<void*>(NULL), segment_);
+    EXPECT_NE(static_cast<void*>(NULL), ztable_segment_);
 }
 
 // Helper function to check const and non-const methods.
 template <typename TS, typename TH, typename TT>
 void
-testGetHeader(ZoneTableSegment* segment) {
-    TH& header = static_cast<TS*>(segment)->getHeader();
+testGetHeader(ZoneTableSegment* ztable_segment) {
+    TH& header = static_cast<TS*>(ztable_segment)->getHeader();
 
-    // The zone table is unset.
+    // The zone table must be set.
     TT* table = header.getTable();
-    EXPECT_EQ(static_cast<void*>(NULL), table);
+    EXPECT_NE(static_cast<void*>(NULL), table);
 }
 
 TEST_F(ZoneTableSegmentTest, getHeader) {
     // non-const version.
-    testGetHeader<ZoneTableSegment, ZoneTableHeader, ZoneTable>(segment_);
+    testGetHeader<ZoneTableSegment, ZoneTableHeader, ZoneTable>
+        (ztable_segment_);
 
     // const version.
     testGetHeader<const ZoneTableSegment, const ZoneTableHeader,
-                  const ZoneTable>(segment_);
+                  const ZoneTable>(ztable_segment_);
 }
 
 TEST_F(ZoneTableSegmentTest, getMemorySegment) {
     // This doesn't do anything fun except test the API.
-    MemorySegment& mem_sgmt = segment_->getMemorySegment();
-    EXPECT_TRUE(mem_sgmt.allMemoryDeallocated());
+    MemorySegment& mem_sgmt = ztable_segment_->getMemorySegment();
+    mem_sgmt.allMemoryDeallocated(); // use mem_sgmt
 }
 
 ZoneData*
@@ -95,8 +85,8 @@ loadAction(MemorySegment&) {
 // Test we can get a writer.
 TEST_F(ZoneTableSegmentTest, getZoneWriter) {
     scoped_ptr<ZoneWriter>
-        writer(segment_->getZoneWriter(loadAction, Name("example.org"),
-                                       RRClass::IN()));
+        writer(ztable_segment_->getZoneWriter(loadAction, Name("example.org"),
+                                              RRClass::IN()));
     // We have to get something
     EXPECT_NE(static_cast<void*>(NULL), writer.get());
     // And for now, it should be the local writer

src/lib/datasrc/tests/memory/zone_table_unittest.cc

@@ -46,11 +46,11 @@ protected:
     {}
     ~ZoneTableTest() {
         if (zone_table != NULL) {
-            ZoneTable::destroy(mem_sgmt_, zone_table, zclass_);
+            ZoneTable::destroy(mem_sgmt_, zone_table);
         }
     }
     void TearDown() {
-        ZoneTable::destroy(mem_sgmt_, zone_table, zclass_);
+        ZoneTable::destroy(mem_sgmt_, zone_table);
         zone_table = NULL;
         EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated()); // catch any leak here.
     }

src/lib/datasrc/tests/memory/zone_writer_unittest.cc

@@ -41,7 +41,8 @@ public:
         // FIXME: The NullElement probably isn't the best one, but we don't
         // know how the config will look, so it just fills the argument
         // (which is currently ignored)
-        segment_(ZoneTableSegment::create(isc::data::NullElement())),
+        segment_(ZoneTableSegment::create(isc::data::NullElement(),
+                                          RRClass::IN())),
         writer_(new
             ZoneWriterLocal(dynamic_cast<ZoneTableSegmentLocal*>(segment_.
                                                                  get()),
@@ -51,20 +52,10 @@ public:
         load_throw_(false),
         load_null_(false),
         load_data_(false)
-    {
-        // TODO: The setTable is only a temporary interface
-        segment_->getHeader().
-            setTable(ZoneTable::create(segment_->getMemorySegment(),
-                                       RRClass::IN()));
-    }
+    {}
     void TearDown() {
         // Release the writer
         writer_.reset();
-        // Release the table we used
-        ZoneTable::destroy(segment_->getMemorySegment(),
-                           segment_->getHeader().getTable(), RRClass::IN());
-        // And check we freed all memory
-        EXPECT_TRUE(segment_->getMemorySegment().allMemoryDeallocated());
     }
 protected:
     scoped_ptr<ZoneTableSegment> segment_;

src/lib/datasrc/tests/zone_finder_context_unittest.cc

@@ -14,14 +14,13 @@
 
 #include <exceptions/exceptions.h>
 
-#include <util/memory_segment_local.h>
-
 #include <dns/masterload.h>
 #include <dns/name.h>
 #include <dns/rrclass.h>
 
 #include <datasrc/zone.h>
 #include <datasrc/memory/memory_client.h>
+#include <datasrc/memory/zone_table_segment.h>
 #include <datasrc/database.h>
 #include <datasrc/sqlite3_accessor.h>
 
@@ -41,10 +40,12 @@
 using namespace std;
 using boost::shared_ptr;
 
+using namespace isc::data;
 using namespace isc::util;
 using namespace isc::dns;
 using namespace isc::datasrc;
 using isc::datasrc::memory::InMemoryClient;
+using isc::datasrc::memory::ZoneTableSegment;
 using namespace isc::testutils;
 
 namespace {
@@ -58,15 +59,17 @@ typedef shared_ptr<DataSourceClient> DataSourceClientPtr;
 // This is the type used as the test parameter.  Note that this is
 // intentionally a plain old type (i.e. a function pointer), not a class;
 // otherwise it could cause initialization fiasco at the instantiation time.
-typedef DataSourceClientPtr (*ClientCreator)(MemorySegment&, RRClass,
-                                             const Name&);
+typedef DataSourceClientPtr (*ClientCreator)(RRClass, const Name&);
 
 // Creator for the in-memory client to be tested
 DataSourceClientPtr
-createInMemoryClient(MemorySegment& mem_sgmt, RRClass zclass,
-                     const Name& zname)
+createInMemoryClient(RRClass zclass, const Name& zname)
 {
-    shared_ptr<InMemoryClient> client(new InMemoryClient(mem_sgmt, zclass));
+    const ElementPtr config(Element::fromJSON("{}"));
+    shared_ptr<ZoneTableSegment> ztable_segment(
+        ZoneTableSegment::create(*config, zclass));
+    shared_ptr<InMemoryClient> client(new InMemoryClient(ztable_segment,
+                                                         zclass));
     client->load(zname, TEST_ZONE_FILE);
 
     return (client);
@@ -78,7 +81,7 @@ addRRset(ZoneUpdaterPtr updater, ConstRRsetPtr rrset) {
 }
 
 DataSourceClientPtr
-createSQLite3Client(MemorySegment&, RRClass zclass, const Name& zname) {
+createSQLite3Client(RRClass zclass, const Name& zname) {
     // We always begin with an empty template SQLite3 DB file and install
     // the zone data from the zone file to ensure both cases have the
     // same test data.
@@ -105,7 +108,7 @@ class ZoneFinderContextTest :
 {
 protected:
     ZoneFinderContextTest() : qclass_(RRClass::IN()), qzone_("example.org") {
-        client_ = (*GetParam())(mem_sgmt_, qclass_, qzone_);
+        client_ = (*GetParam())(qclass_, qzone_);
         REQUESTED_A.push_back(RRType::A());
         REQUESTED_AAAA.push_back(RRType::AAAA());
         REQUESTED_BOTH.push_back(RRType::A());
@@ -116,7 +119,6 @@ protected:
         ASSERT_TRUE(finder_);
     }
 
-    MemorySegmentLocal mem_sgmt_;
     const RRClass qclass_;
     const Name qzone_;
     DataSourceClientPtr client_;