What is the proper way to use conditional variables?

97 views Asked by At

My assignment provides running code that uses a high percentage of the CPU when running. The goal is to reduce that amount by implementing conditional variables in the producer consumer problem.

I followed the instructions provided to my best understanding, by adding a wait() function on the producer side, just before the mutex is unlocked, then sending the signal from the else portion of the if-else statement.

            // if buffer is full, release mutex lock and check again
            if (shared_count == NITEMS){

                 // wait when full
                pthread_cond_wait(&cond1, &mutex);
                pthread_mutex_unlock(&mutex);

            }
            else{
                //signal
                pthread_cond_signal(&cond1);
                break;
            }

I did the same thing on the consumer side as well.

the issue is that my overall code is no longer giving any output.

I've included the complete code below in case it helps

/* minor3.c - using producer-consumer paradigm. */

#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

#define NITEMS 10       // number of items in shared buffer

// shared variables
char shared_buffer[NITEMS]; // echo buffer
int shared_count;       // item count

pthread_mutex_t mutex;      // pthread mutex
unsigned int prod_index = 0;    // producer index into shared buffer
unsigned int cons_index = 0;    // consumer index into shard buffer

// function prototypes
void * producer(void *arg);
void * consumer(void *arg);

// Consumer variables
pthread_cond_t cond1 = PTHREAD_COND_INITIALIZER;
pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER;

int main()
{
    pthread_t prod_tid, cons_tid1, cons_tid2;

    // initialize pthread variables
    pthread_mutex_init(&mutex, NULL);

    // initialize condition variables
    pthread_cond_init(&cond1, NULL);
    pthread_cond_init(&cond2, NULL);

    // start producer thread
    pthread_create(&prod_tid, NULL, producer, NULL);

    // start consumer threads
    pthread_create(&cons_tid1, NULL, consumer, NULL);
    pthread_create(&cons_tid2, NULL, consumer, NULL);

    // wait for threads to finish
    pthread_join(prod_tid, NULL);
    pthread_join(cons_tid1, NULL);
    pthread_join(cons_tid2, NULL);

    // clean up
    pthread_mutex_destroy(&mutex);

    pthread_cond_destroy(&cond1);
    pthread_cond_destroy(&cond2);

    return 0;
}

// producer thread executes this function
void * producer(void *arg)
{
    char key;

    printf("Enter text for producer to read and consumer to print, use Ctrl-C to exit.\n\n");

    // this loop has the producer read data in from stdin and place the read data on the shared buffer
    while (1)
    {
        // read input key
        scanf("%c", &key);

        // this loop is used to poll the shared buffer to see if it is full:
        // -- if full, unlock and loop again to keep polling
        // -- if not full, keep locked and proceed to place character on shared buffer
        while (1)
        {
            // acquire mutex lock
            pthread_mutex_lock(&mutex);

            // if buffer is full, release mutex lock and check again
            if (shared_count == NITEMS){

                 // wait when full
                pthread_cond_wait(&cond1, &mutex);
                pthread_mutex_unlock(&mutex);

            }
            else{
                //signal
                pthread_cond_signal(&cond1);
                break;
            }

        }

        // store key in shared buffer
        shared_buffer[prod_index] = key;

        // update shared count variable
        shared_count++;

        // update producer index
        if (prod_index == NITEMS - 1)
            prod_index = 0;
        else
            prod_index++;

        // release mutex lock
        pthread_mutex_unlock(&mutex); 
    }

    return NULL;
}

// consumer thread executes this function
void * consumer(void *arg)
{
    char key;

    long unsigned int id = (long unsigned int)pthread_self();

    // this loop has the consumer get data from the shared buffer and print to stdout
    while (1)
    {
        // this loop is used to poll the shared buffer to see if it is empty:
        // -- if empty, unlock and loop again to keep polling
        // -- if not empty, keep locked and proceed to get character from shared buffer
        while (1)
        {
            // acquire mutex lock
            pthread_mutex_lock(&mutex);

            // if buffer is empty, release mutex lock and check again
            if (shared_count == 0){
                
                pthread_cond_wait(&cond2, &mutex);
                pthread_mutex_unlock(&mutex);

                }
            else{

                pthread_cond_signal(&cond2);
                break;

                }
        }

        // read key from shared buffer
        key = shared_buffer[cons_index];

        // echo key
        printf("consumer %lu: %c\n", (long unsigned int) id, key);

        // update shared count variable
        shared_count--;

        // update consumer index
        if (cons_index == NITEMS - 1)
            cons_index = 0;
        else
            cons_index++;

        // release mutex lock
        pthread_mutex_unlock(&mutex);
    }

    return NULL;
}



