0

I'm trying to implement a producer-consumer pattern. I did my homework but still couldn't be sure about it. The implementation is as follows:

boost::mutex m_mutex;
boost::container::deque<T> m_buffer;
boost::condition_variable fifo_loaded;

T pop(void)
{
    boost::mutex::scoped_lock lock(m_mutex);

    while (m_buffer.empty())
    {  
        fifo_loaded.wait(lock); // As i understand, it releases the mutex, 
                                   and whenever it is notified, 
                                   gets it back again   
    }       
    T tmp = m_buffer.front();       
    m_buffer.pop_front();       
    return tmp; 
}


void push(const T &newElem) 
{       
    boost::mutex::scoped_lock lock(m_mutex);        
    m_buffer.push_back(newElem);        
    lock.unlock();      
    fifo_loaded.notify_one();   
}

And the producer-consumer pair is like below. Is it OK or do i need synchronization in here too ?

void produce_thread()
{
    while(true)
    {
        double data = generate_data();  
        m_buffer.push(data);
    }   
}

void consume_thread()
{
    while (true)
    {
        double data = m_buffer.pop();
        process_data(data);
    }
}

void start_system()
{
    boost::thread* thread_a = new boost::thread(capture_thread);
    boost::thread* thread_b = new boost::thread(process_thread);
}

And how can i stop the threads manually ? Is it OK to manage it with a bool like the one below ?

bool enabled;

void produce_thread()
{
    while(enabled)
    {
        // Do stuff
    }   
}

void consume_thread()
{
    while (enabled)
    {
        // Do stuff
    }
}
TemplateRex
  • 69,038
  • 19
  • 164
  • 304
Murat Şeker
  • 1,651
  • 1
  • 16
  • 29
  • You need no more synchronization, and use [Boost.Atomic](http://www.boost.org/doc/libs/1_54_0/doc/html/atomic/interface.html#atomic.interface.interface_atomic_object.interface_atomic_generic) `boost::atomic` instead of `bool` for stop-flag. – yohjp Aug 03 '13 at 13:07

1 Answers1

0

Your example does not actually use the threadsafe push and pop functions you wrote, it calls the deque's push and pop function directly. If it did use those, then it would be threadsafe and producer_thread/consumer_thread would not need additional synchronization

That being said, yohjp is correct. You cannot just have an unprotected boolean like "enabled." The C++11 spec defines a data rate to be any time where one thread is able to write to a value while another is able to read or write to it. This definition matches that used by compilers before C++11 (it just makes it official).

enabled either needs to be an atomic boolean, such as boost::atomic, or you need an additional mutex which protects 'enabled,' with the rule that you cannot read nor write to enabled unless you hold the mutex.

Cort Ammon
  • 10,221
  • 31
  • 45
  • I didn't understand what you said in your first paragraph because of my poor English i think. Did you mean that my example doesn't use my own push and pop methods but calls deque's own push and pop ? But i use these : "m_buffer.push(data);" "double data = m_buffer.pop();" – Murat Şeker Nov 13 '13 at 16:11