Skip to content

Commit

Permalink
[#100] Introduce ContainerHandle; improve readability
Browse files Browse the repository at this point in the history
  • Loading branch information
elfenpiff committed Feb 8, 2024
1 parent 49b6954 commit 85b9c9d
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 67 deletions.
51 changes: 28 additions & 23 deletions iceoryx2-bb/lock-free/src/mpmc/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ use std::{
sync::atomic::{AtomicBool, Ordering},
};

/// A handle that corresponds to an element inside the [`Container`]. Will be acquired when using
/// [`Container::add_with_handle()`] and can be released with [`Container::remove_with_handle()`].
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub struct ContainerHandle(u32);

/// Contains a state of the [`Container`]. Can be created with [`Container::get_state()`] and
/// updated when the [`Container`] has changed with [`Container::update_state()`].
#[derive(Debug)]
Expand Down Expand Up @@ -300,11 +305,11 @@ impl<T: Copy + Debug> Container<T> {
///
/// * Ensure that the either [`Container::new()`] was used or [`Container::init()`] was used
/// before calling this method
/// * Use [`Container::remove_raw_index()`] to release the acquired index again. Otherwise, the
/// * Use [`Container::remove_with_handle()`] to release the acquired index again. Otherwise, the
/// element will leak.
///
pub unsafe fn add_raw(&self, value: T) -> Option<u32> {
self.verify_memory_initialization("add_raw");
pub unsafe fn add_with_handle(&self, value: T) -> Option<ContainerHandle> {
self.verify_memory_initialization("add_with_handle");

match self.index_set.acquire_raw_index() {
Some(index) => {
Expand All @@ -319,7 +324,7 @@ impl<T: Copy + Debug> Container<T> {
unsafe { &*self.active_index_ptr.as_ptr().offset(index as isize) }
.store(true, Ordering::Release);

Some(index)
Some(ContainerHandle(index))
}
None => None,
}
Expand All @@ -337,11 +342,11 @@ impl<T: Copy + Debug> Container<T> {
/// **Important:** If the UniqueIndex still exists it causes double frees or freeing an index
/// which was allocated afterwards
///
pub unsafe fn remove_raw_index(&self, index: u32) {
self.verify_memory_initialization("remove_raw_index");
unsafe { &*self.active_index_ptr.as_ptr().offset(index as isize) }
pub unsafe fn remove_with_handle(&self, handle: ContainerHandle) {
self.verify_memory_initialization("remove_with_handle");
unsafe { &*self.active_index_ptr.as_ptr().offset(handle.0 as isize) }
.store(false, Ordering::Relaxed);
self.index_set.release_raw_index(index);
self.index_set.release_raw_index(handle.0);
}

/// Returns [`ContainerState`] which contains all elements of this container. Be aware that
Expand All @@ -368,7 +373,7 @@ impl<T: Copy + Debug> Container<T> {
/// * Ensure that the either [`Container::new()`] was used or [`Container::init()`] was used
/// before calling this method
/// * Ensure that the input argument `previous_state` was acquired by the same [`Container`]
/// with [`Container::get_state()`].
/// with [`Container::get_state()`], otherwise the method will panic.
///
pub unsafe fn update_state(&self, previous_state: &mut ContainerState<T>) -> bool {
if previous_state.container_id != self.container_id.value() {
Expand Down Expand Up @@ -431,7 +436,7 @@ impl<T: Copy + Debug> Container<T> {
#[repr(C)]
#[derive(Debug)]
pub struct FixedSizeContainer<T: Copy + Debug, const CAPACITY: usize> {
state: Container<T>,
container: Container<T>,

// DO NOT CHANGE MEMBER ORDER UniqueIndexSet variable data
next_free_index: [UnsafeCell<u32>; CAPACITY],
Expand All @@ -445,7 +450,7 @@ pub struct FixedSizeContainer<T: Copy + Debug, const CAPACITY: usize> {
impl<T: Copy + Debug, const CAPACITY: usize> Default for FixedSizeContainer<T, CAPACITY> {
fn default() -> Self {
Self {
state: unsafe {
container: unsafe {
Container::new(
CAPACITY,
align_to::<u32>(std::mem::size_of::<Container<T>>()) as isize,
Expand All @@ -470,7 +475,7 @@ impl<T: Copy + Debug, const CAPACITY: usize> FixedSizeContainer<T, CAPACITY> {

/// Returns the capacity of the container
pub fn capacity(&self) -> usize {
self.state.capacity()
self.container.capacity()
}

/// Adds a new element to the [`FixedSizeContainer`]. If there is no more space available it returns
Expand All @@ -489,7 +494,7 @@ impl<T: Copy + Debug, const CAPACITY: usize> FixedSizeContainer<T, CAPACITY> {
/// };
/// ```
pub fn add(&self, value: T) -> Option<UniqueIndex<'_>> {
unsafe { self.state.add(value) }
unsafe { self.container.add(value) }
}

/// Adds a new element to the [`FixedSizeContainer`]. If there is no more space available it returns
Expand All @@ -501,10 +506,10 @@ impl<T: Copy + Debug, const CAPACITY: usize> FixedSizeContainer<T, CAPACITY> {
/// const CAPACITY: usize = 139;
/// let container = FixedSizeContainer::<u128, CAPACITY>::new();
///
/// match unsafe { container.add_raw(1234567) } {
/// match unsafe { container.add_with_handle(1234567) } {
/// Some(index) => {
/// println!("added at index {}", index);
/// unsafe { container.remove_raw_index(index) };
/// println!("added at index {:?}", index);
/// unsafe { container.remove_with_handle(index) };
/// },
/// None => println!("container is full"),
/// };
Expand All @@ -513,11 +518,11 @@ impl<T: Copy + Debug, const CAPACITY: usize> FixedSizeContainer<T, CAPACITY> {
///
/// # Safety
///
/// * Use [`FixedSizeContainer::remove_raw_index()`] to release the acquired index again. Otherwise,
/// * Use [`FixedSizeContainer::remove_with_handle()`] to release the acquired index again. Otherwise,
/// the element will leak.
///
pub unsafe fn add_raw(&self, value: T) -> Option<u32> {
self.state.add_raw(value)
pub unsafe fn add_with_handle(&self, value: T) -> Option<ContainerHandle> {
self.container.add_with_handle(value)
}

/// Useful in IPC context when an application holding the UniqueIndex has died.
Expand All @@ -526,14 +531,14 @@ impl<T: Copy + Debug, const CAPACITY: usize> FixedSizeContainer<T, CAPACITY> {
///
/// * If the UniqueIndex still exists it causes double frees or freeing an index
/// which was allocated afterwards
pub unsafe fn remove_raw_index(&self, index: u32) {
self.state.remove_raw_index(index)
pub unsafe fn remove_with_handle(&self, handle: ContainerHandle) {
self.container.remove_with_handle(handle)
}

/// Returns [`ContainerState`] which contains all elements of this container. Be aware that
/// this state can be out of date as soon as it is returned from this function.
pub fn get_state(&self) -> ContainerState<T> {
unsafe { self.state.get_state() }
unsafe { self.container.get_state() }
}

/// Syncs the [`ContainerState`] with the current state of the [`FixedSizeContainer`].
Expand All @@ -559,6 +564,6 @@ impl<T: Copy + Debug, const CAPACITY: usize> FixedSizeContainer<T, CAPACITY> {
/// with [`Container::get_state()`].
///
pub unsafe fn update_state(&self, previous_state: &mut ContainerState<T>) -> bool {
unsafe { self.state.update_state(previous_state) }
unsafe { self.container.update_state(previous_state) }
}
}
27 changes: 10 additions & 17 deletions iceoryx2-bb/lock-free/tests/mpmc_container_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ mod mpmc_container {
use std::collections::HashMap;
use std::collections::HashSet;
use std::fmt::Debug;
use std::mem::MaybeUninit;
use std::sync::atomic::AtomicU64;
use std::sync::atomic::Ordering;
use std::sync::{Barrier, Mutex};
Expand Down Expand Up @@ -143,28 +142,22 @@ mod mpmc_container {
}

#[test]
fn mpmc_container_add_and_unsafe_remove_elements_works<
fn mpmc_container_add_and_unsafe_remove_with_handle_works<
T: Debug + Copy + From<usize> + Into<usize>,
>() {
let sut = FixedSizeContainer::<T, CAPACITY>::new();
let mut stored_indices: Vec<MaybeUninit<UniqueIndex>> = vec![];
let mut stored_handles: Vec<ContainerHandle> = vec![];

for i in 0..CAPACITY - 1 {
let index = sut.add((i * 3 + 1).into());
assert_that!(index, is_some);
stored_indices.push(MaybeUninit::new(index.unwrap()));
let handle = unsafe { sut.add_with_handle((i * 3 + 1).into()) };
assert_that!(handle, is_some);
stored_handles.push(handle.unwrap());

let index = sut.add((i * 7 + 5).into());
assert_that!(index, is_some);
stored_indices.push(MaybeUninit::new(index.unwrap()));

unsafe {
sut.remove_raw_index(
stored_indices[stored_indices.len() - 2]
.assume_init_ref()
.value(),
)
};
let handle = unsafe { sut.add_with_handle((i * 7 + 5).into()) };
assert_that!(handle, is_some);
stored_handles.push(handle.unwrap());

unsafe { sut.remove_with_handle(stored_handles[stored_handles.len() - 2].clone()) };
}

let state = sut.get_state();
Expand Down
5 changes: 3 additions & 2 deletions iceoryx2/src/port/listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
//!
//! See also [`crate::port::listener::Listener`]
use iceoryx2_bb_lock_free::mpmc::container::ContainerHandle;
use iceoryx2_bb_log::fail;
use iceoryx2_cal::dynamic_storage::DynamicStorage;
use iceoryx2_cal::event::{ListenerBuilder, ListenerWaitError};
Expand All @@ -48,7 +49,7 @@ use super::listen::{Listen, ListenerCreateError};
/// Represents the receiving endpoint of an event based communication.
#[derive(Debug)]
pub struct Listener<Service: service::Service> {
dynamic_listener_handle: u32,
dynamic_listener_handle: ContainerHandle,
listener: <Service::Event as iceoryx2_cal::event::Event<EventId>>::Listener,
cache: Vec<EventId>,
dynamic_storage: Rc<Service::DynamicStorage>,
Expand All @@ -59,7 +60,7 @@ impl<Service: service::Service> Drop for Listener<Service> {
self.dynamic_storage
.get()
.event()
.release_listener_id(self.dynamic_listener_handle)
.release_listener_handle(self.dynamic_listener_handle)
}
}

Expand Down
6 changes: 3 additions & 3 deletions iceoryx2/src/port/notifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use crate::{
port::port_identifiers::UniqueNotifierId,
service::{self, naming_scheme::event_concept_name},
};
use iceoryx2_bb_lock_free::mpmc::container::ContainerState;
use iceoryx2_bb_lock_free::mpmc::container::{ContainerHandle, ContainerState};
use iceoryx2_bb_log::{fail, warn};
use iceoryx2_cal::named_concept::NamedConceptBuilder;
use iceoryx2_cal::{dynamic_storage::DynamicStorage, event::NotifierBuilder};
Expand Down Expand Up @@ -117,15 +117,15 @@ pub struct Notifier<Service: service::Service> {
listener_list_state: UnsafeCell<ContainerState<UniqueListenerId>>,
default_event_id: EventId,
dynamic_storage: Rc<Service::DynamicStorage>,
dynamic_notifier_handle: u32,
dynamic_notifier_handle: ContainerHandle,
}

impl<Service: service::Service> Drop for Notifier<Service> {
fn drop(&mut self) {
self.dynamic_storage
.get()
.event()
.release_notifier_id(self.dynamic_notifier_handle)
.release_notifier_handle(self.dynamic_notifier_handle)
}
}

Expand Down
6 changes: 3 additions & 3 deletions iceoryx2/src/port/publisher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ use crate::service::static_config::publish_subscribe;
use crate::{config, sample_mut::SampleMut};
use iceoryx2_bb_container::queue::Queue;
use iceoryx2_bb_elementary::allocator::AllocationError;
use iceoryx2_bb_lock_free::mpmc::container::ContainerState;
use iceoryx2_bb_lock_free::mpmc::container::{ContainerHandle, ContainerState};
use iceoryx2_bb_log::{fail, fatal_panic, warn};
use iceoryx2_cal::dynamic_storage::DynamicStorage;
use iceoryx2_cal::named_concept::NamedConceptBuilder;
Expand All @@ -108,7 +108,7 @@ pub struct Publisher<'a, Service: service::Service, MessageType: Debug> {
service: &'a Service,
degration_callback: Option<DegrationCallback<'a>>,
loan_counter: AtomicUsize,
dynamic_publisher_handle: u32,
dynamic_publisher_handle: ContainerHandle,
_phantom_message_type: PhantomData<MessageType>,
}

Expand All @@ -119,7 +119,7 @@ impl<'a, Service: service::Service, MessageType: Debug> Drop
self.dynamic_storage
.get()
.publish_subscribe()
.release_publisher_id(self.dynamic_publisher_handle)
.release_publisher_handle(self.dynamic_publisher_handle)
}
}

Expand Down
6 changes: 3 additions & 3 deletions iceoryx2/src/port/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use std::fmt::Debug;
use std::marker::PhantomData;
use std::rc::Rc;

use iceoryx2_bb_lock_free::mpmc::container::ContainerState;
use iceoryx2_bb_lock_free::mpmc::container::{ContainerHandle, ContainerState};
use iceoryx2_bb_log::{fail, fatal_panic, warn};
use iceoryx2_cal::dynamic_storage::DynamicStorage;
use iceoryx2_cal::{shared_memory::*, zero_copy_connection::*};
Expand All @@ -58,7 +58,7 @@ use super::DegrationCallback;
/// The receiving endpoint of a publish-subscribe communication.
#[derive(Debug)]
pub struct Subscriber<'a, Service: service::Service, MessageType: Debug> {
dynamic_subscriber_handle: u32,
dynamic_subscriber_handle: ContainerHandle,
publisher_connections: PublisherConnections<Service>,
dynamic_storage: Rc<Service::DynamicStorage>,
service: &'a Service,
Expand All @@ -75,7 +75,7 @@ impl<'a, Service: service::Service, MessageType: Debug> Drop
self.dynamic_storage
.get()
.publish_subscribe()
.release_subscriber_id(self.dynamic_subscriber_handle)
.release_subscriber_handle(self.dynamic_subscriber_handle)
}
}

Expand Down
16 changes: 8 additions & 8 deletions iceoryx2/src/service/dynamic_config/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ impl DynamicConfig {
self.notifiers.len()
}

pub(crate) fn add_listener_id(&self, id: UniqueListenerId) -> Option<u32> {
unsafe { self.listeners.add_raw(id) }
pub(crate) fn add_listener_id(&self, id: UniqueListenerId) -> Option<ContainerHandle> {
unsafe { self.listeners.add_with_handle(id) }
}

pub(crate) fn release_listener_id(&self, id: u32) {
unsafe { self.listeners.remove_raw_index(id) }
pub(crate) fn release_listener_handle(&self, handle: ContainerHandle) {
unsafe { self.listeners.remove_with_handle(handle) }
}

pub(crate) fn add_notifier_id(&self, id: UniqueNotifierId) -> Option<u32> {
unsafe { self.notifiers.add_raw(id) }
pub(crate) fn add_notifier_id(&self, id: UniqueNotifierId) -> Option<ContainerHandle> {
unsafe { self.notifiers.add_with_handle(id) }
}

pub(crate) fn release_notifier_id(&self, id: u32) {
unsafe { self.notifiers.remove_raw_index(id) }
pub(crate) fn release_notifier_handle(&self, handle: ContainerHandle) {
unsafe { self.notifiers.remove_with_handle(handle) }
}
}
16 changes: 8 additions & 8 deletions iceoryx2/src/service/dynamic_config/publish_subscribe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,19 @@ impl DynamicConfig {
self.subscribers.len()
}

pub(crate) fn add_subscriber_id(&self, id: UniqueSubscriberId) -> Option<u32> {
unsafe { self.subscribers.add_raw(id) }
pub(crate) fn add_subscriber_id(&self, id: UniqueSubscriberId) -> Option<ContainerHandle> {
unsafe { self.subscribers.add_with_handle(id) }
}

pub(crate) fn release_subscriber_id(&self, id: u32) {
unsafe { self.subscribers.remove_raw_index(id) }
pub(crate) fn release_subscriber_handle(&self, handle: ContainerHandle) {
unsafe { self.subscribers.remove_with_handle(handle) }
}

pub(crate) fn add_publisher_id(&self, id: UniquePublisherId) -> Option<u32> {
unsafe { self.publishers.add_raw(id) }
pub(crate) fn add_publisher_id(&self, id: UniquePublisherId) -> Option<ContainerHandle> {
unsafe { self.publishers.add_with_handle(id) }
}

pub(crate) fn release_publisher_id(&self, id: u32) {
unsafe { self.publishers.remove_raw_index(id) }
pub(crate) fn release_publisher_handle(&self, handle: ContainerHandle) {
unsafe { self.publishers.remove_with_handle(handle) }
}
}

0 comments on commit 85b9c9d

Please sign in to comment.