BIND 10 trac2831, updated. a2fcb467b96cb4229e35330a9ca72db8065953bb [2831] added a test (compile time in fact) for def of INITIAL_SIZE.
BIND 10 source code commits
bind10-changes at lists.isc.org
Sat Apr 13 07:01:26 UTC 2013
The branch, trac2831 has been updated
via a2fcb467b96cb4229e35330a9ca72db8065953bb (commit)
via d70fad8d81f6fb7a4dcadcad45043c76e85f58af (commit)
via 329d5b2238df1813bde496a44a31c0e57dbbac00 (commit)
via 0f6cfdfd2a611d2a440fcb01596c4f6173e9ee04 (commit)
via 3877992c69722447eeb9e4819bdadc64fb44754c (commit)
via 059b6a3e1f0c3c9a5d4526664d272f16bb664903 (commit)
via 510d6d2e9f16f9bf1dfff251d84e311b685fd454 (commit)
via e41c43c1cc533686b04734fa35377bbaebf356ea (commit)
via e3b63fb33c64d7cafbc59895c803b5ed7ddeb82a (commit)
via 8297b3a7425d2a0de1f1f0f5cb1a4572a1780820 (commit)
via 6eae3579a0dc0ed27f33c3e36cec2ad64be7329d (commit)
from d43543b1fea006c3ac4bca51749e7cffbcc84085 (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 a2fcb467b96cb4229e35330a9ca72db8065953bb
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 23:50:19 2013 -0700
[2831] added a test (compile time in fact) for def of INITIAL_SIZE.
commit d70fad8d81f6fb7a4dcadcad45043c76e85f58af
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 23:32:02 2013 -0700
[2831] make sure resetting a segment before opening one
commit 329d5b2238df1813bde496a44a31c0e57dbbac00
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 23:31:29 2013 -0700
[2831] more doc about reader-writer conflicts
commit 0f6cfdfd2a611d2a440fcb01596c4f6173e9ee04
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 23:05:24 2013 -0700
[2831] use boost file lock to detect any violation of read-write conflicts
this is done best-effort basis. update doc accordingly.
commit 3877992c69722447eeb9e4819bdadc64fb44754c
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:52:33 2013 -0700
[2831] clarified the check after repeated shrinkToFit.
commit 059b6a3e1f0c3c9a5d4526664d272f16bb664903
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:38:23 2013 -0700
[2831] corrected assert condition for overflow check
commit 510d6d2e9f16f9bf1dfff251d84e311b685fd454
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:31:24 2013 -0700
[2831] corrected a typo
commit e41c43c1cc533686b04734fa35377bbaebf356ea
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:31:01 2013 -0700
[2831] more clarified when mapped setNamedAddress returns true.
commit e3b63fb33c64d7cafbc59895c803b5ed7ddeb82a
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:22:07 2013 -0700
[2831] in multiProcess test make sure two processes open segments in parallel.
commit 8297b3a7425d2a0de1f1f0f5cb1a4572a1780820
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:13:22 2013 -0700
[2831] reverted the change for deleting checkNamedData().
the order of deletion was intentional; BOOST_FOREACH for map wouldn't work
that way.
commit 6eae3579a0dc0ed27f33c3e36cec2ad64be7329d
Author: JINMEI Tatuya <jinmei at isc.org>
Date: Fri Apr 12 16:09:21 2013 -0700
[2831] removed test for segment size after shrink.
I don't think the underlying boost shrink_to_fit() ensures this.
-----------------------------------------------------------------------
Summary of changes:
src/lib/util/memory_segment_mapped.cc | 106 ++++++--
src/lib/util/memory_segment_mapped.h | 39 ++-
.../util/tests/memory_segment_mapped_unittest.cc | 253 +++++++++++++++-----
3 files changed, 310 insertions(+), 88 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/util/memory_segment_mapped.cc b/src/lib/util/memory_segment_mapped.cc
index a3d94e7..e2ac944 100644
--- a/src/lib/util/memory_segment_mapped.cc
+++ b/src/lib/util/memory_segment_mapped.cc
@@ -22,12 +22,25 @@
#include <boost/interprocess/managed_mapped_file.hpp>
#include <boost/interprocess/offset_ptr.hpp>
#include <boost/interprocess/mapped_region.hpp>
+#include <boost/interprocess/sync/file_lock.hpp>
#include <cassert>
#include <string>
#include <new>
-using namespace boost::interprocess;
+// boost::interprocess namespace is big and can cause unexpected import
+// (e.g., it has "read_only"), so it's safer to be specific for shortcuts.
+using boost::interprocess::basic_managed_mapped_file;
+using boost::interprocess::rbtree_best_fit;
+using boost::interprocess::null_mutex_family;
+using boost::interprocess::iset_index;
+using boost::interprocess::create_only_t;
+using boost::interprocess::create_only;
+using boost::interprocess::open_or_create_t;
+using boost::interprocess::open_or_create;
+using boost::interprocess::open_read_only;
+using boost::interprocess::open_only;
+using boost::interprocess::offset_ptr;
namespace isc {
namespace util {
@@ -47,27 +60,70 @@ typedef basic_managed_mapped_file<char,
iset_index> BaseSegment;
struct MemorySegmentMapped::Impl {
- // Constructor for create-only (and read-write) mode
+ // Constructor for create-only (and read-write) mode. this case is
+ // tricky because we want to remove any existing file but we also want
+ // to detect possible conflict with other readers or writers using
+ // file lock.
Impl(const std::string& filename, create_only_t, size_t initial_size) :
- read_only_(false), filename_(filename),
- base_sgmt_(new BaseSegment(create_only, filename.c_str(),
- initial_size))
- {}
+ read_only_(false), filename_(filename)
+ {
+ try {
+ // First, try opening it in boost create_only mode; it fails if
+ // the file exists (among other reasons).
+ base_sgmt_.reset(new BaseSegment(create_only, filename.c_str(),
+ initial_size));
+ } catch (const boost::interprocess::interprocess_exception& ex) {
+ // We assume this is because the file exists; otherwise creating
+ // file_lock would fail with interprocess_exception, and that's
+ // what we want here (we wouldn't be able to create a segment
+ // anyway).
+ lock_.reset(new boost::interprocess::file_lock(filename.c_str()));
+
+ // Confirm there's no other reader or writer, and then release
+ // the lock before we remove the file; there's a chance of race
+ // here, but this check doesn't intend to guarantee 100% safety
+ // and so it should be okay.
+ checkWriter();
+ lock_.reset();
+
+ // now remove the file (if it happens to have been delete, this
+ // will be no-op), then re-open it with create_only. this time
+ // it should succeed, and if it fails again, that's fatal for this
+ // constructor.
+ boost::interprocess::file_mapping::remove(filename.c_str());
+ base_sgmt_.reset(new BaseSegment(create_only, filename.c_str(),
+ initial_size));
+ }
+
+ // confirm there's no other user and there won't either.
+ lock_.reset(new boost::interprocess::file_lock(filename.c_str()));
+ checkWriter();
+ }
// Constructor for open-or-write (and read-write) mode
Impl(const std::string& filename, open_or_create_t, size_t initial_size) :
read_only_(false), filename_(filename),
base_sgmt_(new BaseSegment(open_or_create, filename.c_str(),
- initial_size))
- {}
+ initial_size)),
+ lock_(new boost::interprocess::file_lock(filename.c_str()))
+ {
+ checkWriter();
+ }
// Constructor for existing segment, either read-only or read-write
Impl(const std::string& filename, bool read_only) :
read_only_(read_only), filename_(filename),
- base_sgmt_(read_only ?
+ base_sgmt_(read_only_ ?
new BaseSegment(open_read_only, filename.c_str()) :
- new BaseSegment(open_only, filename.c_str()))
- {}
+ new BaseSegment(open_only, filename.c_str())),
+ lock_(new boost::interprocess::file_lock(filename.c_str()))
+ {
+ if (read_only_) {
+ checkReader();
+ } else {
+ checkWriter();
+ }
+ }
// Internal helper to grow the underlying mapped segment.
void growSegment() {
@@ -81,7 +137,7 @@ struct MemorySegmentMapped::Impl {
// behavior. But we basically assume grow() would fail before this
// happens, so we assert it shouldn't happen.
const size_t new_size = prev_size * 2;
- assert(new_size != 0);
+ assert(new_size > prev_size);
if (!BaseSegment::grow(filename_.c_str(), new_size - prev_size)) {
throw std::bad_alloc();
@@ -105,6 +161,30 @@ struct MemorySegmentMapped::Impl {
// actual Boost implementation of mapped segment.
boost::scoped_ptr<BaseSegment> base_sgmt_;
+
+private:
+ // helper methods and member to detect any reader-writer conflict at
+ // the time of construction using an advisory file lock. The lock will
+ // be held throughout the lifetime of the object and will be released
+ // automatically.
+
+ void checkReader() {
+ if (!lock_->try_lock_sharable()) {
+ isc_throw(MemorySegmentOpenError,
+ "mapped memory segment can't be opened as read-only "
+ "with a writer process");
+ }
+ }
+
+ void checkWriter() {
+ if (!lock_->try_lock()) {
+ isc_throw(MemorySegmentOpenError,
+ "mapped memory segment can't be opened as read-write "
+ "with other reader or writer processes");
+ }
+ }
+
+ boost::scoped_ptr<boost::interprocess::file_lock> lock_;
};
MemorySegmentMapped::MemorySegmentMapped(const std::string& filename) :
@@ -132,8 +212,6 @@ MemorySegmentMapped::MemorySegmentMapped(const std::string& filename,
impl_ = new Impl(filename, open_or_create, initial_size);
break;
case CREATE_ONLY:
- // Remove any existing file then create a new one
- file_mapping::remove(filename.c_str());
impl_ = new Impl(filename, create_only, initial_size);
break;
default:
diff --git a/src/lib/util/memory_segment_mapped.h b/src/lib/util/memory_segment_mapped.h
index 476974c..fe8a29a 100644
--- a/src/lib/util/memory_segment_mapped.h
+++ b/src/lib/util/memory_segment_mapped.h
@@ -35,13 +35,24 @@ namespace util {
/// In the read-write mode the object can allocate or deallocate memory
/// from the mapped image, and the owner process can change the content.
///
-/// This class does not provide any synchronization mechanism between
-/// read-only and read-write objects that share the same mapped image;
-/// in fact, the expected usage is the application (or a system of related
-/// processes) ensures there's at most one read-write object and if there's
-/// such an object, no read-only object shares the image. If an application
-/// uses this class beyond that expectation, it's the application's
-/// responsibility to provide necessary synchronization between the processes.
+/// Multiple processes can open multiple segments for the same file in
+/// read-only mode at the same time. But there shouldn't be more than
+/// one process that opens segments for the same file in read-write mode
+/// at the same time. Likewise, if one process opens a segment for a file
+/// there shouldn't be any other process that opens a segment for the file
+/// in read-only mode. This class tries to detect any violation of this
+/// restriction, but this does not intend to provide 100% safety. It's
+/// generally the user's responsibility to ensure this condition.
+///
+/// The same restriction applies within the single process, whether
+/// multi-threaded or not: a process shouldn't open read-only and read-write
+/// (or multiple read-write) segments for the same file. The violation
+/// detection mentioned above may or may not work in such cases due to
+/// limitation of the underlying API. It's completely user's responsibility
+/// to prevent this from happening. A single process may open multiple
+/// segments in read-only mode for the same file, but that shouldn't be
+/// necessary in practice; since it's read-only there wouldn't be a reason
+/// to have a redundant copy.
class MemorySegmentMapped : boost::noncopyable, public MemorySegment {
public:
/// \brief The default value of the mapped file size when newly created.
@@ -74,7 +85,8 @@ public:
/// exception will be thrown.
///
/// \throw MemorySegmentOpenError The given file does not exist, is not
- /// readable, or not valid mappable segment.
+ /// readable, or not valid mappable segment. Or there is another process
+ /// that has already opened a segment for the file.
/// \throw std::bad_alloc (rare case) internal resource allocation
/// failure.
///
@@ -106,6 +118,10 @@ public:
/// create or initialization fails, \c MemorySegmentOpenError exception
/// will be thrown.
///
+ /// This constructor also throws \c MemorySegmentOpenError when it
+ /// detects violation of the restriction on the mixed open of read-only
+ /// and read-write mode (see the class description).
+ ///
/// When initial_size is specified but is too small (including a value of
/// 0), the underlying Boost library will reject it, and this constructor
/// throws \c MemorySegmentOpenError exception. The Boost documentation
@@ -163,8 +179,11 @@ public:
/// This implementation detects if \c addr is invalid (see the base class
/// description) and throws \c MemorySegmentError in that case.
///
- /// This version can actually return true (see also \c MemorySegmentLocal
- /// version of the method).
+ /// This version of method should normally return false. However,
+ /// it internally allocates memory in the segment for the name and
+ /// address to be stored, which can require segment extension, just like
+ /// allocate(). So it's possible to return true unlike
+ /// \c MemorySegmentLocal version of the method.
///
/// This method cannot be called if the segment object is created in the
/// read-only mode; in that case MemorySegmentError will be thrown.
diff --git a/src/lib/util/tests/memory_segment_mapped_unittest.cc b/src/lib/util/tests/memory_segment_mapped_unittest.cc
index 18adad8..1d9979d 100644
--- a/src/lib/util/tests/memory_segment_mapped_unittest.cc
+++ b/src/lib/util/tests/memory_segment_mapped_unittest.cc
@@ -56,6 +56,25 @@ const MemorySegmentMapped::OpenMode CREATE_ONLY =
const char* const mapped_file = TEST_DATA_BUILDDIR "/test.mapped";
const size_t DEFAULT_INITIAL_SIZE = 32 * 1024; // intentionally hardcoded
+// A simple RAII-style wrapper for a pipe. Several tests in this file use
+// pipes, so this helper will be useful.
+class PipeHolder {
+public:
+ PipeHolder() {
+ if (pipe(fds_) == -1) {
+ isc_throw(isc::Unexpected, "pipe failed");
+ }
+ }
+ ~PipeHolder() {
+ close(fds_[0]);
+ close(fds_[1]);
+ }
+ int getReadFD() const { return (fds_[0]); }
+ int getWriteFD() const { return (fds_[1]); }
+private:
+ int fds_[2];
+};
+
class MemorySegmentMappedTest : public ::testing::Test {
protected:
MemorySegmentMappedTest() {
@@ -78,6 +97,13 @@ protected:
scoped_ptr<MemorySegmentMapped> segment_;
};
+TEST(MemorySegmentMappedConstantTest, staticVariables) {
+ // Attempt to take address of MemorySegmentMapped::INITIAL_SIZE.
+ // It helps in case we accidentally remove the definition from the main
+ // code.
+ EXPECT_EQ(DEFAULT_INITIAL_SIZE, *(&MemorySegmentMapped::INITIAL_SIZE));
+}
+
TEST_F(MemorySegmentMappedTest, createAndModify) {
// We are going to do the same set of basic tests twice; one after creating
// the mapped file, the other by re-opening the existing file in the
@@ -102,6 +128,7 @@ TEST_F(MemorySegmentMappedTest, createAndModify) {
// re-open it in read-write mode, but don't try to create it
// this time.
+ segment_.reset(); // make sure close is first.
segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_FOR_WRITE));
}
}
@@ -113,6 +140,7 @@ TEST_F(MemorySegmentMappedTest, createWithSize) {
// the size is actually the specified one.
const size_t new_size = 64 * 1024;
EXPECT_NE(new_size, segment_->getSize());
+ segment_.reset();
segment_.reset(new MemorySegmentMapped(mapped_file, OPEN_OR_CREATE,
new_size));
EXPECT_EQ(new_size, segment_->getSize());
@@ -263,22 +291,6 @@ checkNamedData(const std::string& name, const std::vector<uint8_t>& data,
ASSERT_TRUE(dp);
EXPECT_EQ(0, std::memcmp(dp, &data[0], data.size()));
- // Open a separate read-only segment and checks the same named data
- // Since the mapped space should be different, the resulting bare address
- // from getNamedAddress should also be different, but it should actually
- // point to the same data.
- // Note: it's mostly violation of the API assumption to open read-only
- // and read-write segments at the same time, but unless we modify the
- // segment throughout the lifetime of the read-only segment, it should
- // work.
- scoped_ptr<MemorySegmentMapped> segment_ro(
- new MemorySegmentMapped(mapped_file));
- void* dp2 = segment_ro->getNamedAddress(name.c_str());
- ASSERT_NE(static_cast<void*>(NULL), dp2);
- EXPECT_NE(dp, dp2);
- EXPECT_EQ(0, std::memcmp(dp2, &data[0], data.size()));
- segment_ro.reset();
-
if (delete_after_check) {
sgmt.deallocate(dp, data.size());
sgmt.clearNamedAddress(name.c_str());
@@ -294,13 +306,15 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
const uint16_t test_val16 = 42000;
*static_cast<uint16_t*>(ptr16) = test_val16;
EXPECT_FALSE(segment_->setNamedAddress("test address", ptr16));
- MemorySegmentMapped segment_ro(mapped_file);
+ segment_.reset(); // close it before opening another one
+
+ segment_.reset(new MemorySegmentMapped(mapped_file));
EXPECT_NE(static_cast<void*>(NULL),
- segment_ro.getNamedAddress("test address"));
+ segment_->getNamedAddress("test address"));
EXPECT_EQ(test_val16, *static_cast<const uint16_t*>(
- segment_ro.getNamedAddress("test address")));
+ segment_->getNamedAddress("test address")));
- // try to set an unusually long name. We re-create the file so the
+ // try to set an unusually long name. We re-create the file so
// creating the name would cause allocation failure and trigger internal
// segment extension.
segment_.reset();
@@ -357,13 +371,14 @@ TEST_F(MemorySegmentMappedTest, namedAddress) {
checkNamedData(it->first, it->second, *segment_);
}
- const size_t old_size = segment_->getSize();
- // Confirm they are still valid, while we shrink the segment
- BOOST_FOREACH(TestData::value_type e, data_list) {
- checkNamedData(e.first, e.second, *segment_, true);
+ // Confirm they are still valid, while we shrink the segment. We'll
+ // intentionally delete bigger data first so it'll be more likely that
+ // shrink has some real effect.
+ const char* const names[] = { "data3", "data2", "data1", NULL };
+ for (int i = 0; names[i]; ++i) {
+ checkNamedData(names[i], data_list[names[i]], *segment_, true);
segment_->shrinkToFit();
}
- EXPECT_GT(old_size, segment_->getSize());
}
TEST_F(MemorySegmentMappedTest, multiProcess) {
@@ -377,23 +392,23 @@ TEST_F(MemorySegmentMappedTest, multiProcess) {
*static_cast<uint32_t*>(ptr) = 424242;
segment_->setNamedAddress("test address", ptr);
- // reopen it in read-only. our intended use case is to have one or
- // more reader process or at most one exclusive writer process. so we
- // don't mix reader and writer.
- segment_.reset();
- segment_.reset(new MemorySegmentMapped(mapped_file));
- ptr = segment_->getNamedAddress("test address");
- ASSERT_TRUE(ptr);
- EXPECT_EQ(424242, *static_cast<const uint32_t*>(ptr));
+ // close the read-write segment at this point. our intended use case is
+ // to have one or more reader process or at most one exclusive writer
+ // process. so we don't mix reader and writer.
segment_.reset();
- // Spawn another process and have it open and read the same data
- int pipes[2];
- EXPECT_EQ(0, pipe(pipes));
+ // Spawn another process and have it open and read the same data.
+ PipeHolder pipe_to_child;
+ PipeHolder pipe_to_parent;
const pid_t child_pid = fork();
ASSERT_NE(-1, child_pid);
- if (child_pid == 0) { // child
- close(pipes[0]);
+ if (child_pid == 0) {
+ // child: wait until the parent has opened the read-only segment.
+ char from_parent;
+ EXPECT_EQ(1, read(pipe_to_child.getReadFD(), &from_parent,
+ sizeof(from_parent)));
+ EXPECT_EQ(0, from_parent);
+
MemorySegmentMapped sgmt(mapped_file);
void* ptr_child = sgmt.getNamedAddress("test address");
EXPECT_TRUE(ptr_child);
@@ -403,15 +418,22 @@ TEST_F(MemorySegmentMappedTest, multiProcess) {
// tell the parent whether it succeeded. 0 means it did,
// 0xff means it failed.
const char ok = (val == 424242) ? 0 : 0xff;
- EXPECT_EQ(1, write(pipes[1], &ok, sizeof(ok)));
+ EXPECT_EQ(1, write(pipe_to_parent.getWriteFD(), &ok, sizeof(ok)));
}
- close(pipes[1]);
exit(0);
}
- // parent: wait for the completion of the child and checks the result.
- close(pipes[1]);
- EXPECT_EQ(0, parentReadState(pipes[0]));
- close(pipes[0]);
+ // parent: open another read-only segment, then tell the child to open
+ // its own segment.
+ segment_.reset(new MemorySegmentMapped(mapped_file));
+ ptr = segment_->getNamedAddress("test address");
+ ASSERT_TRUE(ptr);
+ EXPECT_EQ(424242, *static_cast<const uint32_t*>(ptr));
+ const char some_data = 0;
+ EXPECT_EQ(1, write(pipe_to_child.getWriteFD(), &some_data,
+ sizeof(some_data)));
+
+ // wait for the completion of the child and checks the result.
+ EXPECT_EQ(0, parentReadState(pipe_to_parent.getReadFD()));
}
TEST_F(MemorySegmentMappedTest, nullDeallocate) {
@@ -429,8 +451,11 @@ TEST_F(MemorySegmentMappedTest, shrink) {
const size_t shrinked_size = segment_->getSize();
EXPECT_GT(DEFAULT_INITIAL_SIZE, shrinked_size);
- // Another shrink shouldn't cause disruption, and the size shouldn't
- // change.
+ // Another shrink shouldn't cause disruption. We expect the size is
+ // the same so we confirm it. The underlying library doesn't guarantee
+ // that, so we may have to change it to EXPECT_GE if the test fails
+ // on that (MemorySegmentMapped class doesn't rely on this expectation,
+ // so it's okay even if it does not always hold).
segment_->shrinkToFit();
EXPECT_EQ(shrinked_size, segment_->getSize());
@@ -440,25 +465,12 @@ TEST_F(MemorySegmentMappedTest, shrink) {
}
TEST_F(MemorySegmentMappedTest, violateReadOnly) {
- // If the segment is opened in the read-only mode, modification
- // attempts are prohibited. When detectable it must result in an
- // exception.
- EXPECT_THROW(MemorySegmentMapped(mapped_file).allocate(16),
- MemorySegmentError);
- // allocation that would otherwise require growing the segment; permission
- // check should be performed before that.
- EXPECT_THROW(MemorySegmentMapped(mapped_file).
- allocate(DEFAULT_INITIAL_SIZE * 2), MemorySegmentError);
- EXPECT_THROW(MemorySegmentMapped(mapped_file).setNamedAddress("test",
- NULL),
- MemorySegmentError);
- EXPECT_THROW(MemorySegmentMapped(mapped_file).clearNamedAddress("test"),
- MemorySegmentError);
- EXPECT_THROW(MemorySegmentMapped(mapped_file).shrinkToFit(),
- MemorySegmentError);
-
+ // Create a named address for the tests below, then reset the writer
+ // segment so that it won't fail for different reason (i.e., read-write
+ // conflict).
void* ptr = segment_->allocate(sizeof(uint32_t));
segment_->setNamedAddress("test address", ptr);
+ segment_.reset();
// Attempts to modify memory from the read-only segment directly
// will result in a crash.
@@ -471,8 +483,23 @@ TEST_F(MemorySegmentMappedTest, violateReadOnly) {
}, "");
}
- EXPECT_THROW(MemorySegmentMapped(mapped_file).deallocate(ptr, 4),
+ // If the segment is opened in the read-only mode, modification
+ // attempts are prohibited. When detectable it must result in an
+ // exception.
+ MemorySegmentMapped segment_ro(mapped_file);
+ ptr = segment_ro.getNamedAddress("test address");
+ EXPECT_NE(static_cast<void*>(NULL), ptr);
+
+ EXPECT_THROW(segment_ro.deallocate(ptr, 4), MemorySegmentError);
+
+ EXPECT_THROW(segment_ro.allocate(16), MemorySegmentError);
+ // allocation that would otherwise require growing the segment; permission
+ // check should be performed before that.
+ EXPECT_THROW(segment_ro.allocate(DEFAULT_INITIAL_SIZE * 2),
MemorySegmentError);
+ EXPECT_THROW(segment_ro.setNamedAddress("test", NULL), MemorySegmentError);
+ EXPECT_THROW(segment_ro.clearNamedAddress("test"), MemorySegmentError);
+ EXPECT_THROW(segment_ro.shrinkToFit(), MemorySegmentError);
}
TEST_F(MemorySegmentMappedTest, getCheckSum) {
@@ -492,4 +519,102 @@ TEST_F(MemorySegmentMappedTest, getCheckSum) {
EXPECT_EQ(old_cksum + 1, segment_->getCheckSum());
}
+// Mode of opening segments in the tests below.
+enum TestOpenMode {
+ READER = 0,
+ WRITER_FOR_WRITE,
+ WRITER_OPEN_OR_CREATE,
+ WRITER_CREATE_ONLY
+};
+
+// A shortcut to attempt to open a specified type of segment (generally
+// expecting it to fail)
+void
+setSegment(TestOpenMode mode, scoped_ptr<MemorySegmentMapped>& sgmt_ptr) {
+ switch (mode) {
+ case READER:
+ sgmt_ptr.reset(new MemorySegmentMapped(mapped_file));
+ break;
+ case WRITER_FOR_WRITE:
+ sgmt_ptr.reset(new MemorySegmentMapped(mapped_file, OPEN_FOR_WRITE));
+ break;
+ case WRITER_OPEN_OR_CREATE:
+ sgmt_ptr.reset(new MemorySegmentMapped(mapped_file, OPEN_OR_CREATE));
+ break;
+ case WRITER_CREATE_ONLY:
+ sgmt_ptr.reset(new MemorySegmentMapped(mapped_file, CREATE_ONLY));
+ break;
+ }
+}
+
+// Common logic for conflictReaderWriter test. The segment opened in the
+// parent process will prevent the segment in the child from being used.
+void
+conflictCheck(TestOpenMode parent_mode, TestOpenMode child_mode) {
+ PipeHolder pipe_to_child;
+ PipeHolder pipe_to_parent;
+ const pid_t child_pid = fork();
+ ASSERT_NE(-1, child_pid);
+
+ if (child_pid == 0) {
+ char ch;
+ EXPECT_EQ(1, read(pipe_to_child.getReadFD(), &ch, sizeof(ch)));
+
+ ch = 0; // 0 = open success, 1 = fail
+ try {
+ scoped_ptr<MemorySegmentMapped> sgmt;
+ setSegment(child_mode, sgmt);
+ EXPECT_EQ(1, write(pipe_to_parent.getWriteFD(), &ch, sizeof(ch)));
+ } catch (const MemorySegmentOpenError&) {
+ ch = 1;
+ EXPECT_EQ(1, write(pipe_to_parent.getWriteFD(), &ch, sizeof(ch)));
+ }
+ exit(0);
+ }
+
+ // parent: open a segment, then tell the child to open its own segment of
+ // the specified type.
+ scoped_ptr<MemorySegmentMapped> sgmt;
+ setSegment(parent_mode, sgmt);
+ const char some_data = 0;
+ EXPECT_EQ(1, write(pipe_to_child.getWriteFD(), &some_data,
+ sizeof(some_data)));
+
+ // wait for the completion of the child and checks the result. open at
+ // the child side should fail, so the parent should get the value of 1.
+ EXPECT_EQ(1, parentReadState(pipe_to_parent.getReadFD()));
+}
+
+TEST_F(MemorySegmentMappedTest, conflictReaderWriter) {
+ // Test using fork() doesn't work well on valgrind
+ if (isc::util::unittests::runningOnValgrind()) {
+ return;
+ }
+
+ // Below, we check all combinations of conflicts between reader and writer
+ // will fail. We first make sure there's no other reader or writer.
+ segment_.reset();
+
+ // reader opens segment, then writer (OPEN_FOR_WRITE) tries to open
+ conflictCheck(READER, WRITER_FOR_WRITE);
+ // reader opens segment, then writer (OPEN_OR_CREATE) tries to open
+ conflictCheck(READER, WRITER_OPEN_OR_CREATE);
+ // reader opens segment, then writer (CREATE_ONLY) tries to open
+ conflictCheck(READER, WRITER_CREATE_ONLY);
+
+ // writer (OPEN_FOR_WRITE) opens a segment, then reader tries to open
+ conflictCheck(WRITER_FOR_WRITE, READER);
+ // writer (OPEN_OR_CREATE) opens a segment, then reader tries to open
+ conflictCheck(WRITER_OPEN_OR_CREATE, READER);
+ // writer (CREATE_ONLY) opens a segment, then reader tries to open
+ conflictCheck(WRITER_CREATE_ONLY, READER);
+
+ // writer opens segment, then another writer (OPEN_FOR_WRITE) tries to open
+ conflictCheck(WRITER_FOR_WRITE, WRITER_FOR_WRITE);
+ // writer opens segment, then another writer (OPEN_OR_CREATE) tries to open
+ conflictCheck(WRITER_FOR_WRITE, WRITER_OPEN_OR_CREATE);
+ // writer opens segment, then another writer (CREATE_ONLY) tries to open
+ conflictCheck(WRITER_FOR_WRITE, WRITER_CREATE_ONLY);
+}
+
}
More information about the bind10-changes
mailing list