Skip to content

Commit

Permalink
iox-eclipse-iceoryx#119 fixing memory sync issue in fifo
Browse files Browse the repository at this point in the history
Signed-off-by: Christian Eltzschig <christian.eltzschig2@de.bosch.com>
  • Loading branch information
elfenpiff committed Jun 2, 2020
1 parent 5dc4280 commit 6aca3db
Showing 1 changed file with 16 additions and 7 deletions.
23 changes: 16 additions & 7 deletions iceoryx_utils/include/iceoryx_utils/internal/concurrent/fifo.inl
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ inline bool FiFo<ValueType, Capacity>::push(const ValueType& f_param_r)
}
else
{
m_data[m_write_pos.load(std::memory_order_relaxed) % Capacity] = f_param_r;
auto currentWritePos = m_write_pos.load(std::memory_order_relaxed);
m_data[currentWritePos % Capacity] = f_param_r;

// m_write_pos must be increased after writing the new value otherwise
// it is possible that the value is read by pop while it is written
m_write_pos.fetch_add(1u, std::memory_order_acq_rel);
// it is possible that the value is read by pop while it is written.
// this fifo is a single producer, single consumer fifo therefore
// store is allowed.
m_write_pos.store(currentWritePos + 1, std::memory_order_release);
return true;
}
}
Expand All @@ -51,17 +54,23 @@ inline bool FiFo<ValueType, Capacity>::empty() const
template <class ValueType, uint32_t Capacity>
inline cxx::optional<ValueType> FiFo<ValueType, Capacity>::pop()
{
if (empty())
bool isEmpty = (m_read_pos.load(std::memory_order_relaxed) ==
// we are not allowed to use the empty method since we have to sync with
// the producer pop - this is done here
m_write_pos.load(std::memory_order_acquire));
if (isEmpty)
{
return cxx::nullopt_t();
}
else
{
ValueType out = m_data[m_read_pos.load(std::memory_order_acquire) % Capacity];
auto currentReadPos = m_read_pos.load(std::memory_order_relaxed);
ValueType out = m_data[currentReadPos % Capacity];

// m_read_pos must be increased after reading the pop'ed value otherwise
// it is possible that the pop'ed value is overwritten by push while it is read
m_read_pos.fetch_add(1, std::memory_order_relaxed);
// it is possible that the pop'ed value is overwritten by push while it is read.
// Implementing a single consumer fifo here allows us to use store.
m_read_pos.store(currentReadPos + 1, std::memory_order_relaxed);
return out;
}
}
Expand Down

0 comments on commit 6aca3db

Please sign in to comment.