Merge pull request #88 from elfenpiff/iox2-87-fix-issue-in-lock-free-…

…queue [#87] Fix undefined behavior in spsc {queue|index_queue}
eclipse-iceoryx · Jan 20, 2024 · ba45191 · ba45191
2 parents 5d91cc7 + 287cb16
commit ba45191
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 2 deletions.
diff --git a/doc/release-notes/iceoryx2-unreleased.md b/doc/release-notes/iceoryx2-unreleased.md
@@ -16,6 +16,7 @@
 
  * Fix `clock_nanosleep` on macOS [#80](https://github.com/eclipse-iceoryx/iceoryx2/issues/80)
  * Fix broken `sighandler_t` translation [#81](https://github.com/eclipse-iceoryx/iceoryx2/issues/81)
+ * Fix undefined behavior in `spsc::{queue|index_queue}` [#87](https://github.com/eclipse-iceoryx/iceoryx2/issues/87)
 
 ### Refactoring
 

diff --git a/iceoryx2-bb/lock-free/src/spsc/index_queue.rs b/iceoryx2-bb/lock-free/src/spsc/index_queue.rs
@@ -316,6 +316,9 @@ pub mod details {
             }
 
             let value = unsafe { *self.at(read_position) };
+            // prevent that `out` and `read_position` statements are reordered according to
+            // the AS-IF rule.
+            core::sync::atomic::fence(Ordering::AcqRel);
             self.read_position
                 .store(read_position + 1, Ordering::Relaxed);
 

diff --git a/iceoryx2-bb/lock-free/src/spsc/queue.rs b/iceoryx2-bb/lock-free/src/spsc/queue.rs
@@ -215,8 +215,11 @@ impl<T: Copy, const CAPACITY: usize> Queue<T, CAPACITY> {
                         .as_ptr()
                 };
 
+                // prevent that `out` and `read_position` statements are reordered according to
+                // the AS-IF rule.
+                core::sync::atomic::fence(Ordering::AcqRel);
                 self.read_position
-                    .store(current_read_pos + 1, Ordering::Release);
+                    .store(current_read_pos + 1, Ordering::Relaxed);
 
                 Some(out)
             }

diff --git a/iceoryx2-bb/lock-free/src/spsc/safely_overflowing_index_queue.rs b/iceoryx2-bb/lock-free/src/spsc/safely_overflowing_index_queue.rs
@@ -302,7 +302,12 @@ pub mod details {
         ///  * It has to be ensured that the memory is initialized with
         ///    [`SafelyOverflowingIndexQueue::init()`].
         pub unsafe fn push(&self, value: usize) -> Option<usize> {
-            let write_position = self.write_position.load(Ordering::Relaxed);
+            ////////////////
+            // SYNC POINT R
+            ////////////////
+            // required when push in overflow case is called non-concurrently from a different
+            // thread
+            let write_position = self.write_position.load(Ordering::Acquire);
             let read_position = self.read_position.load(Ordering::Relaxed);
             let is_full = write_position == read_position + self.capacity;