[2831] tested other corner cases of deallocate()

src/lib/util/memory_segment_mapped.h

@@ -45,6 +45,19 @@ public:
     virtual void* allocate(size_t size);
 
     /// \brief Deallocate/release a segment of memory.
+    ///
+    /// This implementation does not check the validity of \c size, because
+    /// if this segment object was constructed for an existing file to map,
+    /// the underlying segment may already contain allocated regions, so
+    /// this object cannot reliably detect whether it's safe to deallocate
+    /// the given size of memory from the underlying segment.
+    ///
+    /// Parameter \c ptr must point to an address that was returned by a
+    /// prior call to \c allocate() of this segment object, and there should
+    /// not be a \c MemorySegmentGrown exception thrown from \c allocate()
+    /// since then; if it was thrown the corresponding address must have been
+    /// adjusted some way; e.g., by re-fetching the latest mapped address
+    /// via \c getNamedAddress().
     virtual void deallocate(void* ptr, size_t size);
 
     virtual bool allMemoryDeallocated() const;

src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -34,14 +34,7 @@ const size_t DEFAULT_INITIAL_SIZE = 32 * 1024; // intentionally hardcoded
 class MemorySegmentMappedTest : public ::testing::Test {
 protected:
     MemorySegmentMappedTest() {
-        // Make sure the mapped file doesn't exist; actually it shouldn't
-        // exist in normal cases so remove() should normally fail (returning
-        // false), but we don't care.
-        boost::interprocess::file_mapping::remove(mapped_file);
-
-        // Create a new segment with a new file.  It also confirms the
-        // behavior of this mode of constructor.
-        segment_.reset(new MemorySegmentMapped(mapped_file, true));
+        resetSegment();
     }
 
     ~MemorySegmentMappedTest() {
@@ -49,6 +42,14 @@ protected:
         boost::interprocess::file_mapping::remove(mapped_file);
     }
 
+    // For initialization and for tests after the segment possibly becomes
+    // broken.
+    void resetSegment() {
+        segment_.reset();
+        boost::interprocess::file_mapping::remove(mapped_file);
+        segment_.reset(new MemorySegmentMapped(mapped_file, true));
+    }
+
     scoped_ptr<MemorySegmentMapped> segment_;
 };
 
@@ -151,6 +152,32 @@ TEST_F(MemorySegmentMappedTest, DISABLED_allocateHuge) {
                  std::bad_alloc);
 }
 
+TEST_F(MemorySegmentMappedTest, badDeallocate) {
+    void* ptr = segment_->allocate(4);
+    EXPECT_NE(static_cast<void*>(0), ptr);
+
+    segment_->deallocate(ptr, 4); // this is okay
+    // This is duplicate dealloc; should trigger assertion failure.
+    EXPECT_DEATH_IF_SUPPORTED({segment_->deallocate(ptr, 4);}, "");
+    resetSegment();             // the segment is possibly broken; reset it.
+
+    // Deallocating at an invalid address; this would result in crash (the
+    // behavior may not be portable enough; if so we should disable it by
+    // default).
+    ptr = segment_->allocate(4);
+    EXPECT_NE(static_cast<void*>(0), ptr);
+    EXPECT_DEATH_IF_SUPPORTED({
+            segment_->deallocate(static_cast<char*>(ptr) + 1, 3);
+        }, "");
+    resetSegment();
+
+    // Invalid size; this implementation doesn't detect such errors.
+    ptr = segment_->allocate(4);
+    EXPECT_NE(static_cast<void*>(0), ptr);
+    segment_->deallocate(ptr, 8);
+    EXPECT_TRUE(segment_->allMemoryDeallocated());
+}
+
 TEST_F(MemorySegmentMappedTest, DISABLED_basics) {
     MemorySegmentMapped segment(mapped_file);