A multi-threaded Producer Consumer with C++11 I am trying to learn concurrent programming in C++11. I tried to write code for a classic producer consumer concurrency problem. Would you please review and make any comments about it?

#include &lt;iostream&gt;
#include &lt;thread&gt;
#include &lt;deque&gt;
#include &lt;mutex&gt;
#include &lt;chrono&gt;
#include &lt;condition_variable&gt;

using std::deque;
std::mutex mu,cout_mu;
std::condition_variable cond;

class Buffer
{
public:
    void add(int num) {
        while (true) {
            std::unique_lock&lt;std::mutex&gt; locker(mu);
            cond.wait(locker, [this](){return buffer_.size() &lt; size_;});
            buffer_.push_back(num);
            locker.unlock();
            cond.notify_all();
            return;
        }
    }
    int remove() {
        while (true)
        {
            std::unique_lock&lt;std::mutex&gt; locker(mu);
            cond.wait(locker, [this](){return buffer_.size() &gt; 0;});
            int back = buffer_.back();
            buffer_.pop_back(); 
            locker.unlock();
            cond.notify_all();
            return back;
        }
    }
    Buffer() {}
private:
    deque&lt;int&gt; buffer_;
    const unsigned int size_ = 10;
};

class Producer
{
public:
    Producer(Buffer* buffer)
    {
        this-&gt;buffer_ = buffer;
    }
    void run() {
        while (true) {
            int num = std::rand() % 100;
            buffer_-&gt;add(num);
            cout_mu.lock();
            std::cout &lt;&lt; "Produced: " &lt;&lt; num &lt;&lt; std::endl;
            std::this_thread::sleep_for(std::chrono::milliseconds(50));
            cout_mu.unlock();
        }
    }
private:
    Buffer *buffer_;
};

class Consumer
{
public:
    Consumer(Buffer* buffer)
    {
        this-&gt;buffer_ = buffer;
    }
    void run() {
        while (true) {
            int num = buffer_-&gt;remove();
            cout_mu.lock();
            std::cout &lt;&lt; "Consumed: " &lt;&lt; num &lt;&lt; std::endl;
            std::this_thread::sleep_for(std::chrono::milliseconds(50));
            cout_mu.unlock();
        }
    }
private:
    Buffer *buffer_;
};

int main() {
    Buffer b;
    Producer p(&amp;b);
    Consumer c(&amp;b);

    std::thread producer_thread(&amp;Producer::run, &amp;p);
    std::thread consumer_thread(&amp;Consumer::run, &amp;c);

    producer_thread.join();
    consumer_thread.join();
    getchar();
    return 0;
}
Prefer Reference over Pointer

Since the producer and consumer must have a buffer you should pass it by reference (rather than pointer). This also makes sure there is no confusion over ownership of the buffer (the owner of a pointer is responsible for deleting it). By using a RAW pointer you can not tell the owner but by using a reference you are explicitly stating that you are not passing ownership.

class Producer
{
    Buffer&amp;  buffer_;
public:
    Producer(Buffer&amp; buffer)
        : buffer_(buffer)
    {}


Prefer to avoid this->

When you use this-&gt; it means you have scoping issues with your variables (which is a code smell for bad design). Use accurate variable names so that there is no confusion on where the variables belongs.

Member variables over globals.

Looks like your encapsulation of the mu and cond is wrong.

std::mutex mu,cout_mu;


Because they are global, you use a single mutex/condition for all buffer objects. This looks like a design flaw. It seems like the mutex/condition belongs to the class so that you are just locking the buffer you are manipulating (so you can use multiple buffers in the same application).

Sleeping inside lock

        cout_mu.lock();
        std::cout &lt;&lt; "Produced: " &lt;&lt; num &lt;&lt; std::endl;
        std::this_thread::sleep_for(std::chrono::milliseconds(50));
        cout_mu.unlock();


@Morwenn already suggested using scoped lock to make sure the mutex were correctly locked and unlocked. But I would also move the sleep outside the lock. This causes the current thread to stall but the other thread can not continue as the lock is still held while it is sleeping.

        {
            std::unique_lock&lt;std::mutex&gt; locker(cout_mu);
            std::cout &lt;&lt; "Produced: " &lt;&lt; num &lt;&lt; std::endl;
        }
        std::this_thread::sleep_for(std::chrono::milliseconds(50));


Prefer '\n' over std::endl

std::cout &lt;&lt; "Produced: " &lt;&lt; num &lt;&lt; std::endl;


The difference between the two is a flush. Usually you do not want to manually flush (the stream have good flushing techniques built in). So it is usually best to let the streams flush themselves when appropriate.



In answer to comments:

class Buffer
{
public:
    void add(int num) {
        while (true) {
            std::unique_lock&lt;std::mutex&gt; locker(mu);
            cond.wait(locker, [this](){return buffer_.size() &lt; size_;});
            buffer_.push_back(num);
            locker.unlock();
            cond.notify_all();
            return;
        }
    }
    int remove() {
        while (true)
        {
            std::unique_lock&lt;std::mutex&gt; locker(mu);
            cond.wait(locker, [this](){return buffer_.size() &gt; 0;});
            int back = buffer_.back();
            buffer_.pop_back();
            locker.unlock();
            cond.notify_all();
            return back;
        }
    }
    Buffer() {}
private:
   // Add them as member variables here
    std::mutex mu;
    std::condition_variable cond;

   // Your normal variables here
    deque&lt;int&gt; buffer_;
    const unsigned int size_ = 10;
};