CODEWITHSUNDEEP
c++multithreadingpthreadsqueuemutex
jensph
jensph

Reputation: 785

Another thread safe queue implementation

I have a class, Queue, that I tried to make thread safe. It has these three member variables:

    std::queue<T>   m_queue;
    pthread_mutex_t m_mutex;
    pthread_cond_t  m_condition;

and a push and pop implemented as:

    template<class T> void Queue<T>::push(T value)
    {
        pthread_mutex_lock( &m_mutex );
        m_queue.push(value);
        if( !m_queue.empty() )
        {
            pthread_cond_signal( &m_condition );
        }
        pthread_mutex_unlock( &m_mutex );
    }

    template<class T> bool Queue<T>::pop(T& value, bool block)
    {
        bool rtn = false;
        pthread_mutex_lock( &m_mutex );
        if( block )
        {
            while( m_queue.empty() )
            {
                pthread_cond_wait( &m_condition, &m_mutex );
            }
        }
        if( !m_queue.empty() )
        {
            value = m_queue.front();
            m_queue.pop();  
            rtn = true;
        }
        pthread_mutex_unlock( &m_mutex );
        return rtn;
    }

Unfortunately there are occasional issues that may be the fault of this code. That is, there are two threads and sometimes thread 1 never comes out of push() and at other times thread 2 never comes out of pop() (the block parameter is true) though the queue isn't empty.

I understand there are other implementations available, but I'd like to try to fix this code, if needed. Anyone see any issues?

The constructor has the appropriate initializations:

    Queue()
    {
        pthread_mutex_init( &m_mutex, NULL );
        pthread_cond_init( &m_condition, NULL );
    }

and the destructor, the corresponding 'destroy' calls.

Upvotes: 1

Views: 2804

Answers (4)

jensph
jensph

Reputation: 785

I realized the problems occurred when testing a build for ARM. The solution was to update the pthreads library. With the updated pthreads, everything runs fine.

Upvotes: 0

Anthony
Anthony

Reputation: 12407

As mentioned by Paul Rubel, you need to initialise the mutex first. It may be a good idea at this point to wrap the mutex in a class that will handle your initialisation and finalisation for you. For example:

class mutex {
private:
    mutex(const mutex &m);
    mutex &operator=(const mutex &m);

    // OR inherit from boost::noncopyable
public:
    mutex() {
        pthread_mutex_init(&mut_, nullptr);
    }

    ~mutex() {
        pthread_mutex_destroy(&mut_);
    }

    pthread_mutex_t get() const { return mut_; }

private:
    pthread_mutex_t mut_;
};

Of note is the Boost.Threading Library which contains very well-written synchronisation classes.

Upvotes: 1

benjarobin
benjarobin

Reputation: 4487

This is not related to your problem at all, but you could fix the push function :

template<class T> void Queue<T>::push(T value)
{
    pthread_mutex_lock( &m_mutex );
    if( m_queue.empty() )
    {
        m_queue.push(value);
        pthread_cond_signal( &m_condition );
    }
    else
    {
        m_queue.push(value);
    }
    pthread_mutex_unlock( &m_mutex );
}

Upvotes: 0

Paul Rubel
Paul Rubel

Reputation: 27252

You must initialize the mutex before using it: pthread_mutex_init

//somewhere before push/pop
pthread_mutex_init(&m_mutex)

Upvotes: 0

Related Questions