Simple Generator-monitor program with condition variables

Question

I created a simple program that uses condition variables to create synchronization between two threads. I'm getting a strange output that I cannot seem to find the solution to.

What the program does is, in the generator thread, produces 1000 random integers and checks to see if they are perfect squares. If the number is a perfect square then it signals the monitor thread which prints the square root of the number.

The problem I am having is most likely some sort of race condition, because the monitor simply isn't printing out the square root when the generator signals.

The strange part is that when I debug in gdb b stepping through each time the variable is_square changes, the problem is nonexistent.

Any insight would appreciated. I feel it has to do with my mutex or placement of conditions.

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <pthread.h>
#include <time.h>

int square_root;
int is_square = 0;
int done = 0;
int count = 0; //used to count how many perfect squares generator_func finds
int count1 = 0; //used to compare how many responses monitor makes to signal
pthread_mutex_t mutex;
pthread_cond_t mon;


void* generator_func(void* args){
  srand(time(NULL)); 
  int i, temp, sq;
  for(i = 0; i<1000; i++){
    temp = rand() % 10000;
    sq = sqrt((double)temp);
    if((pow(sq,2)) == temp){
      pthread_mutex_lock(&mutex);
      count++;
      square_root = sq;
      is_square = 1;
      fprintf(stderr, "Square root of %d is", temp);
      pthread_cond_signal(&mon);
      pthread_mutex_unlock(&mutex);
    } 
  }
  pthread_mutex_lock(&mutex);
  done = 1;
  is_square = -1;
  pthread_cond_signal(&mon);
  pthread_mutex_unlock(&mutex);
}


main(){
  pthread_t generator; //declare thread

  pthread_mutex_init(&mutex, NULL); //initialize mutex
  pthread_cond_init(&mon, NULL); //initialize condition variable


  pthread_create(&generator, NULL, generator_func, NULL); //create thread

  //monitor
  while(done != 1){
    pthread_mutex_lock(&mutex);
    while(is_square == 0){
      pthread_cond_wait(&mon, &mutex);
    }
    if(is_square == 1 && done != 1){
      count1++;
      fprintf(stderr, " %d\n", square_root);
      is_square = 0;
    }
    pthread_mutex_unlock(&mutex);
  }

  pthread_join(generator, NULL);

  printf("%d %d\n", count, count1); //shows inconsistency between generator and monitor
  pthread_mutex_destroy(&mutex);
  pthread_cond_destroy(&mon);
} 

Answer 1

All, I have solved this problem very, very easily. I am embarrassed for not catching this sooner, but I took a break from this code for a while. The solution is so trivial. All that was needed was an extra condition variable that the generator waits on until the monitor is finished. Here is the code.

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <pthread.h>
#include <time.h>

int square_root;
volatile int is_square = 0;
int done = 0;
int count = 0; //used to count how many perfect squares generator_func finds
int count1 = 0; //used to compare how many responses monitor makes to signal
pthread_mutex_t mutex;
pthread_cond_t mon, gen;


void* generator_func(void* args){
  srand(time(NULL)); 
  int i, temp, sq;
  for(i = 0; i<1000; i++){
    temp = rand() % 10000;
    sq = sqrt((double)temp);
    if((pow(sq,2)) == temp){
      pthread_mutex_lock(&mutex);
      count++;
      square_root = sq;
      is_square = 1;
      fprintf(stderr, "Square root of %d is", temp);
      pthread_cond_signal(&mon); //signal monitor
      pthread_cond_wait(&gen, &mutex); //wait for monitor to finish *Solution*
      pthread_mutex_unlock(&mutex);
    } 
  }
  pthread_mutex_lock(&mutex);
  done = 1;
  is_square = -1;
  pthread_cond_signal(&mon);
  pthread_mutex_unlock(&mutex);
}


