-3

Can you please review this code. I believe there is nothing wrong here.

You may esp. like to look the dequeue function of a class where template is used.

void enqueue(const T &data)
    {
        _mutex.lock();
        _queue.push(data);
        _mutex.unlock();
    }

T const& dequeue()
    {
        _mutex.lock();
        T &data = _queue.back();
        _queue.pop();
        _mutex.unlock();

        return data;
    }
Faisal
  • 35
  • 1
  • 1
    Is this homework? What exactly do you need help with? – Jonathan Grynspan Jun 21 '11 at 17:06
  • 1
    Why do you think that there is something wrong? Or do you want a general code review? If so, go to codereview.stackexchange.com, where you will be told that this extract does not have sufficient context to review. – Lightness Races in Orbit Jun 21 '11 at 17:07
  • Sorry if it appears ambiguous. Question is here: T &data = _queue.back(); Ppl may say there is reference dangling issue. This is what I want to know. – Faisal Jun 22 '11 at 00:36

2 Answers2

3

In dequeue, you return a dangling reference. Once you've popped the object, it ceases to exist.

James Kanze
  • 150,581
  • 18
  • 184
  • 329
1

Just no.

Firstly, you can't just slap a mutex on it and call it thread safe. Not only is that going to generate horrific overhead when it may well be unnecessary, but you will also break the atomicity of certain operations- for example, you can't guarantee that if I check the size of the queue and then if it's more than zero, take one off the queue- because someone may have emptied it in the meantime. Or what if I take an object off and now it's been popped off? Oops.

Thread safety is in not accessing data concurrently, not just chucking a mutex at the data structure and calling it done.

Secondly, if you are going to build a concurrent container, they do exist and they are necessary, then you have a completely different interface. Look at the concurrent data structures of Intel's TBB and Microsoft's PPL. They have an interface designed for concurrent use which is going to be faster and significantly less buggy than your slap-a-mutex-on-it hacks.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • I agree with you. But this implementation, in fact, is a wrapper of stl::queue to provide some synchronzation stuff using pthread. It is made simple understanding requirements. – Faisal Jun 22 '11 at 00:33