HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Lock-free ringbuffer with multiple readers in C++11

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
freewithreadersringbuffermultiplelock

Problem

Basic Info

I needed a lock-free ringbuffer with multiple readers (but one writer). However, I did not want the writer to check all readers every time in order to prevent an overrun. Thus, I decided to split the buffer in two halves. If the write pointer is in one half, it is only allowed to advance to the next half if all readers are already in the same half as the writer.

This is realized by using an atomic counter named readers_left, which counts the number of readers left in the previous half.

The code to this question can be found here and the most recent version here. In the following, I'll try to explain the code bit by bit.

Classes

This class is the base for a reader and writer:

class ringbuffer_common_t
{
private:
    static std::size_t calc_size(std::size_t sz);
protected:
    const std::size_t size; //!< buffer size (2^n for some n)
    const std::size_t size_mask; //!< = size - 1
public:
    ringbuffer_common_t(std::size_t sz);
};


Next comes the writer, just called ringbuffer_t. It contains the only two atomics:

class ringbuffer_t : protected ringbuffer_common_t
{
    std::atomic w_ptr; //! readers_left;
    std::size_t num_readers = 0; //!> 1;
    }

    //! returns number of bytes that can be written at least
    std::size_t write_space() const;
    //! writes max(cnt, write_space) of src into the buffer
    //! @return number of bytes successfully written
    std::size_t write(const char *src, size_t cnt);
};


And finally, the reader class. It contains a helper class read_sequence_t. If you want to read using this reader, the reader returns such a read_sequence_t object. Only one read per reader at a time is allowed (this is not checked).

```
class ringbuffer_reader_t : protected ringbuffer_common_t
{
const char* const buf;
ringbuffer_t* const ref;
std::size_t read_ptr = 0; //!read_ptr + idx) &
reader_ref->size_mask));
}

std::size_t size() const { return range; }
};

Solution

Barriers missing

I know this is an old question, but I started looking at the "lock-free" tag and came across this unanswered question. I believe that your code is unsafe because it is lacking the proper memory barriers. For example, in this code snippet:

std::copy_n(src, n1, &(buf[w]));
w = (w + n1) & size_mask;
// update so readers are already informed:
w_ptr.store(w, std::memory_order_relaxed);


The std::memory_order_relaxed doesn't provide any protection against store reordering. So the store to w_ptr can be reordered to be before the copy to the buffer. Later on, the reader could see an updated write pointer and try to read from the buffer before the buffer contents have been stored. You should use std::memory_order_release to prevent this.

You have similar problems on the reader side, where you should use std::memory_order_acquire instead of std::memory_order_relaxed.
Why do my tests work?

Most likely, you are testing this on an x86 based system. The x86 architecture has certain memory ordering guarantees that make most memory barriers unnecessary. So your code will in fact operate correctly when run on an x86 architecture.

Code Snippets

std::copy_n(src, n1, &(buf[w]));
w = (w + n1) & size_mask;
// update so readers are already informed:
w_ptr.store(w, std::memory_order_relaxed);

Context

StackExchange Code Review Q#84715, answer score: 3

Revisions (0)

No revisions yet.