main(){
  pthread_t generator; //declare thread

  pthread_mutex_init(&mutex, NULL); //initialize mutex
  pthread_cond_init(&mon, NULL); //initialize condition var for monitor
  pthread_cond_init(&gen, NULL); //initialize condition var for generator

  pthread_create(&generator, NULL, generator_func, NULL); //create thread

  //monitor
  pthread_mutex_lock(&mutex);
  while(done != 1 || is_square == 1){
    while(is_square == 0){
      pthread_cond_wait(&mon, &mutex);
    }
    if(is_square == 1 && done != 1){
      count1++;
      fprintf(stderr, " %d\n", square_root);
      is_square = 0;
    }
    pthread_cond_signal(&gen); //start generator back up *Solution*
  }
  pthread_mutex_unlock(&mutex);


  pthread_join(generator, NULL);

  printf("%d %d\n", count, count1); //shows inconsistency between generator and monitor
  pthread_mutex_destroy(&mutex);
  pthread_cond_destroy(&mon);
}

Answer 2

Be careful with conditional variables. There is one big pitfall here and it's probably happening to you: If you call signal on a conditional variable but no thread is waiting on it, the signal is lost.

My guess is that your main thread does not manage to reach the call to wait by the time the generator thread calls signal (it may call it several times by the time the main thread reaches wait), and that is why you are losing some values.

The debugger is not a good way of examining this, because it tends to make these subtle timing bugs go away.

Other problems:

1. The generator might finish before the main thread even reaches the while.
2. done should be volatile because it's modified cross-thread.

Answer 3

You need to protect all shared (global) variables by the mutexes. Firstly, this means moving the mutex calls in main:

pthread_mutex_lock(&mutex);
while(done != 1){
   while(is_square == 0){
...
}
pthread_mutex_unlock(&mutex);

Secondly, you should be aware that you'll only see a handful of square roots, not all that the generator found. If the generator is fast enough, you'll even see no square roots, because the subthread can finish running before main sees the while (done != 1) loop.

You might consider using a buffer to store the found square roots.

Answer 4

this code should do the trick:

#include <stdio.h>
#include <math.h>
#include <stdlib.h>
#include <pthread.h>
#include <time.h>

int square_root;
int is_square = 0;
int done = 0;
int count = 0; //used to count how many perfect squares generator_func finds
int count1 = 0; //used to compare how many responses monitor makes to signal
pthread_mutex_t mutex;
pthread_cond_t mon;


void* generator_func(void* args){

    //sleep(1);
    for(int j=0; j<1000; j++);  // Some delay to hit a time-slice

  srand(time(NULL)); 
  int i, temp, sq;
  for(i = 0; i<1000; i++){
    temp = rand() % 10000;
    sq = sqrt((double)temp);
    if((pow(sq,2)) == temp){
      pthread_mutex_lock(&mutex);
      count++;
      square_root = sq;
      is_square = 1;
      fprintf(stderr, "Square root of %d is", temp);
      pthread_cond_signal(&mon);
      pthread_mutex_unlock(&mutex);
    } 
  }
  pthread_mutex_lock(&mutex);
  done = 1;
  is_square = -1;
  pthread_cond_signal(&mon);
  pthread_mutex_unlock(&mutex);
}


int main(){
  pthread_t generator; //declare thread

  pthread_mutex_init(&mutex, NULL); //initialize mutex
  pthread_cond_init(&mon, NULL); //initialize condition variable


  pthread_create(&generator, NULL, generator_func, NULL); //create thread

  //monitor
    pthread_mutex_lock(&mutex);  // Protect your variables here
  while(done != 1){
    while(is_square == 0){
      pthread_cond_wait(&mon, &mutex);
    }
    if(is_square == 1 && done != 1){
      count1++;
      fprintf(stderr, " %d\n", square_root);
      is_square = 0;
    }
  }
    pthread_mutex_unlock(&mutex);

  pthread_join(generator, NULL);

  printf("%d %d\n", count, count1); //shows inconsistency between generator and monitor
  pthread_mutex_destroy(&mutex);
  pthread_cond_destroy(&mon);

return 0;
}

You can you a loop to delay that is long enough to hit a time slice before so that your main can rush to wait on the cond_signal(). In the alternative, is to have another mechanism to allow your generator_func() thread to wait on for the main thread to signal it.

Then again, there is a sleep() feature you can use if your library has support for it.

Hope it helped!

Cheers, Vern