[eclipse-iceoryx#497] SegmentId is incrementing and allocations will …

…fail when the maximum amount of reallocations was exceeded
elfenpiff · Nov 19, 2024 · a535a38 · a535a38
1 parent 59011fd
commit a535a38
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 13 deletions.
diff --git a/iceoryx2-cal/src/resizable_shared_memory/dynamic.rs b/iceoryx2-cal/src/resizable_shared_memory/dynamic.rs
@@ -34,9 +34,11 @@ use iceoryx2_pal_concurrency_sync::iox_atomic::{IoxAtomicBool, IoxAtomicU64};
 use super::{
     NamedConcept, NamedConceptBuilder, NamedConceptDoesExistError, NamedConceptListError,
     NamedConceptMgmt, NamedConceptRemoveError, ResizableSharedMemory, ResizableSharedMemoryBuilder,
-    ResizableSharedMemoryView, ResizableShmAllocationError, MAX_DATASEGMENTS,
+    ResizableSharedMemoryView, ResizableShmAllocationError,
 };
 
+const MAX_DATASEGMENTS: usize = SegmentId::max_segment_id() as usize + 1;
+
 #[derive(Debug)]
 struct BuilderConfig<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> {
     base_name: FileName,
@@ -218,6 +220,8 @@ pub struct DynamicView<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> {
 
 impl<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>>
     ResizableSharedMemoryView<Allocator, Shm> for DynamicView<Allocator, Shm>
+where
+    Shm::Builder: Debug,
 {
     fn register_and_translate_offset(
         &mut self,
@@ -314,7 +318,10 @@ impl<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> NamedConceptMgmt
     }
 }
 
-impl<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> DynamicMemory<Allocator, Shm> {
+impl<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> DynamicMemory<Allocator, Shm>
+where
+    Shm::Builder: Debug,
+{
     fn state_mut(&self) -> &mut InternalState<Allocator, Shm> {
         unsafe { &mut *self.state.get() }
     }
@@ -364,12 +371,12 @@ impl<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> DynamicMemory<Alloca
         let adjusted_segment_setup = shm
             .allocator()
             .resize_hint(layout, state.builder_config.allocation_strategy);
-        let segment_id = match state.shared_memory_map.next_free_key() {
-            Some(v) => v,
-            None => {
-                fail!(from self, with SharedMemoryCreateError::InternalError,
-                            "{msg} {:?} since the internal shared memory map cannot hold any more segments.", layout);
-            }
+        let segment_id = if state.current_idx.value() < SegmentId::max_segment_id() as usize {
+            SlotMapKey::new(state.current_idx.value() + 1)
+        } else {
+            fail!(from self, with SharedMemoryCreateError::InternalError,
+                "{msg} {:?} since it would exceed the maximum amount of reallocations of {}. With a better configuration hint, this issue can be avoided.",
+                layout, Self::max_number_of_reallocations());
         };
 
         state.builder_config.allocator_config_hint = adjusted_segment_setup.config;
@@ -386,7 +393,9 @@ impl<Allocator: ShmAllocator, Shm: SharedMemory<Allocator>> DynamicMemory<Alloca
                 }
             }
             None => {
-                fatal_panic!(from self, "This should never happen! Current segment id is unavailable.")
+                fatal_panic!(from self,
+                        "This should never happen! {msg} {:?} since the current segment id is unavailable.",
+                        layout)
             }
         }
 
@@ -407,6 +416,10 @@ where
     type Builder = DynamicBuilder<Allocator, Shm>;
     type View = DynamicView<Allocator, Shm>;
 
+    fn max_number_of_reallocations() -> usize {
+        SegmentId::max_segment_id() as usize + 1
+    }
+
     fn number_of_active_segments(&self) -> usize {
         self.state().shared_memory_map.len()
     }

diff --git a/iceoryx2-cal/src/resizable_shared_memory/mod.rs b/iceoryx2-cal/src/resizable_shared_memory/mod.rs
@@ -24,8 +24,6 @@ use crate::shared_memory::{
 };
 use crate::shm_allocator::{PointerOffset, ShmAllocationError, ShmAllocator};
 
-const MAX_DATASEGMENTS: usize = 256;
-
 enum_gen! { ResizableShmAllocationError
   mapping:
     ShmAllocationError,
@@ -80,6 +78,8 @@ pub trait ResizableSharedMemory<Allocator: ShmAllocator, Shm: SharedMemory<Alloc
     type Builder: ResizableSharedMemoryBuilder<Allocator, Shm, Self, Self::View>;
     type View: ResizableSharedMemoryView<Allocator, Shm>;
 
+    fn max_number_of_reallocations() -> usize;
+
     fn number_of_active_segments(&self) -> usize;
 
     fn allocate(

diff --git a/iceoryx2-cal/src/shm_allocator/mod.rs b/iceoryx2-cal/src/shm_allocator/mod.rs
@@ -13,7 +13,7 @@
 pub mod bump_allocator;
 pub mod pool_allocator;
 
-use std::{alloc::Layout, fmt::Debug, ptr::NonNull};
+use std::{alloc::Layout, fmt::Debug, ptr::NonNull, u16};
 
 pub use iceoryx2_bb_elementary::allocator::AllocationError;
 use iceoryx2_bb_elementary::{allocator::BaseAllocator, enum_gen};
@@ -30,6 +30,10 @@ impl SegmentId {
     pub fn value(&self) -> u16 {
         self.0
     }
+
+    pub const fn max_segment_id() -> u16 {
+        u16::MAX
+    }
 }
 
 #[derive(Clone, Copy, Eq, PartialEq)]
@@ -50,7 +54,7 @@ impl PointerOffset {
     }
 
     pub fn segment_id(&self) -> SegmentId {
-        SegmentId((self.0 & 0x000000000000ffff) as u16)
+        SegmentId((self.0 & SegmentId::max_segment_id() as u64) as u16)
     }
 }