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);
}
CodePudding user response：
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).
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);
}
}
}
There is 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.
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).
2)
#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.
CodePudding user response：
You are using the condition variables and mutex correctly but the algorithm is failing. There are two conditions here:
- t1 must be woken up once count is equal to 5;
- When count is equal to 5, it must not be changed until t1 is woken up.
Your algorithm is missing the second condition in t2. To make it, set a variable named t1_woken_up to indicate that t1 has been woken up to display the value of counter equal to 5.
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
#include <pthread.h>
#include <unistd.h>
pthread_mutex_t lock;
pthread_cond_t cv;
volatile int count = 0;
volatile int t1_woken_up = 0;
void* routine(){
pthread_mutex_lock(&lock);
while(count!=5) pthread_cond_wait(&cv,&lock);
printf("t1: count value is %d\n", count);
printf("t1: Waking up t2\n");
t1_woken_up = 1;
pthread_cond_signal(&cv);
pthread_mutex_unlock(&lock);
}
void* routine2(){
pthread_mutex_lock(&lock);
for(int i=0; i<7; i ){
count ;
printf("t2: %d\n",count);
if(count == 5) {
printf("t2: Waking up t1\n");
pthread_cond_signal(&cv);
while (!t1_woken_up) {
pthread_cond_wait(&cv,&lock);
}
}
}
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);
}
Example of execution:
$ gcc t.c -o t -lpthread
$ ./t
t2: 1
t2: 2
t2: 3
t2: 4
t2: 5
t2: Waking up t1
t1: count value is 5
t1: Waking up t2
t2: 6
t2: 7
NB: It is also advised to set the volatile attribute for the variables read/modified by several threads to ensure that the compiler will not optimize the code in each function arguing that "as they don't change in the function reading them, we can consider them as constant". Hence, the definition of count and t1_woken_up:
volatile int count = 0;
volatile int t1_woken_up = 0;