Example C++ pthreads producers consumers project
Daniel Hams
I've been learning about pthreads and have implemented the
producers/consumers problem in C++. Since this may be useful to anyone
learning pthreads in C++, I've stuck it up on the web for all.
Its a GNU autoconf/automake project, so you _should_ (grin) be able to
type ./configure ./make to build it.
Needs:
recent c++ compiler- I found gcc2.96 on redhat and gcc3.0.3 to work ok
under linux, whereas gcc2.95 under openbsd3.0 doesnt work :-/
pthreads library
stl (I'm using the one that comes with gcc)
Its available here:
http://www.huntthepickle.org/~dan/projects/ProducersConsumers/producersconsumers-1.0.tar.gz
and its info page with a UML piccy is here:
http://www.huntthepickle.org/~dan/projects/ProducersConsumers/ProducersConsumers.html
If anyone spots any bugs, or has comments, or even is able to build it.
I'd love to hear from you.
Best regards,
Daniel Hams
Kaz Kylheku
>Dear all,
>
>I've been learning about pthreads and have implemented the
>producers/consumers problem in C++. Since this may be useful to anyone
>learning pthreads in C++, I've stuck it up on the web for all.
Some comments:
I think that subclassing ConditionableMutex from Mutex is a useless
hairsplitting. I would just have one class. All you did was add some
extra functions in the ConditionableMutex, but no extra state. These
functions can clearly do their job if they lived in the Mutex class.
Also, I would put condition and mutex classes into the same header and
source file, since they are very closely related.
You made a big design mistake by having a method for calling
pthread_cond_signal only, and not pthread_cond_broadcast. If you are only
going to have one condition signaling mechanism, it should be a broadcast.
In a correct program, a any condition signal can always be replaced by
a broadcast. The reverse is not true.
Having your producer/consumer queues based on a concreted Widget type
that carries a binary payload declared as an array member is not
terribly useful. Such a payload is useless for transporting C++ objects;
people will have to fiddle with nonportable uses of placement new to
put an object into the array, and they only have 20000 bytes, which
might not be enough, or which is wasteful if the object is smaller.
The C++ language already has a generic queue template. You can make a
synchronized queue based directly on that, on maybe on std::vector.
Subclassing from an ancestral Thread class with an overridable run()
method is obvious, but turns out to be not such a great idea. It gets
in your way if you want an object with two or more threads. Consider
making a concrete Thread class, similar to your concrete Mutex. The class
represents a thread object only, not the work which the thread executes. A
start() method in the thread class can take a pointer to an ActiveObject
base class. That base class can have methods related to the work that
the thread executes, like run(), cancel_notify() and whatnot.
You are missing loops in the condition variable waits of your WidgetQueue
class. This class seems way too complicated; you don't need counters
representing how many consumers or producers are waiting.
Doing things like only signaling the condition if consumersWaiting > 0
is a complete waste. The condition variable already has this logic
built into it: it does nothing if there are no waiting threads.
Your WidgetQueue is poorly named; it should be called WidgetStack.
Because its push and pop semantics have LIFO (last in, first out)
semantics.
The way you preincrement your currentSize variable in
the push operation, you waste the [0] element of your array.
The push operation should postincrement, the pop should predecrement.
Nationalistic or ethnic slurs like ``don't jump the queue like the
French'' probably don't belong in source code that you want to share
with people.
The implementation takes care of waking up waiting threads in the
appropriate order according to the scheduling policy and priority,
so you don't really have to worry about issues like that. Trust
the objects that you are using. Trying to force the execution order
is usually counterproductive, because you force a thread to sleep
which could otherwise just get on with its work.
You do some stupid things from a purely C++ viewpoint. (Hey, if you
don't mind calling an entire nation queue jumpers, you shouldn't
mind being called stupid, right?). For example, your WidgetQueue class
allocates an object using malloc() in the constructor and then destroys
it using the delete operator. These are within ten lines of each other;
how can you not see that?
Central header files like "proj.h" get in the way of code reuse.
If you want to use, say, your Mutex and Condition classes in some project,
you have to drag along this "proj.h". Every source file should include
the system headers it needs to form its declarations or definitions.
Daniel Hams
I'm just recently getting into C++ and pthreads as I mentioned, so its a
learning process, right?
> I think that subclassing ConditionableMutex from Mutex is a useless
> hairsplitting. I would just have one class. All you did was add some
> extra functions in the ConditionableMutex, but no extra state. These
> functions can clearly do their job if they lived in the Mutex class.
> Also, I would put condition and mutex classes into the same header and
> source file, since they are very closely related.
Hmm. Interesting. My gut feeling with objects is they are just domain
type instantiations, not things that must have state. Reason for
multiple files -> Dans java background....
>
> You made a big design mistake by having a method for calling
> pthread_cond_signal only, and not pthread_cond_broadcast. If you are only
> going to have one condition signaling mechanism, it should be a broadcast.
> In a correct program, a any condition signal can always be replaced by
> a broadcast. The reverse is not true.
The extra method was not required for me to get the project working, so
I didn't pop it in.
The broadcast issue you mention is interesting though. Does
pthread_cond_broadcast wake up _all_ threads waiting on the condition?
If it does, does each thread then, after waking up have to check to see
if things are in the queue (or vice-versa space in the queue)?
>
> Having your producer/consumer queues based on a concreted Widget type
> that carries a binary payload declared as an array member is not
> terribly useful.
Er, this was intended as an example project for me to get to grips with
the concepts, and calls.
> Such a payload is useless for transporting C++ objects;
> people will have to fiddle with nonportable uses of placement new to
> put an object into the array, and they only have 20000 bytes, which
> might not be enough, or which is wasteful if the object is smaller.
>
> The C++ language already has a generic queue template. You can make a
> synchronized queue based directly on that, on maybe on std::vector.
As mentioned inside the README in the project, the std:queue
implementation has memory issues for me, which is why there is code to
use it, but its commented out.
>
> Subclassing from an ancestral Thread class with an overridable run()
> method is obvious, but turns out to be not such a great idea. It gets
> in your way if you want an object with two or more threads. Consider
> making a concrete Thread class, similar to your concrete Mutex. The class
> represents a thread object only, not the work which the thread executes. A
> start() method in the thread class can take a pointer to an ActiveObject
> base class. That base class can have methods related to the work that
> the thread executes, like run(), cancel_notify() and whatnot.
Seems an interesting idea Kaz, I kinda went for the Java'y style as
thats where I'm coming from. I'll give your suggestion a stab too.
>
> You are missing loops in the condition variable waits of your WidgetQueue
> class. This class seems way too complicated; you don't need counters
> representing how many consumers or producers are waiting.
> Doing things like only signaling the condition if consumersWaiting > 0
> is a complete waste. The condition variable already has this logic
> built into it: it does nothing if there are no waiting threads.
You mention that loops are missing in the condition variable waits -
would these loops be to check on waking if the queue has things
available? Is this related to your earlier point about only needing to
use pthread_cond_broadcast to signal?
>
> Your WidgetQueue is poorly named; it should be called WidgetStack.
> Because its push and pop semantics have LIFO (last in, first out)
> semantics.
Again, was intended to be a queue, std::queue has memory issues, but
point taken.
>
> The way you preincrement your currentSize variable in
> the push operation, you waste the [0] element of your array.
> The push operation should postincrement, the pop should predecrement.
>
> Nationalistic or ethnic slurs like ``don't jump the queue like the
> French'' probably don't belong in source code that you want to share
> with people.
A little harsh there mate. Being english married to a frenchie in
belgium, its all water off a ducks back here. And the french do jump
queues. Honest. Ask a French person.
>
> The implementation takes care of waking up waiting threads in the
> appropriate order according to the scheduling policy and priority,
> so you don't really have to worry about issues like that. Trust
> the objects that you are using. Trying to force the execution order
> is usually counterproductive, because you force a thread to sleep
> which could otherwise just get on with its work.
>
> You do some stupid things from a purely C++ viewpoint. (Hey, if you
> don't mind calling an entire nation queue jumpers, you shouldn't
> mind being called stupid, right?).
Course not, we are all on this planet to learn right :-)
> For example, your WidgetQueue class
> allocates an object using malloc() in the constructor and then destroys
> it using the delete operator. These are within ten lines of each other;
> how can you not see that?
Ooopsie. I think the sheer pleasure of getting that sucker working
probably took its toll :-)
Will fix that.
>
> Central header files like "proj.h" get in the way of code reuse.
> If you want to use, say, your Mutex and Condition classes in some project,
> you have to drag along this "proj.h". Every source file should include
> the system headers it needs to form its declarations or definitions.
>
Sound advice.
Thanks again for your feedback Kaz, much appreciated.
Dan
David Butenhof
>> You made a big design mistake by having a method for calling
>> pthread_cond_signal only, and not pthread_cond_broadcast. If you are only
>> going to have one condition signaling mechanism, it should be a
>> broadcast. In a correct program, a any condition signal can always be
>> replaced by a broadcast. The reverse is not true.
>
> The broadcast issue you mention is interesting though. Does
> pthread_cond_broadcast wake up _all_ threads waiting on the condition?
> If it does, does each thread then, after waking up have to check to see
> if things are in the queue (or vice-versa space in the queue)?
I'm trying to catch up after being out of everything for a couple of weeks.
Therefore, I haven't looked at the code (and I don't plan to), but I will
comment on this bit.
1) Yes, pthread_cond_broadcast() MUST unblock all threads currently waiting
on the condition variable.
2) Yes, all CALLERS of pthread_cond_wait() must re-check their predicate on
wakeup from the condition wait and be prepared to wait again. This is
essential for a number of reasons. Any application that doesn't do this is
busted. Period. No saving throw.
If you're counting on the thread library to re-check your predicate, think
again. It doesn't KNOW your predicate, nor have any way to evaluate it. The
predicate, and the responsibility for managing it, is yours.
/------------------[ David.B...@compaq.com ]------------------\
| Compaq Computer Corporation POSIX Thread Architect |
| My book: http://www.awl.com/cseng/titles/0-201-63392-2/ |
\-----[ http://home.earthlink.net/~anneart/family/dave.html ]-----/
Daniel Hams
> Therefore, I haven't looked at the code (and I don't plan to), but I will
> comment on this bit.
>
> 1) Yes, pthread_cond_broadcast() MUST unblock all threads currently waiting
> on the condition variable.
>
> 2) Yes, all CALLERS of pthread_cond_wait() must re-check their predicate on
> wakeup from the condition wait and be prepared to wait again. This is
> essential for a number of reasons. Any application that doesn't do this is
> busted. Period. No saving throw.
>
> If you're counting on the thread library to re-check your predicate, think
> again. It doesn't KNOW your predicate, nor have any way to evaluate it. The
> predicate, and the responsibility for managing it, is yours.
Hi David, thanks for the info.
My understanding then, is that in every case, each pthread_cond_wait
should be wrapped inside a while loop waiting on some required predicate.
pthread_cond_broadcast frees up all threads sat in the wait (although I
imagine only one gets the mutex lock at that time, right?), whereupon
they must check the predicate.
In the case of my WidgetQueue, since only one Widget can ever be placed
into the queue at once, would it seem correct to use pthread_cond_signal.
If a method is introduced on the WidgetQueue allowing multiple widgets
to be placed inside the queue, a pthread_cond_broadcast would become the
more appropriate call to make?
Heres what my widget queue code looks like now (after the advice of Kaz
- thanks again K):
Widget* WidgetQueue::pop()
{
queueMutex.lock();
while (currentSize == 0)
{
// Is a pthread_cond_wait call
queueMutex.waitOnCondition( hasWidgetsCondition);
}
// Get a widget out
currentSize--;
// stl queue code
Widget *w = widgetQueue.front();
widgetQueue.pop();
queueMutex.unlock();
// Signal space has been cleared using pthread_cond_signal
queueMutex.notifyOnCondition( hasSpaceCondition );
return( w );
}
void WidgetQueue::push(Widget *w)
{
queueMutex.lock();
while (currentSize == maxSize)
{
// Is a pthread_cond_wait call
queueMutex.waitOnCondition( hasSpaceCondition );
}
// stl queue code
widgetQueue.push(w);
currentSize++;
queueMutex.unlock();
// Signal widget has been pushed using pthread_cond_signal
queueMutex.notifyOnCondition( hasWidgetsCondition );
}
Best regards,
Dan
Alexander Terekhov
Daniel Hams wrote:
[...]
> Widget* WidgetQueue::pop()
> {
> queueMutex.lock();
>
> while (currentSize == 0)
> {
> // Is a pthread_cond_wait call
> queueMutex.waitOnCondition( hasWidgetsCondition);
> }
>
> // Get a widget out
> currentSize--;
>
> // stl queue code
> Widget *w = widgetQueue.front();
> widgetQueue.pop();
>
Perhaps not really important, but containers (such as
stl ones) usually provide size()/empty() interfaces
which could be used for cv.wait predicates without
any extra counting on top of container interfaces.
Also, for locking, you might want to use some RAII
classes that would ensure automatic unlock on scope
exit... of course, that is presuming that you code
in thread-exception-safe manner (including thread-
cancelation/exit ;) such that there will be no risk
of exposing broken invariants to other threads due
to "unwanted" unlocks (before your victim thread
exception processing would just terminate the
process in "controlled" fashion reporting out-of-
memory condition, for example).
regards,
alexander.
David Butenhof
> My understanding then, is that in every case, each pthread_cond_wait
> should be wrapped inside a while loop waiting on some required predicate.
> pthread_cond_broadcast frees up all threads sat in the wait (although I
> imagine only one gets the mutex lock at that time, right?), whereupon
> they must check the predicate.
That's correct. (Including that only a single thread at a time can get the
mutex; that's basically all a mutex really does, and it's quite good at
that. ;-) )
> In the case of my WidgetQueue, since only one Widget can ever be placed
> into the queue at once, would it seem correct to use pthread_cond_signal.
If there can always be only "no waiters" or "one waiter", then whether you
use "signal" or "broadcast" is completely irrelevant (functionally), though
"signal" may in practice be slightly faster.
> If a method is introduced on the WidgetQueue allowing multiple widgets
> to be placed inside the queue, a pthread_cond_broadcast would become the
> more appropriate call to make?
That depends on what behavior you expect from the waiters.
Really, "the basic way" to awaken waiter(s) on a condition variable is
broadcast. It's general, and always correct.
Think of signal as an OPTIMIZATION for the case where you know that correct
behavior does not require more than one waiter to be awakened. That means
only one waiter can usefully proceed and that ANY waiter may be that one.
If the application requires that more than one waiter proceed, use
broadcast. If multiple waiters use different predicates (generally
having them share a single condition variable is a bad idea, but not
inherently illegal as long as the predicates are under the same mutex),
then you must use broadcast to ensure that the correct waiter is awakened.
(You cannot "chain" single wakeups by having the awakened waiter signal and
then re-wait, because there's no guarantee that you'd ever get to the
correct waiter.)
Given the code you've posted, and if it's possible to have more than one
thread simultaneously trying to push a widget, then broadcast might be
slightly more efficient. All of the waiters would contend for the queue,
and potentially be able to push their widgets before any were popped. With
signal, you'll push one of the waiting widgets, pop it, wake the next
waiter to push its widget, pop it... and so forth. That translates to more
context switches for the same amount of work. (Though broadcast won't
guarantee that all the pushes will complete before someone tries to pop;
you'd need a stronger protocol for that. Still, on average it will likely
be more efficient.)
Daniel Hams
> Also, for locking, you might want to use some RAII
> classes that would ensure automatic unlock on scope
> exit... of course, that is presuming that you code
> in thread-exception-safe manner (including thread-
> cancelation/exit ;) such that there will be no risk
> of exposing broken invariants to other threads due
> to "unwanted" unlocks (before your victim thread
> exception processing would just terminate the
> process in "controlled" fashion reporting out-of-
> memory condition, for example).
Hi Alex,
OOO lots more things for me to learn about :-)
I see what you mean about exception safe and thread cancellation/exit
safe code. (Guaranteeing leaving the container and locks in a sane state)
Thats a whole 'nother kettle of fish for me to read up on.
Ta muchly,
Dan
Daniel Hams
Dan
Alexander Terekhov
David Butenhof wrote:
[...]
> Given the code you've posted, and if it's possible to have more than one
> thread simultaneously trying to push a widget, then broadcast might be
> slightly more efficient. All of the waiters would contend for the queue,
> and potentially be able to push their widgets before any were popped. With
> signal, you'll push one of the waiting widgets, pop it, wake the next
> waiter to push its widget, pop it... and so forth.
hmm... push() code was:
: void WidgetQueue::push(Widget *w)
: {
: queueMutex.lock();
:
: while (currentSize == maxSize)
: {
: // Is a pthread_cond_wait call
: queueMutex.waitOnCondition( hasSpaceCondition );
: }
Threads trying to push a widget start waiting
on condition variable when queue becomes full.
pop() pops only one widget per invocation.
So maybe I am missing something, but I fail to
understand why "broadcast might be slightly more
efficient", given the posted code.
regards,
alexander.
David Butenhof
Y'know, I'm quite sure that I had something important and quite interesting
to communicate with that last paragraph. I clearly didn't write it well,
and now I'm no more able to figure out what I intended to say than you.
Undoubtedly a symptom of rushing too much to get caught up after being out
for two weeks. I suspect that I was imagining the consequences of some
particular behavior or expectations (subtly different from the actual code)
that I neglected to record. But... it's gone now. ;-)
Alexander Terekhov
David Butenhof wrote:
[...]
> Y'know, I'm quite sure that I had something important and quite interesting
> to communicate with that last paragraph. I clearly didn't write it well,
> and now I'm no more able to figure out what I intended to say than you.
> Undoubtedly a symptom of rushing too much to get caught up after being out
> for two weeks. I suspect that I was imagining the consequences of some
> particular behavior or expectations (subtly different from the actual code)
> that I neglected to record. But... it's gone now. ;-)
Apropos "catching up"... (I hope you won t kill me
for the this...) Quoting you out of context:
: And since POSIX 1003.1-2001 has now been formally approved ...
and referring you to your signature:
: | Compaq Computer Corporation POSIX Thread Architect |
I just would like to assert that NOW YOU JUST HAVE NO
EXCUSE NOT TO DEVOTE SOME OF YOUR TIME UPDATING/ADDING
NEW STUFF (WITH YOUR INSIGHTS) AND PUBLISHING THE 2ND
EDITION OF:
: | My book: http://www.awl.com/cseng/titles/0-201-63392-2/ |
and... *HARDCOVER* this time! Please!! ;-)
regards,
alexander.
David Butenhof
> Apropos "catching up"... (I hope you won’t kill me
> for the this...) Quoting you out of context:
>
> : And since POSIX 1003.1-2001 has now been formally approved ...
>
> and referring you to your signature:
>
> : | Compaq Computer Corporation POSIX Thread Architect |
>
> I just would like to assert that NOW YOU JUST HAVE NO
> EXCUSE NOT TO DEVOTE SOME OF YOUR TIME UPDATING/ADDING
> NEW STUFF (WITH YOUR INSIGHTS) AND PUBLISHING THE 2ND
> EDITION OF:
>
> : | My book: http://www.awl.com/cseng/titles/0-201-63392-2/ |
>
> and... *HARDCOVER* this time! Please!! ;-)
Thanks for the implicit compliment. Nice to see people waiting for an
update to the book. I still haven't actually received any free time from
anyone, though. ;-)
[I must be in a "chatty" mood today, because what follows is a lot more
than I need to or really intended to write. I'd feel bad just deleting it,
though, so I'm just going to hit the send button. ;-) ]
Unfortunately, my role as "POSIX Thread Architect" doesn't really refer to
my involvement in the standardization effort. While I was closely involved
in the development of the original thread standard, including attending all
the committee meetings, I haven't done any travelling for 1003.1-2001. I've
followed the work, and contributed occasionally via email, but for the most
part it hasn't taken all that much of my time.
The (defacto) title refers to my involvement in the software development
team that builds the POSIX thread library for Tru64 UNIX and VMS. Our
development work is defined by customer and operating system team goals and
schedules, not simply by the progress of the standard document. (Although
one could also consider that, having completed the new standard, I need to
think about how much of the new stuff we want to implement. I'll worry
about that after I get some annoying alligators off my back...)
I haven't had much free time at work for years, and don't foresee any
coming up soon. If HP actually goes take over Compaq, with their own UNIX
and thread team, I really don't know what happens next. Maybe I find a
whole new game. Maybe I just sit here and annoy them while working on
something completely different, which might be fun. Maybe someone will even
make me "an offer I can't refuse" and I'll leave the company. If HP doesn't
take over, I have even less idea what'll happen next. The official party
line is that Tru64 UNIX still goes away, but there's been some indication
that at least a few members of upper management may not have their heads
COMPLETELY buried, and might actually suspect that being dependent on a
direct competitor's OS (which, even if it's not substantially worse than
Tru64 UNIX, certainly isn't substantially better) might not be the best
long-term strategy. On the other hand, we've already thrown out the best
processor in the world and "one of" if not "the" best compiler teams in the
world... so logic clearly cannot cover the decisions being made around here.
Furthermore, I'd really rather do this at home than at work. Though I can't
really claim it was unfair, I often feel that I got a bad deal on the first
book. Since it was done entirely on company time, for which I was paid
normal salary, the company (Digital) takes the book royalties. While the
money isn't much even against our department budget, much less the whole
company, let's just say it'd be a pretty nice bit of extra change for ME.
When I write another book, I want to enjoy the income from it myself, and I
think that means doing it on my own. With two young children (9 and 5), my
time at home is at least as busy as my time at work. Although, give me a
reliable Mac OS X build of OpenOffice and some improvements in the Darwin
POSIX thread implementation, and I might start finding enough spare moments
to get something started... ;-)
As for hardcover... that's the decision of the publisher. Given that my
first book wasn't much of a disaster, perhaps my second would earn a
hardcover! Though the way my editor explained it, the decision had more to
do with their rough estimates of how they expected people to USE the book.
/------------------[ David.B...@compaq.com ]------------------\
| Compaq Computer Corporation POSIX Thread Architect |
Sergey P. Derevyago
> time at home is at least as busy as my time at work. Although, give me a
> reliable Mac OS X build of OpenOffice and some improvements in the Darwin
But why Mac OS X and OpenOffice? IMHO you can write pretty good book(s) in
other environments too, can't you? :)
--
With all respect, Sergey. http://cpp3.virtualave.net/
mailto : ders at skeptik.net