BIND 10 trac2850_2, updated. 622d3e7123555d0115149e3f4527dbe5f11da864 [2850] Implement the strong exception guarantee
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri May 3 11:35:33 UTC 2013
The branch, trac2850_2 has been updated
via 622d3e7123555d0115149e3f4527dbe5f11da864 (commit)
from 6bb2e2de9bc899e31c8233c1ef74e8ff79c1bccc (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 622d3e7123555d0115149e3f4527dbe5f11da864
Author: Mukund Sivaraman <muks at isc.org>
Date: Fri May 3 16:04:03 2013 +0530
[2850] Implement the strong exception guarantee
-----------------------------------------------------------------------
Summary of changes:
src/lib/datasrc/memory/zone_table_segment.h | 48 +++---
.../datasrc/memory/zone_table_segment_mapped.cc | 158 +++++++++++++-------
src/lib/datasrc/memory/zone_table_segment_mapped.h | 14 +-
.../memory/zone_table_segment_mapped_unittest.cc | 17 +--
4 files changed, 151 insertions(+), 86 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/zone_table_segment.h b/src/lib/datasrc/memory/zone_table_segment.h
index 756aa49..0c95d8d 100644
--- a/src/lib/datasrc/memory/zone_table_segment.h
+++ b/src/lib/datasrc/memory/zone_table_segment.h
@@ -73,19 +73,6 @@ public:
{}
};
-/// \brief Exception thrown when a \c reset() on a ZoneTableSegment
-/// fails because it was determined that the backing memory store is
-/// corrupted. This is typically an unexpected condition that may arise
-/// in rare cases. When this exception is thrown, there is a strong
-/// guarantee that the previously existing backing memory store was
-/// cleared.
-class BrokenSegment : public ResetFailedAndSegmentCleared {
-public:
- BrokenSegment(const char* file, size_t line, const char* what) :
- ResetFailedAndSegmentCleared(file, line, what)
- {}
-};
-
/// \brief Memory-management independent entry point that contains a
/// pointer to a zone table in memory.
///
@@ -214,6 +201,26 @@ public:
/// \brief Unload the current memory store (if loaded) and load the
/// specified one.
///
+ /// In case opening/loading the new memory store fails for some
+ /// reason, one of the following documented (further below)
+ /// exceptions may be thrown. In case failures occur,
+ /// implementations of this method must strictly provide the
+ /// associated behavior as follows, and in the exception
+ /// documentation below. Code that uses \c ZoneTableSegment would
+ /// depend on such assurances.
+ ///
+ /// In case an existing memory store is in use, and an attempt to
+ /// load a different memory store fails, the existing memory store
+ /// must still be available and the \c ResetFailed exception must be
+ /// thrown. In this case, the segment is still usable.
+ ///
+ /// In case an existing memory store is in use, and an attempt is
+ /// made to reload the same memory store which results in a failure,
+ /// the existing memory store must no longer be available and the
+ /// \c ResetFailedAndSegmentCleared exception must be thrown. In
+ /// this case, the segment is no longer usable without a further
+ /// successful call to \c reset().
+ ///
/// See the \c MemorySegmentOpenMode documentation above for the
/// various modes in which a ZoneTableSegment can be created.
///
@@ -223,10 +230,17 @@ public:
/// argument.
///
/// \throws isc::InvalidParameter if the configuration in \c params
- /// has incorrect syntax.
- /// \throws isc::Unexpected for a variety of cases where an
- /// unexpected condition occurs. These should not occur normally in
- /// correctly written code.
+ /// has incorrect syntax, but the segment is still usable due to the
+ /// old memory store still being in use.
+ ///
+ /// \throw ResetFailed if there was a problem in loading the new
+ /// memory store, but the segment is still usable due to the old
+ /// memory store still being in use.
+ ///
+ /// \throw ResetFailedAndSegmentCleared if there was a problem in
+ /// loading the new memory store, but the old memory store was also
+ /// unloaded and is no longer in use. The segment is not usable
+ /// without a further successful \c reset().
///
/// \param mode The open mode (see the MemorySegmentOpenMode
/// documentation).
diff --git a/src/lib/datasrc/memory/zone_table_segment_mapped.cc b/src/lib/datasrc/memory/zone_table_segment_mapped.cc
index 3ffcb62..c80879e 100644
--- a/src/lib/datasrc/memory/zone_table_segment_mapped.cc
+++ b/src/lib/datasrc/memory/zone_table_segment_mapped.cc
@@ -43,16 +43,17 @@ ZoneTableSegmentMapped::ZoneTableSegmentMapped(const RRClass& rrclass) :
bool
ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
- bool createMode)
+ bool create,
+ std::string& error_msg)
{
MemorySegment::NamedAddressResult result =
segment.getNamedAddress(ZONE_TABLE_CHECKSUM_NAME);
if (result.first) {
- if (createMode) {
+ if (create) {
// There must be no previously saved checksum.
- isc_throw(isc::Unexpected,
- "There is already a saved checksum in a mapped segment "
- "opened in create mode.");
+ error_msg = "There is already a saved checksum in the segment "
+ "opened in create mode";
+ return (false);
} else {
// The segment was already shrunk when it was last
// closed. Check that its checksum is consistent.
@@ -64,11 +65,10 @@ ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
*checksum = 0;
const uint32_t new_checksum = segment.getCheckSum();
if (saved_checksum != new_checksum) {
- isc_throw(isc::Unexpected,
- "Saved checksum doesn't match mapped segment data");
+ error_msg = "Saved checksum doesn't match segment data";
+ return (false);
}
}
- return (true);
} else {
// Allocate space for a checksum (which is saved during close).
@@ -87,27 +87,34 @@ ZoneTableSegmentMapped::processChecksum(MemorySegmentMapped& segment,
*static_cast<uint32_t*>(checksum) = 0;
const bool grew = segment.setNamedAddress(ZONE_TABLE_CHECKSUM_NAME,
checksum);
- // If the segment grew here, we have a problem as the checksum
- // address may no longer be valid. In this case, we cannot
- // recover. This case is extremely unlikely as we reserved
- // memory for the ZONE_TABLE_CHECKSUM_NAME above. It indicates a
- // very restrictive MemorySegment which we should not use.
- return (!grew);
+ if (grew) {
+ // If the segment grew here, we have a problem as the
+ // checksum address may no longer be valid. In this case, we
+ // cannot recover. This case is extremely unlikely as we
+ // reserved memory for the ZONE_TABLE_CHECKSUM_NAME
+ // above. It indicates a very restrictive MemorySegment
+ // which we should not use.
+ error_msg = "Segment grew unexpectedly in setNamedAddress()";
+ return (false);
+ }
}
+
+ return (true);
}
bool
ZoneTableSegmentMapped::processHeader(MemorySegmentMapped& segment,
- bool createMode)
+ bool create,
+ std::string& error_msg)
{
MemorySegment::NamedAddressResult result =
segment.getNamedAddress(ZONE_TABLE_HEADER_NAME);
if (result.first) {
- if (createMode) {
+ if (create) {
// There must be no previously saved checksum.
- isc_throw(isc::Unexpected,
- "There is already a saved ZoneTableHeader in a "
- "mapped segment opened in create mode.");
+ error_msg = "There is already a saved ZoneTableHeader in the "
+ "segment opened in create mode";
+ return (false);
} else {
assert(result.second);
header_ = static_cast<ZoneTableHeader*>(result.second);
@@ -127,42 +134,57 @@ ZoneTableSegmentMapped::processHeader(MemorySegmentMapped& segment,
const bool grew = segment.setNamedAddress(ZONE_TABLE_HEADER_NAME,
new_header);
if (grew) {
- // If the segment grew here, we have a problem as the table
- // header address may no longer be valid. In this case, we
- // cannot recover. This case is extremely unlikely as we
- // reserved memory for the ZONE_TABLE_HEADER_NAME above. It
- // indicates a very restrictive MemorySegment which we
- // should not use.
- return (false);
+ // If the segment grew here, we have a problem as the table
+ // header address may no longer be valid. In this case, we
+ // cannot recover. This case is extremely unlikely as we
+ // reserved memory for the ZONE_TABLE_HEADER_NAME above. It
+ // indicates a very restrictive MemorySegment which we
+ // should not use.
+ error_msg = "Segment grew unexpectedly in setNamedAddress()";
+ return (false);
}
header_ = new_header;
}
- return (true);
-}
-
-void
-ZoneTableSegmentMapped::openCreate(const std::string& filename) {
- // In case there is a checksum mismatch, we throw. We want the
- // segment to be automatically destroyed then.
- std::auto_ptr<MemorySegmentMapped> segment
- (new MemorySegmentMapped(filename, MemorySegmentMapped::CREATE_ONLY));
- processChecksum(*segment, true);
- processHeader(*segment, true);
-
- mem_sgmt_.reset(segment.release());
+ return (true);
}
void
-ZoneTableSegmentMapped::openReadWrite(const std::string& filename) {
- // In case there is a checksum mismatch, we throw. We want the
- // segment to be automatically destroyed then.
+ZoneTableSegmentMapped::openReadWrite(const std::string& filename,
+ bool create)
+{
+ const MemorySegmentMapped::OpenMode mode = create ?
+ MemorySegmentMapped::CREATE_ONLY :
+ MemorySegmentMapped::OPEN_OR_CREATE;
+ // In case there is a problem, we throw. We want the segment to be
+ // automatically destroyed then.
std::auto_ptr<MemorySegmentMapped> segment
- (new MemorySegmentMapped(filename,
- MemorySegmentMapped::OPEN_OR_CREATE));
+ (new MemorySegmentMapped(filename, mode));
+
+ std::string error_msg;
+ if (!processChecksum(*segment, create, error_msg)) {
+ if (mem_sgmt_) {
+ isc_throw(ResetFailed,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ } else {
+ isc_throw(ResetFailedAndSegmentCleared,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ }
+ }
- processChecksum(*segment, false);
- processHeader(*segment, false);
+ if (!processHeader(*segment, create, error_msg)) {
+ if (mem_sgmt_) {
+ isc_throw(ResetFailed,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ } else {
+ isc_throw(ResetFailedAndSegmentCleared,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ }
+ }
mem_sgmt_.reset(segment.release());
}
@@ -177,9 +199,18 @@ ZoneTableSegmentMapped::openReadOnly(const std::string& filename) {
MemorySegment::NamedAddressResult result =
segment->getNamedAddress(ZONE_TABLE_CHECKSUM_NAME);
if (!result.first) {
- isc_throw(isc::Unexpected,
- "There is no previously saved checksum in a "
- "mapped segment opened in read-only mode.");
+ const std::string error_msg =
+ "There is no previously saved checksum in a "
+ "mapped segment opened in read-only mode";
+ if (mem_sgmt_) {
+ isc_throw(ResetFailed,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ } else {
+ isc_throw(ResetFailedAndSegmentCleared,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ }
}
// We can't verify the checksum here as we can't set the checksum to
@@ -192,9 +223,18 @@ ZoneTableSegmentMapped::openReadOnly(const std::string& filename) {
assert(result.second);
header_ = static_cast<ZoneTableHeader*>(result.second);
} else {
- isc_throw(isc::Unexpected,
- "There is no previously saved ZoneTableHeader in a "
- "mapped segment opened in read-only mode.");
+ const std::string error_msg =
+ "There is no previously saved ZoneTableHeader in a "
+ "mapped segment opened in read-only mode.";
+ if (mem_sgmt_) {
+ isc_throw(ResetFailed,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ } else {
+ isc_throw(ResetFailedAndSegmentCleared,
+ "Error in resetting zone table segment to use "
+ << filename << ": " << error_msg);
+ }
}
mem_sgmt_.reset(segment.release());
@@ -204,8 +244,6 @@ void
ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
isc::data::ConstElementPtr params)
{
- clear();
-
if (!params || params->getType() != Element::map) {
isc_throw(isc::InvalidParameter,
"Configuration does not contain a map");
@@ -224,13 +262,20 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
const std::string filename = mapped_file->stringValue();
+ if (mem_sgmt_ && (filename == current_filename_)) {
+ // This reset() is an attempt to re-open the currently open
+ // mapped file. We cannot do this in many mode combinations
+ // unless we close the existing mapped file. So just close it.
+ clear();
+ }
+
switch (mode) {
case CREATE:
- openCreate(filename);
+ openReadWrite(filename, true);
break;
case READ_WRITE:
- openReadWrite(filename);
+ openReadWrite(filename, false);
break;
case READ_ONLY:
@@ -238,6 +283,7 @@ ZoneTableSegmentMapped::reset(MemorySegmentOpenMode mode,
}
current_mode_ = mode;
+ current_filename_ = filename;
}
void
diff --git a/src/lib/datasrc/memory/zone_table_segment_mapped.h b/src/lib/datasrc/memory/zone_table_segment_mapped.h
index 02e5cb1..2206630 100644
--- a/src/lib/datasrc/memory/zone_table_segment_mapped.h
+++ b/src/lib/datasrc/memory/zone_table_segment_mapped.h
@@ -74,6 +74,10 @@ public:
/// \brief Unmap the current file (if mapped) and map the specified
/// one.
///
+ /// In case of exceptions, the current existing mapped file may be
+ /// left open, or may be cleared. Please see the \c ZoneTableSegment
+ /// API documentation for the behavior.
+ ///
/// See the \c MemorySegmentOpenMode documentation (in
/// \c ZoneTableSegment class) for the various modes in which a
/// \c ZoneTableSegmentMapped can be created.
@@ -101,16 +105,18 @@ public:
virtual void clear();
private:
- bool processChecksum(isc::util::MemorySegmentMapped& segment, bool create);
- bool processHeader(isc::util::MemorySegmentMapped& segment, bool create);
+ bool processChecksum(isc::util::MemorySegmentMapped& segment, bool create,
+ std::string& error_msg);
+ bool processHeader(isc::util::MemorySegmentMapped& segment, bool create,
+ std::string& error_msg);
- void openCreate(const std::string& filename);
- void openReadWrite(const std::string& filename);
+ void openReadWrite(const std::string& filename, bool create);
void openReadOnly(const std::string& filename);
private:
isc::dns::RRClass rrclass_;
MemorySegmentOpenMode current_mode_;
+ std::string current_filename_;
// Internally holds a MemorySegmentMapped. This is NULL on
// construction, and is set by the \c reset() method.
boost::scoped_ptr<isc::util::MemorySegmentMapped> mem_sgmt_;
diff --git a/src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc
index d56b9ef..e13a3e6 100644
--- a/src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_table_segment_mapped_unittest.cc
@@ -151,21 +151,20 @@ TEST_F(ZoneTableSegmentMappedTest, reset) {
config_params_);
EXPECT_TRUE(ztable_segment_->isWritable());
- // When we reset() and it fails, then the segment should be
- // unusable.
+ // When we reset() with an invalid paramter and it fails, then the
+ // segment should still be usable.
EXPECT_THROW({
ztable_segment_->reset(ZoneTableSegment::CREATE,
Element::fromJSON("{}"));
}, isc::InvalidParameter);
- // The following should throw now.
- EXPECT_THROW(ztable_segment_->getHeader(), isc::InvalidOperation);
- EXPECT_THROW(ztable_segment_->getMemorySegment(), isc::InvalidOperation);
-
- // isWritable() must return false, because the last reset() failed.
- EXPECT_FALSE(ztable_segment_->isWritable());
+ EXPECT_TRUE(ztable_segment_->isWritable());
+ // The following should not throw.
+ EXPECT_NO_THROW(ztable_segment_->getHeader());
+ EXPECT_NO_THROW(ztable_segment_->getMemorySegment());
// READ_WRITE with an existing map file ought to work too. This
- // would use existing named addresses.
+ // would use existing named addresses. This actually re-opens the
+ // currently open map.
ztable_segment_->reset(ZoneTableSegment::READ_WRITE,
config_params_);
EXPECT_TRUE(ztable_segment_->isWritable());
More information about the bind10-changes
mailing list