Re: implementing a thread-safe queue
itcecsa wrote:
this is my implementation. feel free to give me commands.
I'll restrict myself to comments.
#include <pthread.h>
#include <deque>
#include <exception>
/* exception class for pop() with empty queue */
class ReadEmptyQueue : public std::exception {
public:
virtual const char* what() const throw() {
return "Queue is empty!";
}
};
template <class T>
class Queue {
private:
std::deque<T> *container; // container for the
elements
pthread_mutex_t mutex; // mutex for sync
the queue
Queue(const Queue& q); // disable copy
constructor
Queue& operator= (const Queue& q); // disable assign operator
public:
// to make it simple, this is the only constructor for Queue instance
explicit Queue():mutex(PTHREAD_MUTEX_INITIALIZER), container(new
std::deque<T>())
{}
// destructor
virtual ~Queue(){
delete container;
}
Do you need the constructor to be virtual? is polymorphic use of Queue<T>*
really intended? or is this a coding guideline you are following?
// number of elements
typename std::deque<T>::size_type size() const {
return container->size();
}
//is queue empty?
bool empty() const {
return container->empty();
}
In a multi-threaded setting, the result of this method would be useless
(even if empty() was implemented to be atomic) as another thread could
change the state of the container just between the return of empty() and the
use of the result.
I would ditch this method.
// insert element into the queue
void push (const T& elem) {
pthread_mutex_lock( &mutex );
container->push_back(elem);
pthread_mutex_unlock( &mutex );
}
// pop element from the top of the queue
T pop () {
pthread_mutex_lock( &mutex );
if (container->empty()) {
throw ReadEmptyQueue();
}
T elem(container->front());
container->pop_front();
pthread_mutex_unlock( &mutex );
return elem;
}
};
a) I don't know what happens to a locked mutex when this method throws, but
I doubt that it will magically get unlocked. As a consequence, the queue
might become unusable.
b) a more natural thing to do might be to make pop() block the calling
thread until an item becomes available. You can do that using condition
variables.
c) within pop(), an object of type T is copy constructed. When that
constructor throws, the element will not be removed from the queue and you
might have trouble unlocking the queue. The adaptor std::queue<T> has an
interface, designed to be exception safe even when copying objects of type T
might throw. It might be tricky to come up with an interface like that for a
thread communication queue. I would not object to the interface from above;
but you should document that copying T-objects must not throw. If T is a
pointer type, this is not a problem.
Best,
Kai-Uwe Bux
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]