Browse Source

[2107] make ZoneData::create() exception safe.

copied (with some modification) object holder and test memory segment
from #2100.  They should be unified when this branch and #2100 are merged.
JINMEI Tatuya 12 years ago
parent
commit
af5cb72b4d

+ 63 - 0
src/lib/datasrc/memory/segment_object_holder.h

@@ -0,0 +1,63 @@
+// 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_SEGMENT_OBJECT_HOLDER_H
+#define DATASRC_MEMORY_SEGMENT_OBJECT_HOLDER_H 1
+
+#include <util/memory_segment.h>
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+namespace detail {
+
+// A simple holder to create and use some objects in this implementation
+// in an exception safe manner.   It works like std::auto_ptr but much
+// more simplified.
+// template parameter T is the type of object allocated by mem_sgmt.
+// template parameter ARG_T is the type that will be passed to destroy()
+// (deleter functor, etc).  It must be copyable.
+template <typename T, typename ARG_T>
+class SegmentObjectHolder {
+public:
+    SegmentObjectHolder(util::MemorySegment& mem_sgmt, T* obj, ARG_T arg) :
+        mem_sgmt_(mem_sgmt), obj_(obj), arg_(arg)
+    {}
+    ~SegmentObjectHolder() {
+        if (obj_ != NULL) {
+            T::destroy(mem_sgmt_, obj_, arg_);
+        }
+    }
+    T* get() { return (obj_); }
+    T* release() {
+        T* ret = obj_;
+        obj_ = NULL;
+        return (ret);
+    }
+private:
+    util::MemorySegment& mem_sgmt_;
+    T* obj_;
+    ARG_T arg_;
+};
+
+} // detail
+} // namespace memory
+} // namespace datasrc
+} // namespace isc
+
+#endif // DATASRC_MEMORY_SEGMENT_OBJECT_HOLDER_H
+
+// Local Variables:
+// mode: c++
+// End:

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

@@ -22,6 +22,7 @@ run_unittests_SOURCES += rdata_serialization_unittest.cc
 run_unittests_SOURCES += rdataset_unittest.cc
 run_unittests_SOURCES += domaintree_unittest.cc
 run_unittests_SOURCES += zone_data_unittest.cc
+run_unittests_SOURCES += memory_segment_test.h
 
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 run_unittests_LDFLAGS  = $(AM_LDFLAGS)  $(GTEST_LDFLAGS)

+ 58 - 0
src/lib/datasrc/memory/tests/memory_segment_test.h

@@ -0,0 +1,58 @@
+// 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_SEGMENT_TEST_H
+#define DATASRC_MEMORY_SEGMENT_TEST_H 1
+
+#include <util/memory_segment_local.h>
+
+#include <cstddef>              // for size_t
+#include <new>                  // for bad_alloc
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+namespace test {
+
+// A special memory segment that can be used for tests.  It normally behaves
+// like a "local" memory segment.  If "throw count" is set to non 0 via
+// setThrowCount(), it continues the normal behavior up to the specified
+// number of calls to allocate(), and throws an exception at the next call.
+class MemorySegmentTest : public isc::util::MemorySegmentLocal {
+public:
+    MemorySegmentTest() : throw_count_(0) {}
+    virtual void* allocate(std::size_t size) {
+        if (throw_count_ > 0) {
+            if (--throw_count_ == 0) {
+                throw std::bad_alloc();
+            }
+        }
+        return (isc::util::MemorySegmentLocal::allocate(size));
+    }
+    void setThrowCount(std::size_t count) { throw_count_ = count; }
+
+private:
+    std::size_t throw_count_;
+};
+
+} // namespace test
+} // namespace memory
+} // namespace datasrc
+} // namespace isc
+
+#endif // DATASRC_MEMORY_SEGMENT_TEST_H
+
+// Local Variables:
+// mode: c++
+// End:

+ 22 - 3
src/lib/datasrc/memory/tests/zone_data_unittest.cc

@@ -12,9 +12,9 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <exceptions/exceptions.h>
+#include "memory_segment_test.h"
 
-#include <util/memory_segment_local.h>
+#include <exceptions/exceptions.h>
 
 #include <dns/name.h>
 #include <dns/labelsequence.h>
@@ -28,8 +28,11 @@
 
 #include <gtest/gtest.h>
 