1

There are 1 answers

2
David Schwartz On

You have two bugs.

First, you never set shared_count to zero. You increment, decrement, and test it. But you never initialize it.

Second, the consumers signal the condition variable that consumers wait for and vice versa. You need the consumers to signal the producers and vice versa!

Lastly, though it's not a bug, your loops needlessly unlock the mutex just to lock it again immediately on the next iteration. It makes more sense to unlock the mutex outside of the inner loop.

Here's the code with both bugs fixed and a more traditional looping structure:

/* minor3.c - using producer-consumer paradigm. */
  
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>

#define NITEMS 10       // number of items in shared buffer

// shared variables
char shared_buffer[NITEMS]; // echo buffer
int shared_count = 0;       // item count

pthread_mutex_t mutex;      // pthread mutex
unsigned int prod_index = 0;    // producer index into shared buffer
unsigned int cons_index = 0;    // consumer index into shard buffer

// function prototypes
void * producer(void *arg);
void * consumer(void *arg);

// Consumer variables
pthread_cond_t cond1 = PTHREAD_COND_INITIALIZER;
pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER;

int main()
{
    pthread_t prod_tid, cons_tid1, cons_tid2;

    // initialize pthread variables
    pthread_mutex_init(&mutex, NULL);

    // initialize condition variables
    pthread_cond_init(&cond1, NULL);
    pthread_cond_init(&cond2, NULL);

    // start producer thread
    pthread_create(&prod_tid, NULL, producer, NULL);

    // start consumer threads
    pthread_create(&cons_tid1, NULL, consumer, NULL);
    pthread_create(&cons_tid2, NULL, consumer, NULL);

    // wait for threads to finish
    pthread_join(prod_tid, NULL);
    pthread_join(cons_tid1, NULL);
    pthread_join(cons_tid2, NULL);

    // clean up
    pthread_mutex_destroy(&mutex);

    pthread_cond_destroy(&cond1);
    pthread_cond_destroy(&cond2);

    return 0;
}


// producer thread executes this function
void * producer(void *arg)
{
    char key;

    printf("Enter text for producer to read and consumer to print, use Ctrl-C to exit.\n\n");

    // this loop has the producer read data in from stdin and place the read data on the shared buffer
    while (1)
    {
        // read input key
        scanf("%c", &key);

        pthread_mutex_lock(&mutex);

        while (shared_count == NITEMS)
            pthread_cond_wait(&cond1, &mutex);

        // store key in shared buffer
        shared_buffer[prod_index] = key;

        // update shared count variable
        shared_count++;

        // update producer index
        if (prod_index == (NITEMS - 1))
            prod_index = 0;
        else
            prod_index++;

        // release mutex lock
        pthread_cond_signal(&cond2);
        pthread_mutex_unlock(&mutex); 
    }

    return NULL;
}

// consumer thread executes this function
void * consumer(void *arg)
{
    char key;

    long unsigned int id = (long unsigned int)pthread_self();

    // this loop has the consumer get data from the shared buffer and print to stdout
    while (1)
    {
        // this loop is used to poll the shared buffer to see if it is empty:
        // -- if empty, unlock and loop again to keep polling
        // -- if not empty, keep locked and proceed to get character from shared buffer
        pthread_mutex_lock(&mutex);
        while (shared_count == 0)
            pthread_cond_wait(&cond2, &mutex);

        // read key from shared buffer
        key = shared_buffer[cons_index];

        // echo key
        printf("consumer %lu: %c\n", (long unsigned int) id, key);

        // update shared count variable
        shared_count--;

        // update consumer index
        if (cons_index == (NITEMS - 1))
            cons_index = 0;
        else
            cons_index++;
        pthread_cond_signal(&cond1);

        // release mutex lock
        pthread_mutex_unlock(&mutex);
    }

    return NULL;
}