-
Notifications
You must be signed in to change notification settings - Fork 5
Open
Description
The following is an excerpt from ring_buffer.cc, see
https://github.com/jpcima/ring-buffer/blob/e0c7b5ee052ab67cecdf26ca20dda77363506a5d/sources/ring_buffer.cc#L127C1-L147C2
template <bool Atomic>
bool Ring_Buffer_Ex<Atomic>::putbytes_(const void *data, size_t len)
{
if (len == 0)
return true;
if (size_free() < len)
return false;
const size_t wp = atomic_load_maybe(wp_, std::memory_order_relaxed);
const size_t cap = cap_;
const uint8_t *src = (const uint8_t *)data;
uint8_t *dst = rbdata_.get();
const size_t taillen = std::min(len, cap - wp);
std::copy_n(src, taillen, &dst[wp]);
std::copy_n(src + taillen, len - taillen, dst);
atomic_store_maybe(wp_, (wp + len < cap) ? (wp + len) : (wp + len - cap), std::memory_order_release);
return true;
}I believe concurrent calls to Ring_Buffer::put(...) can cause a data race on the same memory region currently pointed to by wp_. There is no protection against two threads concurrently executing
const size_t wp = atomic_load_maybe(wp_, std::memory_order_relaxed);and subsequently writing to the same address &dst[wp]. Specifically, the update of wp is not atomic. I believe this can lead to a lost update or corrupted data. As such, Ring_Buffer is not thread-safe.
xiaoxiayu
Metadata
Metadata
Assignees
Labels
No labels