   Multithreading with Pthread
  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);}
  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.   ;  this is not very helpfull but in my opinion dump pthreads and try using OpenMp.i think pthreads are horribly bad.   ;  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];Quote:Original post by Robert007I 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.   ;  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.   ;  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.   ;  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.;Quote:Original post by BrianLarsenworker2{ 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 requiredvoid* 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 worker1while( 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"#endifcout << "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];Quote:Original post by Makaanthis 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)   