Slightly complicated thread synchronization

问题:

Background info: I am Trying to create a C program that allows me to search through several different files (2 in the source) for the largest prime number. The program is multi-threaded to speed up the process. In this program I am preferring computational time rather that wasted time by having a the threads wait until all of the threads have assigned globalLargestPrime.

The problem: I believe that in my program somewhere either the id is not being passed as a parameter properly, or that somewhere my program starves one of the threads.

The weird part: When I run my program it will run and finish but sometimes it will only spawn one thread, therefore it will not search through both text files. and other times it will spawn both threads and read from both text files

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

pthread_mutex_t mutex;
pthread_cond_t monitor[2];
int globalLargestPrime = 0;
const int numThreads = 2;
FILE *fIN[2];

typedef enum{
  FREE,
  IN_USE
}lrgstPrm;
lrgstPrm monLargestPrime;//create struct

int timed(){
 return time(NULL);
}

int ChkPrim(int n){
  int i;
  int isPrime = 0;
  int root = sqrt(n);
  for(i=2; i<root; i++){
        if(n % i == 0)
          isPrime = 1;
        else
          isPrime = 0;
     }
  return isPrime;
}

void *calc_sqrt(void *threadID){//Create Threads
  int index, currentNum;
  int localLargestPrime = 0;
  int id = *(int *)threadID;
  int thousandsOfTimes = 0;
  //FILE *fIN = fopen("data.txt", "r");

  //printf(PTHREAD_MUTEX_ERRORCHECK);
  //Calculate some sqrts
  while(!feof(fIN[id])){
  for(index = 0; index < 1000; index++ )
  {//Check every thousand times
    fscanf(fIN[id], "%d\n", &currentNum);
    if(currentNum>localLargestPrime)
      if(ChkPrim(currentNum) == 1)
        localLargestPrime = currentNum;
  }

    pthread_mutex_lock(&mutex);

    thousandsOfTimes++;

    while(monLargestPrime == IN_USE)
    pthread_cond_wait(&monitor[id], &mutex);//wait untill mutex is unlocked
    monLargestPrime = IN_USE;
    //Critical Zone
    printf("Entering Critical Zone My ID: %d\n",id);
    if(localLargestPrime > globalLargestPrime)//Check for largest num
      globalLargestPrime = localLargestPrime;
    else
      localLargestPrime = globalLargestPrime;

    for(index = 0; index < numThreads; index++)
      if(index != id)
        pthread_cond_signal(&monitor[id]);//signal all threads that mutex is unlocked
    monLargestPrime = FREE;
    printf("Exiting Critical Zone My ID: %d\n",id);
    pthread_mutex_unlock(&mutex);
 //   printf("done searching thousand times %d My ID: %d\n",thousandsOfTimes, id);
  }
}

void createText(){
  FILE *fOUT = fopen("data.txt", "w");
  int i;
  srand(time(NULL));
  for(i=0; i<10000; i++)
  fprintf(fOUT, "%d\n",rand()%5000);
  fclose(fOUT);
}


int main(){
   printf("This is before creating threads\n");
  int index, timeDiff;
  pthread_t threads[2];
  pthread_mutex_init(&mutex, NULL);
  for(index = 0; index < numThreads; index++)
    pthread_cond_init(&monitor[index], NULL);
  fIN[0] = fopen("data0.txt","r");
  fIN[1] = fopen("data1.txt","r");

  timeDiff = time(NULL);
  //createText();

  for(index = 0; index < numThreads; index++){
    //int *id = malloc(1);
    //*id = index;
    pthread_create(&threads[index],NULL,calc_sqrt,&index);
  }
  for(index = 0; index < numThreads; index++)
    pthread_join(threads[index],NULL);
  printf("This is after creating threads");

  timeDiff = timed() - timeDiff;

  /*Destroy the mutexes & conditional signals*/
  pthread_mutex_destroy(&mutex);
  pthread_cond_destroy(&monitor[0]);
  pthread_cond_destroy(&monitor[1]);


printf("This is the Time %d\n", timeDiff);
printf("This is the Largest Prime Number: %d", globalLargestPrime);
return 0;
}

If anyone Could please give me some insight on the issue it will be appreciated

Thanks

回答1:

You're passing in the address of the same local variable to the threads. Since the variable is updated as each thread is created, when a thread starts it will likely read a value that's intended for a different thread:

pthread_create(&threads[index],NULL,calc_sqrt,&index)
                                              ^^^^^^

What you'll end up with is multiple threads reading using the same FILE*.

Since you're passing in a simple int you can pass the value directly as the thread parameter:

pthread_create(&threads[index],NULL,calc_sqrt,(void*)index)

Then in the thread get the value like so:

int id = (int)threadID;

There's no need for the condition variable at all in your code (though again - I'm not sure it's causing a problem). Your condition variable tracks whether globalLargestPrime is in use by another thread or not. Coincidentally, the mutex does the same thing! try:

pthread_mutex_lock(&mutex);

thousandsOfTimes++;     // not sure why this local variable even exists, 
                        //  much less is in a critical section

//Critical Zone
printf("Entering Critical Zone My ID: %d\n",id);
if(localLargestPrime > globalLargestPrime)//Check for largest num
  globalLargestPrime = localLargestPrime;
else
  localLargestPrime = globalLargestPrime;   // again, not sure why this is here...

printf("Exiting Critical Zone My ID: %d\n",id);
pthread_mutex_unlock(&mutex);

Also, your code uses the antipattern of checking for EOF before reading the file:

while (!feof(somefile)) {
    // whatever...
}

which is wrong, though I think might be a harmless error in this instance. See:

• Common C Programming Errors: Using feof() incorrectly
• "while( !feof( file ) )" is always wrong

回答2:

For starters, you are malloc'ing only one byte, and assigning that to an int*. You should be malloc'ing sizeof(int).

Also, it would be much simpler if each thread would find the largest prime in its own file, then when each thread finishes, take the max of those results. No sense needing any synchronization between threads this way.