Jump to content
  • Advertisement
Sign in to follow this  
Robert007

Multithreading with Pthread

This topic is 3081 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, folks, The Pthread code below sometimes runs correctly (worker2 starts its work after being notified by worker1), but not at other time (worker2 failed to start totally even being notified by worker2 to start, the program just hangs). (1) What is wrong with it? (2) I am running it under Ubuntu Linux 4.4.1 with g++ 4.4.1 compiler. I am wondering what multithreaded debugger/profiling tools you guys would suggest to use / available to get clues to the problem. Thanks a lot in advance.
#include <iostream>
#include <pthread.h>
using namespace std;

int Number;
pthread_t ThreadA, ThreadB;
pthread_mutex_t Mutex, EventMutex;
pthread_cond_t Event;				

void* worker1(void* X)
{
	for (int Count = 1; Count <= 10; Count++ ) {
		pthread_mutex_lock(&Mutex);
		Number++;
		pthread_mutex_unlock(&Mutex);
		cout << "work1: Number = " << Number << endl;
		if ( Number == 7 ) {
			pthread_cond_signal(&Event);
		}
	}
}
void* worker2(void* X)
{
	pthread_mutex_lock(&EventMutex);
	pthread_cond_wait(&Event, &EventMutex);
	pthread_mutex_unlock(&EventMutex);	
	for (int Count = 1; Count <= 10; Count++ ) {
		pthread_mutex_lock(&Mutex);
		Number += 20;
		cout << "work2: Number = " << Number << endl;
		pthread_mutex_unlock(&Mutex);
	}
	return (0);
}

int main(int argc, char* argv[]) {
	pthread_mutex_init(&Mutex, NULL);
	pthread_mutex_init(&EventMutex, NULL);
	pthread_cond_init(&Event, NULL);

	pthread_create(&ThreadA, NULL, worker1, NULL);
	pthread_create(&ThreadB, NULL, worker2, NULL);

	pthread_join(ThreadA, NULL);
	pthread_join(ThreadB, NULL);

	return (0);
}



Share this post


Link to post
Share on other sites
Advertisement
worker2 hangs, because worker1 signals the condition before worker2 starts waiting. This is a classic beginner's mistake.

Here is some pseudo code which shows how to use conditions right:


worker1() {
pthread_mutex_lock(&mutex);
/* TODO: change condition */
pthread_cond_signal(&cond);
pthread_mutex_unlock(&mutex);
}

worker2() {
pthread_mutex_lock(&mutex);
while(/* TODO: condition not satisfied */) {
pthread_cond_wait(&cond, &mutex);
}
pthread_mutex_unlock(&mutex);
}


The "TODO" parts depend on what kind of condition you want to check.

Example: worker1 sets a shared flag to 'true', and worker 2 waits until this flag becomes 'true'. Then the "TODO"s are replaced as follows:


bool my_flag = false;

worker1() {
pthread_mutex_lock(&mutex);
my_flag = true;
pthread_cond_signal(&cond);
pthread_mutex_unlock(&mutex);
}

worker2() {
pthread_mutex_lock(&mutex);
while(!my_flag) {
pthread_cond_wait(&cond, &mutex);
}
pthread_mutex_unlock(&mutex);
}


This is only a simple example. The condition (which is a simple flag here) might be anything worker1 could notify worker2 about, for example a counter being greater/less than some threshold, a queue being empty/non-empty, etc.

However, your condition must not be "empty"! Never call pthread_cond_wait without checking for a condition first!

It is important that all variables which are part of the condition (here: my_flag) are protected by the mutex. Do not access them when the mutex is not locked! Also note that worker2 unlocks the mutex while it is waiting, and that pthread_cond_wait returns when worker1 unlocks the mutex, not when it calls pthread_cond_signal.

worker2 will not wait if the condition is already satisfied (because worker1 was faster with locking the mutex). Therefore, worker2 will not hang if the condition has already been signaled.

You may be wondering why worker2 uses a while loop instead of a simple if statement to wait for the condition to be satisfied. In fact, "if" would be sufficient here, but in more complex cases it might be not (for example, when a condition variable is used for multiple purposes). My advice is to just use "while" all the time, or at least when in doubt.


Finally, you should look up "scoped locking" and start using it. It is one of the many reasons to prefer C++ over C. You should also consider using a C++ threading library instead of pthreads, such as boost::thread.


A helpful tool for finding some threading errors is helgrind (which is part of the valgrind package). It can find misuses of the threading library (invalid arguments), lock-order violations (you should learn about this before adding more mutexes to your code) and potential race conditions.

Share this post


Link to post
Share on other sites
this is not very helpfull but in my opinion dump pthreads and try using OpenMp.

i think pthreads are horribly bad.

Share this post


Link to post
Share on other sites
pthreads/OpenMP address different problems and provide different abstractions.

http://www.drdobbs.com/high-performance-computing/200001985

