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

One reader / one writer no-memory-allocation lock-free ring-buffer

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

Problem

I have two threads - one thread queues items another thread dequeues. dequeue is fast enough and so we can assume that queue is never contains more than 65536 items.

C++ doesn't contain "ring-buffer." Boost has one, but it doesn't allow "reuse" of elements. So I wrote my own ring-buffer, which I think is very fast and requires no memory allocation. The code below has not been tested but it should show the general idea.

Do you find my code fine? Can you suggest how I can improve it or if there's something I can use instead of it?
#include
#include
#include

#include 
#include 

template class ArrayPool
{
public:
    ArrayPool() {
    };

    ~ArrayPool(void) {
    };

    // reader thread
    bool IsEmpty() {
        return curReadNum == curWriteNum;
    }

    // reader thread
    T* TryGet()
    {
        if (curReadNum == curWriteNum)
        {
            return NULL;
        }
        T* result = &storage[curReadNum & MASK];
        ++curReadNum;
        return result;
    }

    // writer thread
    T* Obtain() {
        return &storage[curWriteNum & MASK];
    }

    // writer thread
    void Commit()
    {
                    // Ensure storage is written before mask is incremented ?
                    // insert memory barrier ?
        ++curWriteNum;
        if (curWriteNum - curReadNum > length)
        {
            std::cout  length! "  "  pool;

void ReadThread() {
    myStruct* entry;
    while(true) {
        while ((entry = pool.TryGet()) != NULL) {
            std::cout value value = id;
    pool.Commit();
    std::cout << "Commited value! " << id << std::endl;
}

int main( void )
{
    boost::thread readThread = boost::thread(&ReadThread);
    boost::thread writeThread;
    for (int i = 0; i < 100; i++) {
        writeThread = boost::thread(&WriteThread, i);
    }
    writeThread.join();
    return 0;
}

Solution

-
there is absolutely no thread safety constructs, this means that you'll see a lot of interesting race conditions and memory inconsistencies

-
just assuming dequeues happen fast enough does not mean they will in production

-
you return T by value in Obtain(), instead of by reference so you can't actually write into it

-
Obtain is a bad name, it sounds like it gets the next value like TryGet does

-
memcpy is a bad idea to use to store the values if there is ever a custom copy constructor and a destructor

-
that said Obtain->store->Commit can be put into a single Put(T*) member function

-
eventually curReadNum and curWriteNum will overflow, it's not a real problem here because your length is a power of 2 and they are unsigned

Context

StackExchange Code Review Q#38815, answer score: 11

Revisions (0)

No revisions yet.