Browse Source

[2091] use memory segment for creating/destroying RBTrees.

many parts of the code had to be updated, but this is basically pretty
straightforward refactoring (except for exception consideration).
JINMEI Tatuya 12 years ago
parent
commit
2e99938fc9

+ 54 - 11
src/lib/datasrc/memory_datasrc.cc

@@ -14,6 +14,8 @@
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
+#include <util/memory_segment_local.h>
+
 #include <dns/name.h>
 #include <dns/name.h>
 #include <dns/nsec3hash.h>
 #include <dns/nsec3hash.h>
 #include <dns/rdataclass.h>
 #include <dns/rdataclass.h>
@@ -120,8 +122,14 @@ typedef NSEC3Map::value_type NSEC3Pair;
 // Actual zone data: Essentially a set of zone's RRs.  This is defined as
 // Actual zone data: Essentially a set of zone's RRs.  This is defined as
 // a separate structure so that it'll be replaceable on reload.
 // a separate structure so that it'll be replaceable on reload.
 struct ZoneData {
 struct ZoneData {
+    // Note: this code is not entirely exception safe; domains_storage_ could
+    // leak if the constructor throws.  But since it's an intermediate version
+    // toward a full revision and the actual risk of leak should be very small
+    // in practice, we leave it open for now.
     ZoneData(const Name& origin) :
     ZoneData(const Name& origin) :
-        domains_(true),
+        domains_storage_(DomainTree::create(local_mem_sgmt_, true)),
+        domains_(*domains_storage_),
+        aux_wild_domains_(NULL),
         origin_data_(NULL),
         origin_data_(NULL),
         nsec_signed_(false)
         nsec_signed_(false)
     {
     {
@@ -131,8 +139,29 @@ struct ZoneData {
         origin_data_->setData(origin_domain);
         origin_data_->setData(origin_domain);
     }
     }
 
 
-    // The main data (name + RRsets)
-    DomainTree domains_;
+    ~ZoneData() {
+        DomainTree::destroy(local_mem_sgmt_, domains_storage_);
+        if (aux_wild_domains_ != NULL) {
+            DomainTree::destroy(local_mem_sgmt_, aux_wild_domains_);
+        }
+
+        // The assert may be too harsh, but we assume we'll discard (rewrite)
+        // this code soon enough.  Until then this would be a good way to
+        // detect any memory leak.
+        assert(local_mem_sgmt_.allMemoryDeallocated());
+    }
+
+    // Memory segment to allocate/deallocate memory for the tree and the nodes.
+    // (This will eventually have to be abstract; for now we hardcode the
+    // specific derived segment class).
+    util::MemorySegmentLocal local_mem_sgmt_;
+
+    // The main data (name + RRsets).  We use domains_ as a reference to
+    // domains_storage_ so we don't have to update the rest of the code;
+    // it will eventually have to be revised substantially, at which point
+    // we should clean this up, too.
+    DomainTree* domains_storage_;
+    DomainTree& domains_;
 
 
     // An auxiliary tree for wildcard expanded data used in additional data
     // An auxiliary tree for wildcard expanded data used in additional data
     // processing.  It contains names like "ns.wild.example" in the following
     // processing.  It contains names like "ns.wild.example" in the following
@@ -150,11 +179,11 @@ struct ZoneData {
     // should be even empty, and even if it has content it should be very
     // should be even empty, and even if it has content it should be very
     // small.
     // small.
 private:
 private:
-    scoped_ptr<DomainTree> aux_wild_domains_;
+    DomainTree* aux_wild_domains_;
 public:
 public:
     DomainTree& getAuxWildDomains() {
     DomainTree& getAuxWildDomains() {
-        if (!aux_wild_domains_) {
-            aux_wild_domains_.reset(new DomainTree);
+        if (aux_wild_domains_ == NULL) {
+            aux_wild_domains_ = DomainTree::create(local_mem_sgmt_);
         }
         }
         return (*aux_wild_domains_);
         return (*aux_wild_domains_);
     }
     }
@@ -1782,9 +1811,23 @@ InMemoryZoneFinder::getFileName() const {
 /// member variables later for new features.
 /// member variables later for new features.
 class InMemoryClient::InMemoryClientImpl {
 class InMemoryClient::InMemoryClientImpl {
 public:
 public:
-    InMemoryClientImpl() : zone_count(0) {}
+    InMemoryClientImpl() : zone_count(0),
+                           zone_table(ZoneTable::create(local_mem_sgmt_))
+    {}
+    ~InMemoryClientImpl() {
+        ZoneTable::destroy(local_mem_sgmt_, zone_table);
+
+        // see above for the assert().
+        assert(local_mem_sgmt_.allMemoryDeallocated());
+    }
+private:
+    // Memory segment to allocate/deallocate memory for the zone table.
+    // (This will eventually have to be abstract; for now we hardcode the
+    // specific derived segment class).
+    util::MemorySegmentLocal local_mem_sgmt_;
+public:
     unsigned int zone_count;
     unsigned int zone_count;
-    ZoneTable zone_table;
+    ZoneTable* zone_table;
 };
 };
 
 
 InMemoryClient::InMemoryClient() : impl_(new InMemoryClientImpl)
 InMemoryClient::InMemoryClient() : impl_(new InMemoryClientImpl)
@@ -1809,7 +1852,7 @@ InMemoryClient::addZone(ZoneFinderPtr zone_finder) {
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_ADD_ZONE).
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEM_ADD_ZONE).
         arg(zone_finder->getOrigin()).arg(zone_finder->getClass().toText());
         arg(zone_finder->getOrigin()).arg(zone_finder->getClass().toText());
 
 
-    const result::Result result = impl_->zone_table.addZone(zone_finder);
+    const result::Result result = impl_->zone_table->addZone(zone_finder);
     if (result == result::SUCCESS) {
     if (result == result::SUCCESS) {
         ++impl_->zone_count;
         ++impl_->zone_count;
     }
     }
@@ -1819,7 +1862,7 @@ InMemoryClient::addZone(ZoneFinderPtr zone_finder) {
 InMemoryClient::FindResult
 InMemoryClient::FindResult
 InMemoryClient::findZone(const isc::dns::Name& name) const {
 InMemoryClient::findZone(const isc::dns::Name& name) const {
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_ZONE).arg(name);
     LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_ZONE).arg(name);
-    ZoneTable::FindResult result(impl_->zone_table.findZone(name));
+    ZoneTable::FindResult result(impl_->zone_table->findZone(name));
     return (FindResult(result.code, result.zone));
     return (FindResult(result.code, result.zone));
 }
 }
 
 
@@ -1925,7 +1968,7 @@ public:
 
 
 ZoneIteratorPtr
 ZoneIteratorPtr
 InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
 InMemoryClient::getIterator(const Name& name, bool separate_rrs) const {
-    ZoneTable::FindResult result(impl_->zone_table.findZone(name));
+    ZoneTable::FindResult result(impl_->zone_table->findZone(name));
     if (result.code != result::SUCCESS) {
     if (result.code != result::SUCCESS) {
         isc_throw(DataSourceError, "No such zone: " + name.toText());
         isc_throw(DataSourceError, "No such zone: " + name.toText());
     }
     }

+ 47 - 4
src/lib/datasrc/rbtree.h

@@ -24,11 +24,12 @@
 ///     to be used as a base data structure by other modules.
 ///     to be used as a base data structure by other modules.
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
-
+#include <util/memory_segment.h>
 #include <dns/name.h>
 #include <dns/name.h>
+
 #include <boost/utility.hpp>
 #include <boost/utility.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_ptr.hpp>
-#include <exceptions/exceptions.h>
+
 #include <ostream>
 #include <ostream>
 #include <algorithm>
 #include <algorithm>
 #include <cassert>
 #include <cassert>
@@ -708,18 +709,60 @@ public:
         ALREADYEXISTS,
         ALREADYEXISTS,
     };
     };
 
 
+    /// \brief Allocate and construct \c RBTree
+    ///
+    /// This static method allocates memory for a new \c RBTree object
+    /// from the given memory segment, constructs the object, and returns
+    /// a pointer to it.
+    ///
+    /// \throw std::bad_alloc Memory allocation fails.
+    ///
+    /// \param mem_sgmt A \c MemorySegment from which memory for the new
+    /// \c RBTree is allocated.
+    static RBTree* create(util::MemorySegment& mem_sgmt,
+                          bool return_empty_node = false)
+    {
+        void* p = mem_sgmt.allocate(sizeof(RBTree<T>));
+        return (new(p) RBTree<T>(return_empty_node));
+    }
+
+    /// \brief Destruct and deallocate \c RBTree
+    ///
+    /// \throw none
+    ///
+    /// \param mem_sgmt The \c MemorySegment that allocated memory for
+    /// \c rbtree.
+    /// \param rbtree A non NULL pointer to a valid \c RBTree 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, RBTree<T>* rbtree) {
+        rbtree->~RBTree<T>();
+        mem_sgmt.deallocate(rbtree, sizeof(RBTree<T>));
+    }
+
+private:
     /// \name Constructor and Destructor
     /// \name Constructor and Destructor
     //@{
     //@{
-    /// The constructor.
+    /// \brief The constructor.
+    ///
+    /// An object of this class is always expected to be created by the
+    /// allocator (\c create()), so the constructor is hidden as private.
     ///
     ///
     /// It never throws an exception.
     /// It never throws an exception.
     explicit RBTree(bool returnEmptyNode = false);
     explicit RBTree(bool returnEmptyNode = false);
 
 
-    /// \b Note: RBTree is not intended to be inherited so the destructor
+    /// \brief The destructor.
+    ///
+    /// An object of this class is always expected to be destroyed explicitly
+    /// by \c destroy(), so the constructor is hidden as private.
+    ///
+    /// \note RBTree is not intended to be inherited so the destructor
     /// is not virtual
     /// is not virtual
     ~RBTree();
     ~RBTree();
     //@}
     //@}
 
 
+public:
+
     /// \name Find methods
     /// \name Find methods
     ///
     ///
     /// \brief Find the node that gives a longest match against the given name.
     /// \brief Find the node that gives a longest match against the given name.

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

@@ -60,6 +60,7 @@ run_unittests_SOURCES += sqlite3_unittest.cc
 run_unittests_SOURCES += sqlite3_accessor_unittest.cc
 run_unittests_SOURCES += sqlite3_accessor_unittest.cc
 run_unittests_SOURCES += memory_datasrc_unittest.cc
 run_unittests_SOURCES += memory_datasrc_unittest.cc
 run_unittests_SOURCES += rbnode_rrset_unittest.cc
 run_unittests_SOURCES += rbnode_rrset_unittest.cc
+run_unittests_SOURCES += zonetable_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc
 run_unittests_SOURCES += client_list_unittest.cc

+ 42 - 9
src/lib/datasrc/tests/rbtree_unittest.cc

@@ -16,6 +16,8 @@
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
+#include <util/memory_segment_local.h>
+
 #include <dns/name.h>
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrclass.h>
 #include <dns/rrset.h>
 #include <dns/rrset.h>
@@ -54,9 +56,30 @@ const size_t Name::MAX_LABELS;
  */
  */
 
 
 namespace {
 namespace {
+class TreeHolder {
+public:
+    TreeHolder(util::MemorySegment& mem_sgmt, RBTree<int>* tree) :
+        mem_sgmt_(mem_sgmt), tree_(tree)
+    {}
+    ~TreeHolder() {
+        RBTree<int>::destroy(mem_sgmt_, tree_);
+    }
+    RBTree<int>* get() { return (tree_); }
+private:
+    util::MemorySegment& mem_sgmt_;
+    RBTree<int>* tree_;
+};
+
 class RBTreeTest : public::testing::Test {
 class RBTreeTest : public::testing::Test {
 protected:
 protected:
-    RBTreeTest() : rbtree_expose_empty_node(true), crbtnode(NULL) {
+    RBTreeTest() :
+        rbtree_holder_(mem_sgmt_, RBTree<int>::create(mem_sgmt_)),
+        rbtree_expose_empty_node_holder_(mem_sgmt_,
+                                         RBTree<int>::create(mem_sgmt_, true)),
+        rbtree(*rbtree_holder_.get()),
+        rbtree_expose_empty_node(*rbtree_expose_empty_node_holder_.get()),
+        crbtnode(NULL)
+    {
         const char* const domain_names[] = {
         const char* const domain_names[] = {
             "c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h", "o.w.y.d.e.f",
             "c", "b", "a", "x.d.e.f", "z.d.e.f", "g.h", "i.g.h", "o.w.y.d.e.f",
             "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f", "k.g.h"};
             "j.z.d.e.f", "p.w.y.d.e.f", "q.w.y.d.e.f", "k.g.h"};
@@ -71,8 +94,11 @@ protected:
         }
         }
     }
     }
 
 
-    RBTree<int> rbtree;
-    RBTree<int> rbtree_expose_empty_node;
+    util::MemorySegmentLocal mem_sgmt_;
+    TreeHolder rbtree_holder_;
+    TreeHolder rbtree_expose_empty_node_holder_;
+    RBTree<int>& rbtree;
+    RBTree<int>& rbtree_expose_empty_node;
     RBNode<int>* rbtnode;
     RBNode<int>* rbtnode;
     const RBNode<int>* crbtnode;
     const RBNode<int>* crbtnode;
 };
 };
@@ -275,7 +301,8 @@ TEST_F(RBTreeTest, chainLevel) {
 
 
     // insert one node to the tree and find it.  there should be exactly
     // insert one node to the tree and find it.  there should be exactly
     // one level in the chain.
     // one level in the chain.
-    RBTree<int> tree(true);
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_, true));
+    RBTree<int>& tree(*tree_holder.get());
     Name node_name(Name::ROOT_NAME());
     Name node_name(Name::ROOT_NAME());
     EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(node_name, &rbtnode));
     EXPECT_EQ(RBTree<int>::SUCCESS, tree.insert(node_name, &rbtnode));
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
@@ -542,7 +569,8 @@ TEST_F(RBTreeTest, previousNode) {
     {
     {
         SCOPED_TRACE("Lookup in empty tree");
         SCOPED_TRACE("Lookup in empty tree");
         // Just check it doesn't crash, etc.
         // Just check it doesn't crash, etc.
-        RBTree<int> empty_tree;
+        TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+        RBTree<int>& empty_tree(*tree_holder.get());
         EXPECT_EQ(RBTree<int>::NOTFOUND,
         EXPECT_EQ(RBTree<int>::NOTFOUND,
                   empty_tree.find(Name("x"), &node, node_path));
                   empty_tree.find(Name("x"), &node, node_path));
         EXPECT_EQ(static_cast<void*>(NULL), node);
         EXPECT_EQ(static_cast<void*>(NULL), node);
@@ -586,7 +614,8 @@ TEST_F(RBTreeTest, getLastComparedNode) {
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
 
 
     // A search for an empty tree should result in no 'last compared', too.
     // A search for an empty tree should result in no 'last compared', too.
-    RBTree<int> empty_tree;
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+    RBTree<int>& empty_tree(*tree_holder.get());
     EXPECT_EQ(RBTree<int>::NOTFOUND,
     EXPECT_EQ(RBTree<int>::NOTFOUND,
               empty_tree.find(Name("a"), &crbtnode, chain));
               empty_tree.find(Name("a"), &crbtnode, chain));
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
     EXPECT_EQ(static_cast<void*>(NULL), chain.getLastComparedNode());
@@ -731,7 +760,8 @@ TEST_F(RBTreeTest, swap) {
     size_t count1(rbtree.getNodeCount());
     size_t count1(rbtree.getNodeCount());
 
 
     // Create second one and store state
     // Create second one and store state
-    RBTree<int> tree2;
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+    RBTree<int>& tree2(*tree_holder.get());
     RBNode<int>* node;
     RBNode<int>* node;
     tree2.insert(Name("second"), &node);
     tree2.insert(Name("second"), &node);
     std::ostringstream str2;
     std::ostringstream str2;
@@ -757,7 +787,8 @@ TEST_F(RBTreeTest, swap) {
 // any domain names should be considered a subdomain of it), so it makes
 // any domain names should be considered a subdomain of it), so it makes
 // sense to test cases with the root zone explicitly.
 // sense to test cases with the root zone explicitly.
 TEST_F(RBTreeTest, root) {
 TEST_F(RBTreeTest, root) {
-    RBTree<int> root;
+    TreeHolder tree_holder(mem_sgmt_, RBTree<int>::create(mem_sgmt_));
+    RBTree<int>& root(*tree_holder.get());
     root.insert(Name::ROOT_NAME(), &rbtnode);
     root.insert(Name::ROOT_NAME(), &rbtnode);
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
     rbtnode->setData(RBNode<int>::NodeDataPtr(new int(1)));
 
 
@@ -778,7 +809,9 @@ TEST_F(RBTreeTest, root) {
 
 
     // Perform the same tests for the tree that allows matching against empty
     // Perform the same tests for the tree that allows matching against empty
     // nodes.
     // nodes.
-    RBTree<int> root_emptyok(true);
+    TreeHolder tree_holder_emptyok(mem_sgmt_,
+                                   RBTree<int>::create(mem_sgmt_, true));
+    RBTree<int>& root_emptyok(*tree_holder_emptyok.get());
     root_emptyok.insert(Name::ROOT_NAME(), &rbtnode);
     root_emptyok.insert(Name::ROOT_NAME(), &rbtnode);
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
     EXPECT_EQ(RBTree<int>::EXACTMATCH,
               root_emptyok.find(Name::ROOT_NAME(), &crbtnode));
               root_emptyok.find(Name::ROOT_NAME(), &crbtnode));

+ 34 - 26
src/lib/datasrc/tests/zonetable_unittest.cc

@@ -14,6 +14,8 @@
 
 
 #include <exceptions/exceptions.h>
 #include <exceptions/exceptions.h>
 
 
+#include <util/memory_segment_local.h>
+
 #include <dns/name.h>
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrclass.h>
 
 
@@ -40,7 +42,7 @@ TEST(ZoneTest, init) {
 TEST(ZoneTest, find) {
 TEST(ZoneTest, find) {
     InMemoryZoneFinder zone(RRClass::IN(), Name("example.com"));
     InMemoryZoneFinder zone(RRClass::IN(), Name("example.com"));
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
-              zone.find(Name("www.example.com"), RRType::A()).code);
+              zone.find(Name("www.example.com"), RRType::A())->code);
 }
 }
 
 
 class ZoneTableTest : public ::testing::Test {
 class ZoneTableTest : public ::testing::Test {
@@ -50,68 +52,74 @@ protected:
                       zone2(new InMemoryZoneFinder(RRClass::IN(),
                       zone2(new InMemoryZoneFinder(RRClass::IN(),
                                                    Name("example.net"))),
                                                    Name("example.net"))),
                       zone3(new InMemoryZoneFinder(RRClass::IN(),
                       zone3(new InMemoryZoneFinder(RRClass::IN(),
-                                                   Name("example")))
+                                                   Name("example"))),
+                      zone_table(ZoneTable::create(local_mem_sgmt_))
     {}
     {}
-    ZoneTable zone_table;
+
+    ~ZoneTableTest() {
+        ZoneTable::destroy(local_mem_sgmt_, zone_table);
+    }
     ZoneFinderPtr zone1, zone2, zone3;
     ZoneFinderPtr zone1, zone2, zone3;
+    isc::util::MemorySegmentLocal local_mem_sgmt_;
+    ZoneTable* zone_table;
 };
 };
 
 
 TEST_F(ZoneTableTest, addZone) {
 TEST_F(ZoneTableTest, addZone) {
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
-    EXPECT_EQ(result::EXIST, zone_table.addZone(zone1));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone1));
+    EXPECT_EQ(result::EXIST, zone_table->addZone(zone1));
     // names are compared in a case insensitive manner.
     // names are compared in a case insensitive manner.
-    EXPECT_EQ(result::EXIST, zone_table.addZone(
+    EXPECT_EQ(result::EXIST, zone_table->addZone(
                   ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
                   ZoneFinderPtr(new InMemoryZoneFinder(RRClass::IN(),
                                                        Name("EXAMPLE.COM")))));
                                                        Name("EXAMPLE.COM")))));
 
 
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone2));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone3));
 
 
     // Zone table is indexed only by name.  Duplicate origin name with
     // Zone table is indexed only by name.  Duplicate origin name with
     // different zone class isn't allowed.
     // different zone class isn't allowed.
-    EXPECT_EQ(result::EXIST, zone_table.addZone(
+    EXPECT_EQ(result::EXIST, zone_table->addZone(
                   ZoneFinderPtr(new InMemoryZoneFinder(RRClass::CH(),
                   ZoneFinderPtr(new InMemoryZoneFinder(RRClass::CH(),
                                                        Name("example.com")))));
                                                        Name("example.com")))));
 
 
     /// Bogus zone (NULL)
     /// Bogus zone (NULL)
-    EXPECT_THROW(zone_table.addZone(ZoneFinderPtr()), isc::InvalidParameter);
+    EXPECT_THROW(zone_table->addZone(ZoneFinderPtr()), isc::InvalidParameter);
 }
 }
 
 
 TEST_F(ZoneTableTest, DISABLED_removeZone) {
 TEST_F(ZoneTableTest, DISABLED_removeZone) {
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone1));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone2));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone3));
 
 
-    EXPECT_EQ(result::SUCCESS, zone_table.removeZone(Name("example.net")));
-    EXPECT_EQ(result::NOTFOUND, zone_table.removeZone(Name("example.net")));
+    EXPECT_EQ(result::SUCCESS, zone_table->removeZone(Name("example.net")));
+    EXPECT_EQ(result::NOTFOUND, zone_table->removeZone(Name("example.net")));
 }
 }
 
 
 TEST_F(ZoneTableTest, findZone) {
 TEST_F(ZoneTableTest, findZone) {
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone1));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone2));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone3));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone1));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone2));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone3));
 
 
-    EXPECT_EQ(result::SUCCESS, zone_table.findZone(Name("example.com")).code);
+    EXPECT_EQ(result::SUCCESS, zone_table->findZone(Name("example.com")).code);
     EXPECT_EQ(Name("example.com"),
     EXPECT_EQ(Name("example.com"),
-              zone_table.findZone(Name("example.com")).zone->getOrigin());
+              zone_table->findZone(Name("example.com")).zone->getOrigin());
 
 
     EXPECT_EQ(result::NOTFOUND,
     EXPECT_EQ(result::NOTFOUND,
-              zone_table.findZone(Name("example.org")).code);
+              zone_table->findZone(Name("example.org")).code);
     EXPECT_EQ(ConstZoneFinderPtr(),
     EXPECT_EQ(ConstZoneFinderPtr(),
-              zone_table.findZone(Name("example.org")).zone);
+              zone_table->findZone(Name("example.org")).zone);
 
 
     // there's no exact match.  the result should be the longest match,
     // there's no exact match.  the result should be the longest match,
     // and the code should be PARTIALMATCH.
     // and the code should be PARTIALMATCH.
     EXPECT_EQ(result::PARTIALMATCH,
     EXPECT_EQ(result::PARTIALMATCH,
-              zone_table.findZone(Name("www.example.com")).code);
+              zone_table->findZone(Name("www.example.com")).code);
     EXPECT_EQ(Name("example.com"),
     EXPECT_EQ(Name("example.com"),
-              zone_table.findZone(Name("www.example.com")).zone->getOrigin());
+              zone_table->findZone(Name("www.example.com")).zone->getOrigin());
 
 
     // make sure the partial match is indeed the longest match by adding
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
     // a zone with a shorter origin and query again.
     ZoneFinderPtr zone_com(new InMemoryZoneFinder(RRClass::IN(), Name("com")));
     ZoneFinderPtr zone_com(new InMemoryZoneFinder(RRClass::IN(), Name("com")));
