Why isn't pthread_cond_signal() being called?

Viewed 33

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);
}
2 Answers

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() so it will call pthread_cond_wait, which will release the mutex lock and then wait until the condition is signaled. Not that it also must be able to obtain the mutex lock before it finishes. 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. When this happens routine() deadlocks.

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

Note that if you do this, there is no guarantee that count will be 5 when routine() 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:

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

You are using the condition variables and mutex correctly but the algorithm is failing. There are two conditions here:

  1. t1 must be woken up once count is equal to 5;
  2. 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;
Related