C++ Templated Producer-Consumer BlockingQueue, unbounded buffer: How do I end elegantly?

Question

I wrote a BlockingQueue in order to communicate two threads. You could say it follows the Producer-Consumer pattern, with an unbounded buffer. Therefore, I implemented it using a Critical Section and a Semaphore, like this:

#pragma once

#include "Semaphore.h"
#include "Guard.h"
#include <queue>

namespace DRA{
namespace CommonCpp{
template<class Element>
class BlockingQueue{
    CCriticalSection    m_csQueue;
    CSemaphore          m_semElementCount;
    std::queue<Element> m_Queue;
//Forbid copy and assignment
    BlockingQueue( const BlockingQueue& );
    BlockingQueue& operator=( const BlockingQueue& );
public:
    BlockingQueue( unsigned int maxSize );
    ~BlockingQueue();
    Element Pop();
    void Push( Element newElement );
};
}
}

//Template definitions
template<class Element>
DRA::CommonCpp::BlockingQueue<Element>::BlockingQueue( unsigned int maxSize ):
    m_csQueue( "BlockingQueue::m_csQueue" ),
    m_semElementCount( 0, maxSize ){
}

template<class Element>
DRA::CommonCpp::BlockingQueue<Element>::~BlockingQueue(){
    //TODO What can I do here?
}

template<class Element>
void DRA::CommonCpp::BlockingQueue<Element>::Push( Element newElement ){
    {//RAII block
        CGuard g( m_csQueue );
        m_Queue.push( newElement );
    }
    m_semElementCount.Signal();
}

template<class Element>
Element DRA::CommonCpp::BlockingQueue<Element>::Pop(){
    m_semElementCount.Wait();
    Element popped;
    {//RAII block
        CGuard g( m_csQueue );
        popped = m_Queue.front();
        m_Queue.pop();
    }
    return popped;
}

CGuard is a RAII wrapper for a CCriticalSection, it enters it on construction and leaves it on destruction. CSemaphore is a wrapper for a Windows semaphore.

So far, so good, the threads are communicating perfectly. However, when the producer thread stops producing and ends, and the consumer thread has consumed everything, the consumer thread stays forever hung on a Pop() call.

How can I tell the consumer to end elegantly? I thought of sending a special empty Element, but it seems too sloppy.

Turn `Pop()` into `bool TryPop(Element&)`, and add a `void Finish()` that will cause `TryPop()` to return false. Something like that. — Cory Nelson, Jul 13 '11 at 18:05
If I do that, the consumer will call TryPop all the time and waste CPU, I'd like to keep the blocking Pop() style, since the semaphore's Wait yields the CPU — dario_ramos, Jul 13 '11 at 18:10
Keep blocking on the semaphore in `TryPop()`, but have `Finish()` release the semaphore just like `Push()` does. Then return `false` if the queue size is `0` and `finished == true`. — Cory Nelson, Jul 13 '11 at 18:15

Nikolai Fetissov · Answer 1 · 2011-07-13T18:27:24.473

1

You need a way of telling the consumer to stop. This could be a special element in the queue, say a simple wrapper structure around the Element, or a flag - a member variable of the queue class (in which case you want to make sure the flag is dealt with atomically - lookup windows "interlocked" functions). Then you need to check that condition in the consumer every time it wakes up. Finally, in the destructor, set that stop condition and signal the semaphore.

One issue remains - what to return from the consumer's pop(). I'd go for a boolean return value and an argument of type Element& to copy result into on success.

Edit:

Something like this:

bool Queue::Pop( Element& result ) {
    sema.Wait();
    if ( stop_condition ) return false;
    critical_section.Enter();
    result = m_queue.front();
    m_queue.pop;
    critical_section.Leave();
    return true;
}

edited Jul 13 '11 at 18:27

answered Jul 13 '11 at 18:06

Nikolai Fetissov

82,306
11
110
171

I thought you couldn't return a reference to a local variable. Or did I misunderstand? – dario_ramos Jul 13 '11 at 18:12
I didn't say return, I said *copy*. Pass a reference to the `Pop()` and have it copy the element from the queue to the argument. – Nikolai Fetissov Jul 13 '11 at 18:15
Now I get it, it's the same thing that @Cory Nelson suggested (dunno why he didn't answer). Thanks, gonna try that now – dario_ramos Jul 13 '11 at 18:17

score 1 · Answer 2 · answered Jul 13 '11 at 18:18

Does your Semaphore implementation have a timed wait function available? On Windows, that would be WaitForSingleObject() specifying a timeout. If so, Pop() could be implemented like this:

// Pseudo code
bool Pop(Element& r, timeout)
{
   if(sem.wait(timeout))
   {
      r = m_Queue.front();
      m_Queue.pop();
   }

   return false;
}

This way, the Pop() is still blocking, though it can be easily interrupted. Even with very short timeouts this won't consume significant amounts of CPU (more than absolutely necessary, yes -- and potentially introduce additional context switching -- so note those caveats).

Yes, I considered that. But I'd like to avoid any kind of polling. I know, I could set a really large timeout, but honestly, I prefer @Nikolai's solution — dario_ramos, Jul 13 '11 at 18:21

score 1 · Accepted Answer · answered Jul 14 '11 at 07:48

1

You better use events instead of Semaphore. While adding, take lock on CS, and check element count (store into bIsEmpty local variable). Add into queue, then check if number of elements WAS empty, SetEvent!

On pop method, lock first, then check if it is empty, then WaitForSingleObject - as soon as WFSO returns you get that there is at least one item in queue.

Check this article

answered Jul 14 '11 at 07:48

Ajay

18,086
12
59
105

Great article, it seems to provide a more efficient implementation. I'll check the examples to see how it handles the producer ending. If it's done, consider your answer accepted – dario_ramos Jul 14 '11 at 12:29
Alright, I finished checking the article. If I add your suggestion, it behaves like expected, that is, no polling. It's a better implementation. However, you didn't answer the question: How do I end producer and consumer elegantly? I tried looking at the demo app from the article, but it had 2000 lines of MFC and that freaked me out – dario_ramos Jul 14 '11 at 14:12
If you are using events, you can use WaitForMultipleObjects. WFMO would wait for two sync-objects: first is the STOP event, and second is new-message event. When the first event is set, you know it is time to quit. And do I need to tell how to trigger stop? :) – Ajay Jul 14 '11 at 14:16

score 0 · Answer 4 · answered Jul 13 '11 at 18:17

0

Change pop to return a boost optional (or do it like the standard library does with top/pop to separate the tasks) and then signal m_semElementCount one last time on destruction.

answered Jul 13 '11 at 18:17

Mark B

95,107
10
109
188

If you imply relying on the Boost library, I can't use that. Just Windows stuff, the boss said – dario_ramos Jul 13 '11 at 18:19

C++ Templated Producer-Consumer BlockingQueue, unbounded buffer: How do I end elegantly?

4 Answers4

Edit:

Linked