From: JINMEI Tatuya Date: Wed, 22 Aug 2012 00:52:42 +0000 (-0700) Subject: [2107] make ZoneData::create() exception safe. X-Git-Tag: trac2351_base~109^2~1^2~31 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=af5cb72b4dccefd265437fd7e5b8fc9cb9ad1c1b;p=thirdparty%2Fkea.git [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. --- diff --git a/src/lib/datasrc/memory/segment_object_holder.h b/src/lib/datasrc/memory/segment_object_holder.h new file mode 100644 index 0000000000..384f4ef45f --- /dev/null +++ b/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 + +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 +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: diff --git a/src/lib/datasrc/memory/tests/Makefile.am b/src/lib/datasrc/memory/tests/Makefile.am index 3035253eb7..6cc6de5f82 100644 --- a/src/lib/datasrc/memory/tests/Makefile.am +++ b/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) diff --git a/src/lib/datasrc/memory/tests/memory_segment_test.h b/src/lib/datasrc/memory/tests/memory_segment_test.h new file mode 100644 index 0000000000..78f4182ceb --- /dev/null +++ b/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 + +#include // for size_t +#include // 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: diff --git a/src/lib/datasrc/memory/tests/zone_data_unittest.cc b/src/lib/datasrc/memory/tests/zone_data_unittest.cc index fe36bab452..9a5d19eed1 100644 --- a/src/lib/datasrc/memory/tests/zone_data_unittest.cc +++ b/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 +#include "memory_segment_test.h" -#include +#include #include #include @@ -28,8 +28,11 @@ #include +#include // 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. diff --git a/src/lib/datasrc/memory/zone_data.cc b/src/lib/datasrc/memory/zone_data.cc index c132acb7e2..9bf7406e3f 100644 --- a/src/lib/datasrc/memory/zone_data.cc +++ b/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 +#include #include #include // 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 RdataSetDeleterType; + detail::SegmentObjectHolder 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