+#include <new>                  // for bad_alloc
+
 using namespace isc::dns;
 using namespace isc::datasrc::memory;
+using namespace isc::datasrc::memory::test;
 using namespace isc::testutils;
 
 namespace {
@@ -47,7 +50,7 @@ protected:
         EXPECT_TRUE(mem_sgmt_.allMemoryDeallocated());
     }
 
-    isc::util::MemorySegmentLocal mem_sgmt_;
+    MemorySegmentTest mem_sgmt_;
     const Name zname_;
     ZoneData* zone_data_;
     RdataEncoder encoder_;
@@ -57,6 +60,22 @@ TEST_F(ZoneDataTest, getOriginNode) {
     EXPECT_EQ(LabelSequence(zname_), zone_data_->getOriginNode()->getLabels());
 }
 
+TEST_F(ZoneDataTest, throwOnCreate) {
+    // Note: below, we use our knowledge of how memory allocation happens
+    // within the zone data and the underlying domain tree implementation.
+
+    // allocate() will throw on the insertion of the origin node.
+    mem_sgmt_.setThrowCount(2);
+    EXPECT_THROW(ZoneData::create(mem_sgmt_, zname_), std::bad_alloc);
+
+    // allocate() will throw on creating the zone data.
+    mem_sgmt_.setThrowCount(3);
+    EXPECT_THROW(ZoneData::create(mem_sgmt_, zname_), std::bad_alloc);
+
+    // These incomplete create() attempts shouldn't cause memory leak
+    // (that would be caught in TearDown()).
+}
+
 TEST_F(ZoneDataTest, addRdataSets) {
     // Insert a name to the zone, and add a couple the data (RdataSet) objects
     // to the corresponding node.

+ 29 - 13
src/lib/datasrc/memory/zone_data.cc

@@ -20,8 +20,10 @@
 #include "rdataset.h"
 #include "rdata_encoder.h"
 #include "zone_data.h"
+#include "segment_object_holder.h"
 
 #include <boost/bind.hpp>
+#include <boost/function.hpp>
 
 #include <cassert>
 #include <new>                  // for the placement new
@@ -32,19 +34,6 @@ namespace isc {
 namespace datasrc {
 namespace memory {
 
-ZoneData*
-ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) {
-    ZoneTree* tree = ZoneTree::create(mem_sgmt, true);
-    ZoneNode* origin_node = NULL;
-    const ZoneTree::Result result =
-        tree->insert(mem_sgmt, zone_origin, &origin_node);
-    assert(result == ZoneTree::SUCCESS);
-    void* p = mem_sgmt.allocate(sizeof(ZoneData));
-    ZoneData* zone_data = new(p) ZoneData(tree, origin_node);
-
-    return (zone_data);
-}
-
 namespace {
 void
 rdataSetDeleter(RRClass rrclass, util::MemorySegment* mem_sgmt,
@@ -56,6 +45,33 @@ rdataSetDeleter(RRClass rrclass, util::MemorySegment* mem_sgmt,
         RdataSet::destroy(*mem_sgmt, rrclass, rdataset);
     }
 }
+
+void
+nullDeleter(RdataSet* rdataset_head) {
+    assert(rdataset_head == NULL);
+}
+}
+
+ZoneData*
+ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) {
+    // ZoneTree::insert() and ZoneData allocation can throw.  To avoid
+    // leaking the tree, we manage it in the holder.
+    // Note: we won't add any RdataSet, so we use the NO-OP deleter
+    // (with an assertion check for that).
+    typedef boost::function<void(RdataSet*)> RdataSetDeleterType;
+    detail::SegmentObjectHolder<ZoneTree, RdataSetDeleterType> holder(
+        mem_sgmt, ZoneTree::create(mem_sgmt, true),
+        boost::bind(nullDeleter, _1));
+
+    ZoneTree* tree = holder.get();
+    ZoneNode* origin_node = NULL;
+    const ZoneTree::Result result =
+        tree->insert(mem_sgmt, zone_origin, &origin_node);
+    assert(result == ZoneTree::SUCCESS);
+    void* p = mem_sgmt.allocate(sizeof(ZoneData));
+    ZoneData* zone_data = new(p) ZoneData(holder.release(), origin_node);
+
+    return (zone_data);
 }
 
 void