Thread sync using conditional variables

Can you review the code?

```
#include
#include
#include

#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 c

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;
    }
}

StackExchange Code Review Q#117673, answer score: 2