So I am trying to understand pthread_cond_t variables, but the problem is often sometimes pthread_cond_signal()/pthread_cond_broadcast() doesn't work and the sleeping threads are not woken up, leading to a deadlock in my code.
Is there a problem in code? What is the better/best way to use condition variables?
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <pthread.h>
#include <unistd.h>
pthread_mutex_t lock;
pthread_cond_t cv;
int count = 0;
void* routine(){
pthread_mutex_lock(&lock);
while(count!=5) pthread_cond_wait(&cv,&lock);
printf("count value is : %d\n", count);
pthread_mutex_unlock(&lock);
}
void* routine2(){
pthread_mutex_lock(&lock);
for(int i=0; i<7; i++){
count++;
printf("%d\n",count);
if(count == 5) pthread_cond_signal(&cv);
}
pthread_mutex_unlock(&lock);
}
int main(){
pthread_mutex_init(&lock,NULL);
pthread_cond_init(&cv,NULL);
pthread_t t1,t2;
pthread_create(&t1,NULL,&routine,NULL);
pthread_create(&t2,NULL,&routine2,NULL);
pthread_join(t1,NULL);
pthread_join(t2,NULL);
pthread_mutex_destroy(&lock);
pthread_cond_destroy(&cv);
}
The issue is that you're basing your logic on variables that can change between the time you read the variable and the time that you test the value.
In this code:
while(count!=5) pthread_cond_wait(&cv,&lock);
If routine() is called first this implicitly means that count is 0 (because routine2() is the only function that changes count, and does not have the lock). routine() will call pthread_cond_wait which will release the mutex lock and then block until the condition is signaled. Note that it also must be able to obtain the mutex lock before it finishes (i.e just the signal alone isn't sufficient). From pthread_cond_wait
Upon successful return, the mutex has been locked and is owned by the calling thread
If routine2() gets the lock first it will iterate until count is 5 and then call pthread_cond_signal. This will not however let routine() continue because the lock is not released at the same time. It will continue iterating through the loop and immediately increment count to 6, before routine() ever gets the chance to read from the count variable. As count cannot possibly be 5 at the time routine() resumes, is will deadlock (get stuck doing the above line forever).
It might seem like you could avoid this by simply not obtaining the lock with routine2():
void* routine2(){
for(int i=0; i<7; i++){
count++;
printf("%d\n",count);
if(count == 5) {
pthread_cond_signal(&cv);
}
}
}
However there are also problems with this, as there is no guarantee that count will be 5 when routine() is read it as there is nothing stopping routine2() from continuing to process. If that happens routine() will again deadlock as count will have been incremented above 5. This is also unadvisable as accessing a shared variable from more than one thread at a time is classified as nondeterministic behavior.
The change below resolves the issue with a minimal amount of changes. The one major change is to only wait if count is less than 5 (if it's 5 or more the signal was already sent).
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <pthread.h>
#include <unistd.h>
pthread_mutex_t lock;
pthread_cond_t cv;
int count = 0;
void* routine(){
pthread_mutex_lock(&lock);
if (count < 5) {
pthread_cond_wait(&cv,&lock);
printf("routine: got signal (%d)\n", count);
} else {
printf("routine: signal already sent\n");
}
pthread_mutex_unlock(&lock);
}
void* routine2(){
pthread_mutex_lock(&lock);
for(int i=0; i<7; i++){
count++;
printf("%d\n",count);
if(count == 5) pthread_cond_signal(&cv);
}
pthread_mutex_unlock(&lock);
}
int main(){
pthread_mutex_init(&lock,NULL);
pthread_cond_init(&cv,NULL);
pthread_t t1,t2;
pthread_create(&t1,NULL,&routine,NULL);
pthread_create(&t2,NULL,&routine2,NULL);
pthread_join(t1,NULL);
pthread_join(t2,NULL);
pthread_mutex_destroy(&lock);
pthread_cond_destroy(&cv);
}
This OnlineGDB snippet demonstrates the potential for deadlock.