Is this the right approach for a thread-safe Queue class?

Question

I'm wondering if this is the right approach to writing a thread-safe queue in C++?

template <class T>
class Queue
{
public:

Queue() {}

void Push(T& a)
{
    m_mutex.lock();
    m_q.push_back(a);
    m_mutex.unlock();
}

T& Pop()
{
    m_mutex.lock();
    T& temp = m_q.pop();
    m_mutex.unlock();
    return temp;
}

private:
    std::queue<t> m_q;
    boost::mutex m_mutex;
};

You get the idea... I'm just wondering if this is the best approach. Thanks!

EDIT: Because of the questions I'm getting, I wanted to clarify that the mutex is a boost::mutex

Answer 1

I recommend using the Boost threading libraries to assist you with this.

Your code is fine, except that when you write code in C++ like

some_mutex.lock();
// do something
some_mutex.unlock();

then if the code in the // do something section throws an exception then the lock will never be released. The Boost library solves this with its classes such as lock_guard in which you initialize an object which acquires a lock in its constructor, and whose destructor releases the lock. That way you know that your lock will always be released. Other languages accomplish this through try/finally statements, but C++ doesn't support this construct.

In particular, what happens when you try to read from a queue with no elements? Does that throw an exception? If so, then your code would run into problems.

When trying to get the first element, you probably want to check if something is there, then go to sleep if not and wait until something is. This is a job for a condition object, also provided by the Boost library, though available at a lower level if you prefer.

Answer 2

Herb Sutter wrote an excellent article last year in Dr. Dobbs Journal, covering all of the major concerns for a thread-safe, lock-free, single-producer, single-consumer queue implementation. (Which made corrections over an implementation published the previous month.)

His followup article in the next issue tackled a more generic approach for a multi-user concurrent queue, along with a full discussion of potential pitfalls and performance issues.

There are a few more articles on similar concurrency topics.

Enjoy.