-    EXPECT_EQ(result::SUCCESS, zone_table.addZone(zone_com));
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(zone_com));
     EXPECT_EQ(Name("example.com"),
     EXPECT_EQ(Name("example.com"),
-              zone_table.findZone(Name("www.example.com")).zone->getOrigin());
+              zone_table->findZone(Name("www.example.com")).zone->getOrigin());
 }
 }
 }
 }

+ 36 - 5
src/lib/datasrc/zonetable.cc

@@ -12,13 +12,15 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 // PERFORMANCE OF THIS SOFTWARE.
 
 
-#include <cassert>
+#include <util/memory_segment.h>
 
 
 #include <dns/name.h>
 #include <dns/name.h>
 
 
 #include <datasrc/zonetable.h>
 #include <datasrc/zonetable.h>
 #include <datasrc/rbtree.h>
 #include <datasrc/rbtree.h>
 
 
+#include <cassert>
+
 using namespace std;
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns;
 
 
@@ -30,8 +32,14 @@ struct ZoneTable::ZoneTableImpl {
     // Type aliases to make it shorter
     // Type aliases to make it shorter
     typedef RBTree<ZoneFinder> ZoneTree;
     typedef RBTree<ZoneFinder> ZoneTree;
     typedef RBNode<ZoneFinder> ZoneNode;
     typedef RBNode<ZoneFinder> ZoneNode;
+
     // The actual storage
     // The actual storage
-    ZoneTree zones_;
+    ZoneTree* zones_;
+
+    // Constructor
+    ZoneTableImpl(util::MemorySegment& mem_sgmt) :
+        zones_(ZoneTree::create(mem_sgmt))
+    {}
 
 
     /*
     /*
      * The implementation methods are here and just wrap-called in the
      * The implementation methods are here and just wrap-called in the
@@ -49,7 +57,7 @@ struct ZoneTable::ZoneTableImpl {
 
 
         // Get the node where we put the zone
         // Get the node where we put the zone
         ZoneNode* node(NULL);
         ZoneNode* node(NULL);
-        switch (zones_.insert(zone->getOrigin(), &node)) {
+        switch (zones_->insert(zone->getOrigin(), &node)) {
             // This is OK
             // This is OK
             case ZoneTree::SUCCESS:
             case ZoneTree::SUCCESS:
             case ZoneTree::ALREADYEXISTS:
             case ZoneTree::ALREADYEXISTS:
@@ -76,7 +84,7 @@ struct ZoneTable::ZoneTableImpl {
         result::Result my_result;
         result::Result my_result;
 
 
         // Translate the return codes
         // Translate the return codes
-        switch (zones_.find(name, &node)) {
+        switch (zones_->find(name, &node)) {
             case ZoneTree::EXACTMATCH:
             case ZoneTree::EXACTMATCH:
                 my_result = result::SUCCESS;
                 my_result = result::SUCCESS;
                 break;
                 break;
@@ -100,13 +108,36 @@ struct ZoneTable::ZoneTableImpl {
     }
     }
 };
 };
 
 
-ZoneTable::ZoneTable() : impl_(new ZoneTableImpl)
+ZoneTable::ZoneTable(util::MemorySegment& mem_sgmt) :
+    impl_(new ZoneTableImpl(mem_sgmt))
 {}
 {}
 
 
 ZoneTable::~ZoneTable() {
 ZoneTable::~ZoneTable() {
     delete impl_;
     delete impl_;
 }
 }
 
 
+ZoneTable*
+ZoneTable::create(util::MemorySegment& mem_sgmt) {
+    // The ZoneTable constructor can throw, so we need to prevent memory leak.
+    // This is ugly, but for now this seems to be the only place we need
+    // this, and since we'll substantially revise this code soon, so we don't
+    // work around it by this hack at the moment.
+    void* p = mem_sgmt.allocate(sizeof(ZoneTable));
+    try {
+        return (new(p) ZoneTable(mem_sgmt));
+    } catch (...) {
+        mem_sgmt.deallocate(p, sizeof(ZoneTable));
+        throw;
+    }
+}
+
+void
+ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable) {
+    ZoneTableImpl::ZoneTree::destroy(mem_sgmt, ztable->impl_->zones_);
+    ztable->~ZoneTable();
+    mem_sgmt.deallocate(ztable, sizeof(ZoneTable));
+}
+
 result::Result
 result::Result
 ZoneTable::addZone(ZoneFinderPtr zone) {
 ZoneTable::addZone(ZoneFinderPtr zone) {
     return (impl_->addZone(zone));
     return (impl_->addZone(zone));

+ 35 - 4
src/lib/datasrc/zonetable.h

@@ -15,12 +15,14 @@
 #ifndef __ZONETABLE_H
 #ifndef __ZONETABLE_H
 #define __ZONETABLE_H 1
 #define __ZONETABLE_H 1
 
 
-#include <boost/shared_ptr.hpp>
+#include <util/memory_segment.h>
 
 
 #include <dns/rrset.h>
 #include <dns/rrset.h>
 
 
 #include <datasrc/zone.h>
 #include <datasrc/zone.h>
 
 
+#include <boost/shared_ptr.hpp>
+
 namespace isc {
 namespace isc {
 namespace dns {
 namespace dns {
 class Name;
 class Name;
@@ -58,18 +60,47 @@ private:
     ZoneTable(const ZoneTable& source);
     ZoneTable(const ZoneTable& source);
     ZoneTable& operator=(const ZoneTable& source);
     ZoneTable& operator=(const ZoneTable& source);
 
 
-public:
-    /// Default constructor.
+    /// Constructor.
+    ///
+    /// An object of this class is always expected to be created by the
+    /// allocator (\c create()), so the constructor is hidden as private.
     ///
     ///
     /// This constructor internally involves resource allocation, and if
     /// This constructor internally involves resource allocation, and if
     /// it fails, a corresponding standard exception will be thrown.
     /// it fails, a corresponding standard exception will be thrown.
     /// It never throws an exception otherwise.
     /// It never throws an exception otherwise.
-    ZoneTable();
+    ZoneTable(util::MemorySegment& mem_sgmt);
 
 
     /// The destructor.
     /// The destructor.
+    ///
+    /// An object of this class is always expected to be destroyed explicitly
+    /// by \c destroy(), so the constructor is hidden as private.
     ~ZoneTable();
     ~ZoneTable();
     //@}
     //@}
 
 
+public:
+    /// \brief Allocate and construct \c ZoneTable
+    ///
+    /// This static method allocates memory for a new \c ZoneTable object
+    /// from the given memory segment, constructs the object, and returns
+    /// a pointer to it.
+    ///
+    /// \throw std::bad_alloc Memory allocation fails.
+    ///
+    /// \param mem_sgmt A \c MemorySegment from which memory for the new
+    /// \c ZoneTable is allocated.
+    static ZoneTable* create(util::MemorySegment& mem_sgmt);
+
+    /// \brief Destruct and deallocate \c ZoneTable
+    ///
+    /// \throw none
+    ///
+    /// \param mem_sgmt The \c MemorySegment that allocated memory for
+    /// \c ztable.
+    /// \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);
+
     /// Add a \c Zone to the \c ZoneTable.
     /// Add a \c Zone to the \c ZoneTable.
     ///
     ///
     /// \c Zone must not be associated with a NULL pointer; otherwise
     /// \c Zone must not be associated with a NULL pointer; otherwise