Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#129] fix missing connections #134

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
3cde266
[#129] Remove add with handle from container; fix ordering issue in c…
elfenpiff Feb 21, 2024
33c2e7d
[#129] Convert further tests to stripped API
elfenpiff Feb 21, 2024
d74d0bd
[#129] Add test concurrent test cases to pub/sub
elfenpiff Feb 21, 2024
c25df45
[#129] Fix problem where port handle was added to early
elfenpiff Feb 21, 2024
ceebc3a
[#129] Use copy_nonoverlapping instead of copy
elfenpiff Feb 21, 2024
d6fd33a
[#129] Use u64 as active index signal
elfenpiff Feb 21, 2024
9876b0f
[#129] Fix container race between add and update
elfenpiff Feb 21, 2024
6cc4952
[#129] Fix bug in test
elfenpiff Feb 21, 2024
d81369a
[#129] Introduce UsedChunkList
elfenpiff Feb 21, 2024
238b68f
[#129] Integrate used chunk list and add remove functionality
elfenpiff Feb 22, 2024
a8e523b
[#129] Refactor UsedChunkList so that it requires equi-distant offsets
elfenpiff Feb 22, 2024
685f11e
[#129] Integrate used chunk list into zero copy connection
elfenpiff Feb 22, 2024
cd71098
[#129] Integrate used chunk list into publisher
elfenpiff Feb 22, 2024
0e36d4e
[#129] Add single threaded connection tests
elfenpiff Feb 22, 2024
4223d18
[#129] Connection are cleaned up correctly
elfenpiff Feb 23, 2024
2a9d27c
[#129] Add method to acquire actual payload start address for allocator
elfenpiff Feb 23, 2024
883049c
[#129] ShmAllocator payload starts always at zero when aligned 1
elfenpiff Feb 23, 2024
d74d54b
[#129] Fix subscriber does not detect publisher reconnections
elfenpiff Feb 23, 2024
535024a
[#129] Fix clippy warnings, add release doc
elfenpiff Feb 23, 2024
75c0357
[#129] Fix unique shm name id in mac os
elfenpiff Feb 26, 2024
40da244
[#129] Add additional tests
elfenpiff Feb 26, 2024
bc71b94
[#129] Remove deallocation failure codes; ptr inside allocator check …
elfenpiff Mar 2, 2024
d651564
[#129] Convert expensive API misuse checks into debug_assert!
elfenpiff Mar 2, 2024
b8c9fa6
[#129] Adjust cirrus CI config
elfenpiff Mar 3, 2024
35b97b7
[#129] Deactivate FreeBSD temporarily
elfenpiff Mar 5, 2024
be5af47
[#129] Enable FreeBSD
elfenpiff Mar 5, 2024
5ce7401
[#129] Use only_if to disable entries in cirrus; rename start into st…
elfenpiff Mar 6, 2024
5302e91
[#129] Use callback approach when cleaning up UsedChunkList
elfenpiff Mar 6, 2024
ca5b1f0
[#129] Run debug_assertion tests only when they are enabled
elfenpiff Mar 6, 2024
cc5fca6
[#129] Fix ambiguity in tests
elfenpiff Mar 6, 2024
b787bf5
[#129] Introduce unaligned_mem_size; simplify const_memory_size funct…
elfenpiff Mar 6, 2024
e35af3f
[#129] Store SharedMemoryManagent in separate pointer instead of acqu…
elfenpiff Mar 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 39 additions & 30 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# Constants

iox2_task_timeout_template: &IOX2_TASK_TIMEOUT
timeout_in: 30m # This needs to be reasonable large in order not to run in a timeout in the docker image re-builds
timeout_in: 45m # This needs to be reasonable large in order not to run in a timeout in the docker image re-builds

elfenpiff marked this conversation as resolved.
Show resolved Hide resolved
iox2_common_cpu_and_memory_config_for_build_and_test_template: &IOX2_COMMON_CPU_AND_MEMORY_CONFIG_FOR_BUILD_AND_TEST
cpu: 2
Expand Down Expand Up @@ -82,7 +82,7 @@ iox2_common_build_release_template: &IOX2_COMMON_BUILD_RELEASE

iox2_common_build_and_test_no_doc_tests_release_template: &IOX2_COMMON_BUILD_AND_TEST_NO_DOC_TESTS_RELEASE
<<: *IOX2_COMMON_BUILD_RELEASE
test_script: cargo test --release --tests --workspace --no-fail-fast
test_script: RUSTFLAGS="-C debug-assertions" cargo test --release --tests --workspace --no-fail-fast

iox2_freebsd_setup_template: &IOX2_FREEBSD_SETUP
setup_script:
Expand Down Expand Up @@ -149,12 +149,16 @@ ubuntu_22_04_x64_stable_debug_task:
linux_only_doc_test_script: cargo test --doc -- --ignored

ubuntu_22_04_x64_beta_debug_task:
# TODO commented out due to limited CI time
only_if: false
depends_on: ubuntu_22_04_x64_stable_debug
<<: *IOX2_CONTAINER_UBUNTU_22_04_X64
set_toolchain_script: rustup default beta
<<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG

ubuntu_22_04_x64_nightly_debug_task:
# TODO commented out due to limited CI time
only_if: false
depends_on: ubuntu_22_04_x64_beta_debug
allow_failures: true
<<: *IOX2_CONTAINER_UBUNTU_22_04_X64
Expand All @@ -175,12 +179,13 @@ ubuntu_22_04_x64_stable_release_task:

# Pipeline 4

### TODO commented out due to limited CI time
### ubuntu_22_04_aarch64_min_version_debug_task:
### depends_on: preflight_check
### <<: *IOX2_CONTAINER_UBUNTU_22_04_AARCH64
### set_toolchain_script: rustup default 1.70.0
### <<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG
ubuntu_22_04_aarch64_min_version_debug_task:
# TODO commented out due to limited CI time
only_if: false
depends_on: preflight_check
<<: *IOX2_CONTAINER_UBUNTU_22_04_AARCH64
set_toolchain_script: rustup default 1.70.0
<<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG

ubuntu_22_04_aarch64_stable_debug_task:
depends_on: preflight_check
Expand All @@ -190,6 +195,8 @@ ubuntu_22_04_aarch64_stable_debug_task:
<<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG

ubuntu_22_04_aarch64_beta_debug_task:
# TODO commented out due to limited CI time
only_if: false
depends_on: ubuntu_22_04_aarch64_stable_debug
allow_failures: true
<<: *IOX2_CONTAINER_UBUNTU_22_04_AARCH64
Expand Down Expand Up @@ -260,16 +267,17 @@ windows_server_2019_x64_stable_release_task:

# Pipeline 9

### TODO commented out due to limited CI time
### freebsd_x64_min_version_debug_task:
### depends_on: preflight_check
### <<: *IOX2_CONTAINER_FREEBSD_X64
### env:
### PATH: /root/.cargo/bin:$PATH
### HOME: /root # must be set manually to '/root' or 'rustup' will throw an error
### <<: *IOX2_FREEBSD_SETUP
### set_toolchain_script: rustup default 1.70.0
### <<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG
freebsd_x64_min_version_debug_task:
# TODO commented out due to limited CI time
only_if: false
depends_on: preflight_check
<<: *IOX2_CONTAINER_FREEBSD_X64
env:
PATH: /root/.cargo/bin:$PATH
HOME: /root # must be set manually to '/root' or 'rustup' will throw an error
<<: *IOX2_FREEBSD_SETUP
set_toolchain_script: rustup default 1.70.0
<<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG

freebsd_x64_stable_debug_task:
depends_on: preflight_check
Expand All @@ -288,18 +296,19 @@ freebsd_x64_stable_debug_task:

# Pipeline 10

### TODO commented out due to limited CI time
### macos_aarch64_min_version_debug_task:
### depends_on: preflight_check
### <<: *IOX2_CONTAINER_MACOS_AARCH64
### env:
### PATH: /Users/admin/.cargo/bin:$PATH
### setup_script:
### - uname -a
### - curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal --no-modify-path
### - rustup component add clippy rustfmt
### set_toolchain_script: rustup default 1.70.0
### <<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG
macos_aarch64_min_version_debug_task:
# TODO commented out due to limited CI time
only_if: false
depends_on: preflight_check
<<: *IOX2_CONTAINER_MACOS_AARCH64
env:
PATH: /Users/admin/.cargo/bin:$PATH
setup_script:
- uname -a
- curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile minimal --no-modify-path
- rustup component add clippy rustfmt
set_toolchain_script: rustup default 1.70.0
<<: *IOX2_COMMON_BUILD_AND_TEST_DEBUG

macos_aarch64_stable_debug_task:
depends_on: preflight_check
Expand Down
14 changes: 14 additions & 0 deletions BEST_PRACTICES.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@
}
```

* If a panic error check is expensive and located on the hot path then use `debug_assert!`.
* Use it only when the panic check prevents API misuse and the misbehavior can be easily
detected with unit tests

```rust
// bad
if expensive_check() {
fatal_panic!("oh no ...");
}

// good
debug_assert!(expensive_check(), "oh no ...");
```

### Re-Exports And Preludes

* The most common functionality of a construct shall be fully usable by only
Expand Down
6 changes: 6 additions & 0 deletions doc/release-notes/iceoryx2-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* New constructs from [#123](https://github.com/eclipse-iceoryx/iceoryx2/issues/123)
* Introduce semantic string `iceoryx2-bb-system-types::base64url`
* Introduce `iceoryx2-cal::hash::HashValue` that contains the result of a hash
* Port `UsedChunkList` from iceoryx1 [#129](https://github.com/eclipse-iceoryx/iceoryx2/issues/129)
* Performance improvements, especially for AMD CPUs [#136](https://github.com/eclipse-iceoryx/iceoryx2/issues/136)

### Bugfixes
Expand All @@ -26,8 +27,12 @@
* Fix `CreationMode::OpenOrCreate` in `iceoryx2-bb-posix::SharedMemory`
* Add missing memory synchronization to posix shm zero copy connection
* Remove retrieve buffer full check from zero copy connection - sender had insufficient infos available
* Fix data race in `iceoryx2-bb-lock-free::mpmc::Container`
* Fix insufficient memory reordering protection in `spsc::Queue::push` and `spsc::Queue::pop` [#119](https://github.com/eclipse-iceoryx/iceoryx2/issues/119)
* Fix data race due to operation reordering in `spmc::UnrestrictedAtomic::load` [#125](https://github.com/eclipse-iceoryx/iceoryx2/issues/125)
* Fix broken `Publisher|Subscriber::populate_{subscriber|publisher}_channels()` [#129](https://github.com/eclipse-iceoryx/iceoryx2/issues/129)
* Fix failing reacquire of delivered samples in the zero copy receive channel [#130](https://github.com/eclipse-iceoryx/iceoryx2/issues/130)
* Fix receiving of invalid samples when subscriber is connected [#131](https://github.com/eclipse-iceoryx/iceoryx2/issues/131)
* Fixes for FreeBSD 14.0 [#140](https://github.com/eclipse-iceoryx/iceoryx2/issues/140)
* Fix segfault in `iceoryx2-pal-posix;:shm_list()` caused by `sysctl`
* Adjust test to handle unordered event notifications
Expand All @@ -38,6 +43,7 @@

* Replace `iceoryx2::service::Service` with `iceoryx2::service::Details` [#100](https://github.com/eclipse-iceoryx/iceoryx2/issues/100)
* Remove `'config` lifetime from all structs [#100](https://github.com/eclipse-iceoryx/iceoryx2/issues/100)
* Remove `UniqueIndex` returning method from `iceoryx2-bb-lock-free::mpmc::Container`, cannot be implemented correctly in our context [#116](https://github.com/eclipse-iceoryx/iceoryx2/issues/116)

### Workflow

Expand Down
17 changes: 10 additions & 7 deletions iceoryx2-bb/container/src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ pub type RelocatableQueue<T> = details::Queue<T, RelocatablePointer<MaybeUninit<
mod details {
use std::marker::PhantomData;

use iceoryx2_bb_elementary::math::unaligned_mem_size;

use super::*;
/// **Non-movable** relocatable queue with runtime fixed size capacity.
#[repr(C)]
Expand Down Expand Up @@ -184,18 +186,19 @@ mod details {
}

impl<T, PointerType: PointerTrait<MaybeUninit<T>>> Queue<T, PointerType> {
#[inline(always)]
fn verify_init(&self, source: &str) {
if !self
.is_initialized
.load(std::sync::atomic::Ordering::Relaxed)
{
fatal_panic!(from source, "Undefined behavior - the object was not initialized with 'init' before.");
}
debug_assert!(
self.is_initialized
.load(std::sync::atomic::Ordering::Relaxed),
"From: {}, Undefined behavior - the object was not initialized with 'init' before.",
source
);
}

/// Returns the required memory size for a queue with a specified capacity
pub const fn const_memory_size(capacity: usize) -> usize {
std::mem::size_of::<T>() * capacity + std::mem::align_of::<T>() - 1
unaligned_mem_size::<T>(capacity)
}

/// Returns true if the queue is empty, otherwise false
Expand Down
19 changes: 11 additions & 8 deletions iceoryx2-bb/container/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ use std::{
};

use iceoryx2_bb_elementary::{
math::align_to, pointer_trait::PointerTrait, relocatable_container::RelocatableContainer,
math::{align_to, unaligned_mem_size},
pointer_trait::PointerTrait,
relocatable_container::RelocatableContainer,
relocatable_ptr::RelocatablePointer,
};
use iceoryx2_bb_log::{fail, fatal_panic};
Expand Down Expand Up @@ -175,18 +177,19 @@ impl<T: PartialEq> PartialEq for Vec<T> {
impl<T: Eq> Eq for Vec<T> {}

impl<T> Vec<T> {
#[inline(always)]
fn verify_init(&self, source: &str) {
if !self
.is_initialized
.load(std::sync::atomic::Ordering::Relaxed)
{
fatal_panic!(from source, "Undefined behavior - the object was not initialized with 'init' before.");
}
debug_assert!(
self.is_initialized
.load(std::sync::atomic::Ordering::Relaxed),
"From: {}, Undefined behavior - the object was not initialized with 'init' before.",
source
);
}

/// Returns the required memory size for a vec with a specified capacity
pub const fn const_memory_size(capacity: usize) -> usize {
std::mem::size_of::<T>() * capacity + std::mem::align_of::<T>() - 1
unaligned_mem_size::<T>(capacity)
}

/// Returns the capacity of the vector
Expand Down
11 changes: 1 addition & 10 deletions iceoryx2-bb/elementary/src/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ pub enum AllocationError {
/// Failures caused by [`Allocator::grow()`] or [`Allocator::grow_zeroed()`].
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum AllocationGrowError {
ProvidedPointerNotContainedInAllocator,
GrowWouldShrink,
SizeIsZero,
OutOfMemory,
Expand All @@ -39,20 +38,12 @@ pub enum AllocationGrowError {
/// Failures caused by [`Allocator::shrink()`].
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum AllocationShrinkError {
ProvidedPointerNotContainedInAllocator,
ShrinkWouldGrow,
SizeIsZero,
AlignmentFailure,
InternalError,
}

/// Failures caused by [`BaseAllocator::deallocate()`].
#[derive(Debug, PartialEq, Eq, Copy, Clone)]
pub enum DeallocationError {
ProvidedPointerNotContainedInAllocator,
InternalError,
}

/// The most minimalistic requirement for an allocator
pub trait BaseAllocator {
/// Allocates a memory chunk with the properties provided in layout and either
Expand Down Expand Up @@ -82,7 +73,7 @@ pub trait BaseAllocator {
/// * `layout` must have the same value as in the allocation or, when the memory was
/// resized, the same value as it was resized to
///
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout) -> Result<(), DeallocationError>;
unsafe fn deallocate(&self, ptr: NonNull<u8>, layout: Layout);
}

/// Allocator with grow and shrink features.
Expand Down
7 changes: 1 addition & 6 deletions iceoryx2-bb/elementary/src/bump_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,7 @@ impl BaseAllocator for BumpAllocator {
}
}

unsafe fn deallocate(
&self,
_ptr: std::ptr::NonNull<u8>,
_layout: std::alloc::Layout,
) -> Result<(), crate::allocator::DeallocationError> {
unsafe fn deallocate(&self, _ptr: std::ptr::NonNull<u8>, _layout: std::alloc::Layout) {
self.pos.store(self.start, Ordering::Relaxed);
Ok(())
}
}
5 changes: 5 additions & 0 deletions iceoryx2-bb/elementary/src/math.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@

//! Contains simplistic math functions.

/// Returns the required memory size when alignment adjustments are taken into account
pub const fn unaligned_mem_size<T>(array_capacity: usize) -> usize {
core::mem::size_of::<T>() * array_capacity + core::mem::align_of::<T>() - 1
}

/// Aligns value to alignment. It increments value to the next multiple of alignment.
pub const fn align(value: usize, alignment: usize) -> usize {
if value % alignment == 0 {
Expand Down
Loading
Loading