Thread sync using conditional variables

Question

Can you review the code?

#include <stdio.h>
#include <pthread.h>
#include <windows.h>
#include "bin/include/pthread.h"
#include "bin/include/sched.h"
#include "bin/include/semaphore.h"
#define TCOUNT 10
#define WATCH_COUNT 12
int count = 0;
pthread_mutex_t count_mutex = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t count_threshold_cv = PTHREAD_COND_INITIALIZER;
int thread_ids[2] = {1,2};
/* Function prototypes */
void watch_count(int *idp);
void inc_count(int *idp);
int getDirFileCount(void);
/* End of fuction prototypes */
int main(void)
{
 pthread_t threads[2];
 int retCode = 0;
 int initialFileCount = getDirFileCount();
 count = initialFileCount;
 printf("No of Files initially : %d \n",initialFileCount);
 if ((retCode = pthread_create(&threads[0],NULL,(void *)&inc_count, &thread_ids[0]))) {
 perror("Thread creation error : can't able to create thread");
 }
 if ((retCode = pthread_create(&threads[1],NULL,(void *)&watch_count, &thread_ids[1]))) {
 perror("Thread creation error : can't able to create thread");
 }
 pthread_exit(NULL);
 return 0;
}
void watch_count(int *idp)
{
 pthread_mutex_lock(&count_mutex);
 //int watchCount = getDirFileCount();
 while (1) {
 printf("waiting for changes made to the directory \n");
 pthread_cond_wait(&count_threshold_cv,
 &count_mutex);
 printf("OK , now someone made chaneges to the directory\n");
 }
 pthread_mutex_unlock(&count_mutex);
}
void inc_count(int *idp)
{
 for (;;) {
 pthread_mutex_lock(&count_mutex);
 int countnow = getDirFileCount();
 printf("Number of files now: %d \n",countnow );
 if (count < countnow) {
 pthread_cond_signal(&count_threshold_cv);
 pthread_mutex_unlock(&count_mutex);
 return;
 }
 //pthread_cond_signal(&count_threshold_cv);
 pthread_mutex_unlock(&count_mutex);
 //pthread_exit(NULL)
 }
}
int getDirFileCount(void)
{
 int count = 0;
 WIN32_FIND_DATA fd;
 char *dirFilePath = "D:\\Test\\*";
 HANDLE h = FindFirstFile(dirFilePath, &fd);
 if (h != INVALID_HANDLE_VALUE) {
 do {
 count++;
 } while (FindNextFile(h, &fd));
 FindClose(h);
 }
 return count;
}

• 200_success: You tagged this question as linux, yet I see WIN32_FIND_DATA fd; char *dirFilePath = "D:\\Test\\*";which suggests that this code is for Windows?
• cslrnr: it is ported code , basically written in linux

Answer 1

Could be simplified

For what you are doing, using a mutex and a condition variable is more complex than necessary. You could just use a semaphore or signal instead. A mutex should guard some shared variable or state, but your code doesn't have any. Instead, you are using the mutex + condition variable simply to wake up another thread.

As far as your implementation goes, I have the following comments:

• A loop containing pthread_cond_wait() should always check a predicate variable to guard against spurious wakeups. This is in the documentation for pthread_cond_wait(). So for example:

  pthread_mutex_lock(&count_mutex);
  while (count == initialCount) {
   pthread_cond_wait(&count_threshold_cv, &count_mutex);
  }
  // Do something here...
  pthread_mutex_unlock(&count_mutex);
  

• The loop doing the work should lock the mutex for as short a time as possible. You only need to hold the mutex when you want to access shared variables. So for example:

  for (;;) {
   // Do this long work outside of the mutex
   int countnow = getDirFileCount();
   if (countnow > initialCount) {
   // Only lock the mutex when you need to actually need to access
   // shared state (in this case the variable "count").
   pthread_mutex_lock(&count_mutex);
   count = countnow;
   pthread_cond_signal(&count_threshold_cv);
   pthread_mutex_unlock(&count_mutex);
   return;
   }
  }