Skip to content

Concurrent writes to Ring_Buffer are not thread-safe. #3

@SerenGTI

Description

@SerenGTI

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions