Multiple producer single consumer - Segmentation Fault

Question

I want to solve the multiple producer single consumer problem and, the following code shows one producer and one consumer thread in an infinite while loop, (I did this to test my code) but it gives wrong data output and then a segmentation fault after some time this is the code i tried:

#include<iostream>
#include<mutex>
#include<condition_variable>
#include<malloc.h>
#include<unistd.h>
#include<thread>
#include<assert.h>

#define QUEUE_SIZE 100

using namespace std;
struct node
{
    int data;
};
template<class T, unsigned long Q_SIZE = QUEUE_SIZE>
class NaiveQueue {
private:
    static const unsigned long Q_MASK = Q_SIZE - 1;
    unsigned long head_, tail_;
    std::condition_variable cond_empty_;
    std::condition_variable cond_overflow_;
    std::mutex mtx_;
    T **ptr_array_;

public:
    NaiveQueue(): head_(0), tail_(0)
    {
        ptr_array_ = (T **)::memalign(getpagesize(),Q_SIZE * sizeof(void *));
        assert(ptr_array_);
    }

    void push(T *x)
    {
        std::unique_lock<std::mutex> lock(mtx_);
        cond_overflow_.wait(lock, [this]() {
                    return tail_ + Q_SIZE > head_;
                });
        ptr_array_[head_++ & Q_MASK] = x;
        cond_empty_.notify_one();
    }

    T* pop()
    {
        std::unique_lock<std::mutex> lock(mtx_);
        cond_empty_.wait(lock, [this]() {
                    return tail_ < head_;
                });
        T *x = ptr_array_[tail_++ & Q_MASK];
        cond_overflow_.notify_one();
        return x;
    }
};
void producer(NaiveQueue<node> &obj)
{
    while(1)
    {
    node *temp;
    temp=new node();
    temp->data=10;
    obj.push(temp);
    cout<<"\nPushed : "<<10;
    }
}
void consumer(NaiveQueue<node> &obj)
{
    while(1)
    {
    node *temp;
    temp=NULL;
    temp=obj.pop();
    if(temp)
    {
        cout<<"\nRemoved : "<<temp->data;
        delete temp;
    }
    }
}
int main()
{
    NaiveQueue<node> obj;
    thread t1(producer,ref(obj));   
    thread c1(consumer,ref(obj));
    t1.join();
    c1.join();  
    return 0;
}

Produces this output:

Pushed : 
Removed : 1010
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Removed : 10
Removed : 10
Removed : 10
Removed : 10
Removed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 
Removed : 1010
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Removed : 10
Removed : 10
Removed : 10
Removed : 10
Removed : 10
Removed : -1140848048
Removed : -1140847792
Removed : -1140847760
Removed : -1140847856
Removed : -1140847824
Removed : -1140847792
Removed : -1140847760
Removed : -1140847856
Removed : -1140847824
Removed : -1140847792
Removed : -1140847760
Removed : -1140847856
Removed : -1140847824
Removed : -1140847792
Removed : -1140847760
Removed : -1140847856
Removed : -1140847824
Removed : -1140847792
Pushed : 10
Pushed : 
Removed : 1010
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Pushed : 10
Segmentation fault

Where did I go wrong? Why is -1140847856 being removed when it wasn't pushsed at the first place.

Rakib · Answer 1 · 2014-05-11T06:55:30.610

1

Your waiting condition and index calculation is wrong:

Let

Q_SIZE =10, Q_MASK=9, tail_=2,head_=5

then

tail_ + Q_SIZE > head_

will be true and it will push

head_++ & Q_MASK = 5 & 9 = 1th index

which is wrong.

When

tail_=0, head_= 9

then tail_ + Q_SIZE > head_ will be true and it will push

head_++ & Q_MASK = 9 & 9 = 9th index

But the queue is full, and the push should have waited until tail_ becomes 1, i.e at least one items is popped.

Also tail_ < head_ is wrong condition when popping.

Roughly you can do this (not tested)

void push(T *x)
{
    std::unique_lock<std::mutex> lock(mtx_);
    cond_overflow_.wait(lock, [this]() {
                return abs( head_ - tail_ )< Q_SIZE ;
            });
    ptr_array_[head_ ] = x;
    head_ = (++head) % Q_SIZE;
    cond_empty_.notify_one();
}

T* pop()
{
    std::unique_lock<std::mutex> lock(mtx_);
    cond_empty_.wait(lock, [this]() {
                return abs(head_ - tail_)>0;
            });
    T *x = ptr_array_[tail_];
    tail_ = (++tail_)% Q_SIZE;
    cond_overflow_.notify_one();
    return x;
}

edited May 11 '14 at 06:55

answered May 11 '14 at 05:34

Rakib

7,435
7
29
45

How to fix? Suggest changes please. Should I change them to Mod ? – May 11 '14 at 05:56
It works for by dong mod and maintaing number of elements in Queue separately... Is it ok to do so? – May 11 '14 at 06:06
Any suggestions for : http://stackoverflow.com/questions/23589828/locking-in-multiple-producer-single-consumer – May 11 '14 at 07:16
Hi @Superman, if this or any answer has solved your question please consider [accepting it](http://meta.stackexchange.com/q/5234/179419) by clicking the check-mark. This indicates to the wider community that you've found a solution and gives some reputation to both the answerer and yourself. There is no obligation to do this. – Rakib May 11 '14 at 07:19

Multiple producer single consumer - Segmentation Fault

1 Answers1