[Edited by - Hodgman on April 11, 2010 6:03:48 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Robert007
I am wondering what multithreaded debugger/profiling tools you guys would suggest to use / available to get clues to the problem.

Have you tried gdb? It's designed or debuggung. I understand you can get a GUI for it if you can't hack the CLI but it's certainly the first tool we turn to when there is a problem of any sort. Start with the "info threads" and "backtrace" commands.

Share this post


Link to post
Share on other sites
Intel has a tool (actually a plug in for VTune I think) called Intel Thread Checker, which can monitor your code for things like potential race conditions, etc...
I think it's pretty pricey though, so it's probably aimed at companies rather than individuals.

Share this post


Link to post
Share on other sites
These are very helpful to me. Now back to my code again, the EventMutex is not needed and thus removed. The Number can be used as a flag for the two workers. So the code should be as follows:

#include <iostream>
#include <pthread.h>
using namespace std;

int Number;
pthread_t ThreadA, ThreadB;
pthread_mutex_t Mutex;
pthread_cond_t Event;

void* worker1(void* X)
{
for (int Count = 1; Count <= 10; Count++ ) {
pthread_mutex_lock(&Mutex);
Number++;
pthread_mutex_unlock(&Mutex);
if ( Number == 7 ) {
pthread_cond_signal(&Event);
}
cout << "work1: Number = " << Number << endl;
}
}
void* worker2(void* X)
{
pthread_mutex_lock(&Mutex);
while ( Number < 7 )
{
pthread_cond_wait(&Event, &Mutex);
}
pthread_mutex_unlock(&Mutex);

for (int Count = 1; Count <= 10; Count++ ) {
pthread_mutex_lock(&Mutex);
Number += 20;
pthread_mutex_unlock(&Mutex);
cout << "work2: Number = " << Number << endl;
}
return (0);
}

int main(int argc, char* argv[]) {
pthread_mutex_init(&Mutex, NULL);
pthread_cond_init(&Event, NULL);

pthread_create(&ThreadA, NULL, worker1, NULL);
pthread_create(&ThreadB, NULL, worker2, NULL);

pthread_join(ThreadA, NULL);
pthread_join(ThreadB, NULL);

return (0);
}





Any review of it would be appreciated. Any improvement should I make? Thanks.

Share this post


Link to post
Share on other sites
Assuming that the timing is not overly critical.. Why bother with a Mutex at all?


Worker1 - > flag++



worker2
{
while (flag < 7)
{
sleep(9999);
}



}

The threads will always execute in the correct order and will never hang - A mutex is not needed when access to the data is limited to increments and tests. If the flag needed to be reset to zero then a mutex should be used to prevent an increment during the actual reset but this can actually be avoided in most cases as well.











Share this post


Link to post
Share on other sites
Quote:
Original post by BrianLarsen
worker2
{
while (flag < 7)
{
sleep(9999);
}
}
With that code, the compiler is well within it's rights to determine that flag isn't modified inside the loop, and then 'optimise' it to be:
worker2
{
while(true)
{//uh oh, infinite loop!
sleep(9999);
}
}
The condition variable / signal doesn't suffer from this optimisation.
Quote:
A mutex is not needed when access to the data is limited to increments and tests. If the flag needed to be reset to zero then a mutex should be used to prevent an increment during the actual reset but this can actually be avoided in most cases as well.
No! You've got a potential race condition on all those modifications unless you somehow make them atomic. An increment is a fetch, an add and a store (3 instructions), not 1 indivisible instruction. While you're adding, the other thread might have stored a value, which you'll overwrite.

A 'safe' version without mutexes (mutices?) might look like:
volatile long Number;//volatile prevents the compiler from re-ordering and/or optimising-out reads/writes to this variable.
//Volatile doesn't make modifications atomic though, we need to use compiler intrinsics for that (see below)
//N.B. an Intel/AMD CPU may still re-order reads/writes at runtime, memory fence instructions can be used to inhibit this if required

void* worker1(void* X)
{
for (int Count = 1; Count <= 10; Count++ ) {
//Atomic version of Number++
//This is like a tiny mutex implemented efficiently inside the CPU
#if defined(WIN32)
InterlockedIncrement(&Number);
#elif defined(GCC)
__sync_fetch_and_add(&Number, 1);
#else
#error "unsupported platform"
#endif
//if (Number==7) that's the signal for worker2 to start up.
//spin-lock until we're sure worker2 has received our signal.
while( Number == 7 ) {
Sleep(0);
}
cout << "work1: Number = " << Number << endl;
}
return (0);
}
void* worker2(void* X)
{
//spin-lock until signalled by worker1
while( Number < 7 ) {
Sleep(0);
}

for (int Count = 1; Count <= 10; Count++ ) {
//Atomic version of Number += 20
#if defined(WIN32)
InterlockedExchangeAdd(&Number, 20);
#elif defined(GCC)
__sync_fetch_and_add(&Number, 20);
#else
#error "unsupported platform"
#endif
cout << "work2: Number = " << Number << endl;
}
return (0);
}
On some CPUs these atomic operations are implemented by locking the bus, so that other memory operations can't occur during the fetch/add/store, on other CPUs it's implemented as a loop that can detect race-conditions and keeps trying to perform the operation until there isn't one. Either way, it's important to know what the CPU/cache is actually doing if you want to throw away your mutex objects ;)

[Edited by - Hodgman on April 13, 2010 7:59:20 AM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Makaan
this is not very helpfull but in my opinion dump pthreads and try using OpenMp.

i think pthreads are horribly bad.


Guess what gcc-OpenMP uses under the hood. I would prefer "very low level" over "bad". And I wish Microsoft would finally adopt the widespread PThreads as most other operating systems on the world, for sake of portability.

(and for sake of my own application, where I currently have to disable OpenMP on Windows when run inside my Qt application, which seems to run on Windows threads)

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!