Browse Source

[2850] Use non-const implementation of allMemoryDeallocated() again

Mukund Sivaraman 12 years ago
parent
commit
54c8596393

+ 13 - 14
src/lib/util/memory_segment_mapped.cc

@@ -74,7 +74,7 @@ struct MemorySegmentMapped::Impl {
     // to detect possible conflict with other readers or writers using
     // to detect possible conflict with other readers or writers using
     // file lock.
     // file lock.
     Impl(const std::string& filename, create_only_t, size_t initial_size) :
     Impl(const std::string& filename, create_only_t, size_t initial_size) :
-        read_only_(false), filename_(filename), allocated_size_(0)
+        read_only_(false), filename_(filename)
     {
     {
         try {
         try {
             // First, try opening it in boost create_only mode; it fails if
             // First, try opening it in boost create_only mode; it fails if
@@ -115,7 +115,6 @@ struct MemorySegmentMapped::Impl {
         read_only_(false), filename_(filename),
         read_only_(false), filename_(filename),
         base_sgmt_(new BaseSegment(open_or_create, filename.c_str(),
         base_sgmt_(new BaseSegment(open_or_create, filename.c_str(),
                                    initial_size)),
                                    initial_size)),
-        allocated_size_(0),
         lock_(new boost::interprocess::file_lock(filename.c_str()))
         lock_(new boost::interprocess::file_lock(filename.c_str()))
     {
     {
         checkWriter();
         checkWriter();
@@ -128,7 +127,6 @@ struct MemorySegmentMapped::Impl {
         base_sgmt_(read_only_ ?
         base_sgmt_(read_only_ ?
                    new BaseSegment(open_read_only, filename.c_str()) :
                    new BaseSegment(open_read_only, filename.c_str()) :
                    new BaseSegment(open_only, filename.c_str())),
                    new BaseSegment(open_only, filename.c_str())),
-        allocated_size_(0),
         lock_(new boost::interprocess::file_lock(filename.c_str()))
         lock_(new boost::interprocess::file_lock(filename.c_str()))
     {
     {
         if (read_only_) {
         if (read_only_) {
@@ -212,10 +210,6 @@ struct MemorySegmentMapped::Impl {
     // actual Boost implementation of mapped segment.
     // actual Boost implementation of mapped segment.
     boost::scoped_ptr<BaseSegment> base_sgmt_;
     boost::scoped_ptr<BaseSegment> base_sgmt_;
 
 
-    // number of bytes of memory currently allocated in this segment
-    // through allocate().
-    size_t allocated_size_;
-
 private:
 private:
     // helper methods and member to detect any reader-writer conflict at
     // helper methods and member to detect any reader-writer conflict at
     // the time of construction using an advisory file lock.  The lock will
     // the time of construction using an advisory file lock.  The lock will
@@ -299,7 +293,6 @@ MemorySegmentMapped::allocate(size_t size) {
     if (impl_->base_sgmt_->get_free_memory() >= size) {
     if (impl_->base_sgmt_->get_free_memory() >= size) {
         void* ptr = impl_->base_sgmt_->allocate(size, std::nothrow);
         void* ptr = impl_->base_sgmt_->allocate(size, std::nothrow);
         if (ptr) {
         if (ptr) {
-            impl_->allocated_size_ += size;
             return (ptr);
             return (ptr);
         }
         }
     }
     }
@@ -315,7 +308,7 @@ MemorySegmentMapped::allocate(size_t size) {
 }
 }
 
 
 void
 void
-MemorySegmentMapped::deallocate(void* ptr, size_t size) {
+MemorySegmentMapped::deallocate(void* ptr, size_t) {
     if (impl_->read_only_) {
     if (impl_->read_only_) {
         isc_throw(MemorySegmentError,
         isc_throw(MemorySegmentError,
                   "deallocate attempt on read-only segment");
                   "deallocate attempt on read-only segment");
@@ -328,15 +321,21 @@ MemorySegmentMapped::deallocate(void* ptr, size_t size) {
     }
     }
 
 
     impl_->base_sgmt_->deallocate(ptr);
     impl_->base_sgmt_->deallocate(ptr);
-    impl_->allocated_size_ -= size;
 }
 }
 
 
 bool
 bool
 MemorySegmentMapped::allMemoryDeallocated() const {
 MemorySegmentMapped::allMemoryDeallocated() const {
-     const size_t expected_num_named_objs = impl_->read_only_ ? 0 : 1;
-     const size_t num_named_objs = impl_->base_sgmt_->get_num_named_objects();
-     return ((impl_->allocated_size_ == 0) &&
-             (num_named_objs == expected_num_named_objs));
+    // This method is not technically const, but it reserves the
+    // const-ness property. In case of exceptions, we abort here. (See
+    // ticket #2850 for additional commentary.)
+    try {
+        impl_->freeReservedMemory();
+        const bool result = impl_->base_sgmt_->all_memory_deallocated();
+        impl_->reserveMemory();
+        return (result);
+    } catch (...) {
+        abort();
+    }
 }
 }
 
 
 MemorySegment::NamedAddressResult
 MemorySegment::NamedAddressResult

+ 2 - 4
src/lib/util/tests/memory_segment_mapped_unittest.cc

@@ -276,13 +276,11 @@ TEST_F(MemorySegmentMappedTest, badDeallocate) {
         resetSegment();
         resetSegment();
     }
     }
 
 
-    // Invalid size; this implementation doesn't detect such errors and
-    // will free the memory, but \c allMemoryDeallocated() will detect
-    // it.
+    // Invalid size; this implementation doesn't detect such errors.
     ptr = segment_->allocate(4);
     ptr = segment_->allocate(4);
     EXPECT_NE(static_cast<void*>(NULL), ptr);
     EXPECT_NE(static_cast<void*>(NULL), ptr);
     segment_->deallocate(ptr, 8);
     segment_->deallocate(ptr, 8);
-    EXPECT_FALSE(segment_->allMemoryDeallocated());
+    EXPECT_TRUE(segment_->allMemoryDeallocated());
 }
 }
 
 
 // A helper of namedAddress.
 // A helper of namedAddress.