Skip to content

Commit

Permalink
Merge pull request #2197 from elBoberido/iox-2196-fix-memory-order-in…
Browse files Browse the repository at this point in the history
…-mpmc-loffli-fence-synchronization

iox-#2196 Fix memory order in 'MpmcLoFFLi' fence synchronization
  • Loading branch information
elBoberido authored Feb 19, 2024
2 parents a21603a + e4b543b commit ee7ddcb
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
3 changes: 2 additions & 1 deletion doc/website/release-notes/iceoryx-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@
- Add public functions to create an 'access_rights' object from integer values [#2108](https://github.com/eclipse-iceoryx/iceoryx/issues/2108)
- Fix `historyRequest` may be larger than `queueCapacity` during creating a subscriber [#2121](https://github.com/eclipse-iceoryx/iceoryx/issues/2121)
- Unable to acquire file status due to an unknown failure [#2023](https://github.com/eclipse-iceoryx/iceoryx/issues/2023)
- Bug in 'ListenerImpl' [#2137](https://github.com/eclipse-iceoryx/iceoryx/issues/2137)
- Bug in `ListenerImpl` [#2137](https://github.com/eclipse-iceoryx/iceoryx/issues/2137)
- Wrong memory order in `MpmcLoFFLi` fence synchronization [#2196](https://github.com/eclipse-iceoryx/iceoryx/issues/2196)

**Refactoring:**

Expand Down
9 changes: 7 additions & 2 deletions iceoryx_hoofs/concurrent/buffer/source/mpmc_loffli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ bool MpmcLoFFLi::pop(Index_t& index) noexcept
m_nextFreeIndex.get()[index] = m_invalidIndex;

/// we need to synchronize m_nextFreeIndex with push so that we can perform a validation
/// check right before push to avoid double free's
/// check right before push to avoid double free's;
/// a simple fence without explicit atomic store should be sufficient since the index needs to be transferred to
/// another thread and the most simple method would be a relaxed store of the index in the 'pop' thread and a
/// relaxed load of the same atomic in the 'push' thread which would be the atomic in the fence to fence
/// synchronization; other mechanism would involve stronger synchronizations and implicitly also synchronize
/// m_nextFreeIndex
std::atomic_thread_fence(std::memory_order_release);

return true;
Expand All @@ -83,7 +88,7 @@ bool MpmcLoFFLi::pop(Index_t& index) noexcept
bool MpmcLoFFLi::push(const Index_t index) noexcept
{
/// we synchronize with m_nextFreeIndex in pop to perform the validity check
std::atomic_thread_fence(std::memory_order_release);
std::atomic_thread_fence(std::memory_order_acquire);

/// we want to avoid double free's therefore we check if the index was acquired
/// in pop and the push argument "index" is valid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ bool UsedChunkList<Capacity>::insert(mepoo::SharedChunk chunk) noexcept
// set freeListHead to the next free entry
m_freeListHead = nextFree;

/// @todo iox-#623 can we do this cheaper with a global fence in cleanup?
m_synchronizer.clear(std::memory_order_release);
return true;
}
Expand Down Expand Up @@ -92,7 +91,6 @@ bool UsedChunkList<Capacity>::remove(const mepoo::ChunkHeader* chunkHeader, mepo
m_listIndices[current] = m_freeListHead;
m_freeListHead = current;

/// @todo iox-#623 can we do this cheaper with a global fence in cleanup?
m_synchronizer.clear(std::memory_order_release);
return true;
}
Expand Down

0 comments on commit ee7ddcb

Please sign in to comment.