Race condition in Producer Consumer Problem

Question

In the code below, I expected the print statement would always print s=0 (Since it is a binary semaphore and it has acquired the lock).

/// Producer Consumer problem using semaphores

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

#define BUFF_SIZ 10

int s = 1; /// binary semaphore
int full = 0;
int empt = BUFF_SIZ;

int arr[BUFF_SIZ];
int value = 100;

void wait(int *s)
{
    while(*s <= 0);
    *s = *s - 1;
}
void signal(int* s)
{
    *s = *s + 1;
}
void *producer(void* param)
{
    for(int i=0;i<BUFF_SIZ;i++)
    {
        int new_item = value;
        value++;
        wait(&empt);
        wait(&s);

        printf("s = %d\n",s);
        printf("Producer inside critical section\n");
        printf("Produced item = %d\n\n",new_item);
        arr[i] = new_item;

        signal(&s);
        signal(&full);
    }
    pthread_exit(0);
}
void *consumer(void* param)
{
    for(int i=0;i<BUFF_SIZ;i++)
    {
        wait(&full);
        wait(&s);

        printf("s = %d\n",s);
        printf("Consumer inside critical section\n");
        printf("Consumed item = %d\n\n", arr[i]);

        signal(&s);
        signal(&empt);
    }
    pthread_exit(0);
}
int main()
{
    pthread_t tid_p,tid_c;
    pthread_attr_t attr1,attr2;
    pthread_attr_init(&attr1);
    pthread_attr_init(&attr2);

    pthread_create(&tid_p,&attr1,producer,NULL);
    pthread_create(&tid_c,&attr2,consumer,NULL);

    pthread_join(tid_p,NULL);
    pthread_join(tid_c,NULL);
    return 0;
}

However the output is

s = 0
Producer inside critical section
Produced item = 100

s = 0
Producer inside critical section
Produced item = 101

s = 0
Producer inside critical section
Produced item = 102

s = -1
Consumer inside critical section
Consumed item = 100

s = 0
Producer inside critical section
Produced item = 103

s = 0
Producer inside critical section
Produced item = 104

s = -1
Consumer inside critical section
Consumed item = 101

s = 0
Consumer inside critical section
Consumed item = 102

s = 0
Consumer inside critical section
Consumed item = 103

s = 0
Consumer inside critical section
Consumed item = 104

s = -1
Producer inside critical section
Produced item = 105

s = 0
Producer inside critical section
Produced item = 106

s = 0
Producer inside critical section
Produced item = 107

s = -1
Consumer inside critical section
Consumed item = 105

s = 0
Consumer inside critical section
Consumed item = 106

s = 0
s = 0
Producer inside critical section
Produced item = 108

s = 0
Producer inside critical section
Produced item = 109

Consumer inside critical section
Consumed item = 107

s = 1
Consumer inside critical section
Consumed item = 108

s = 1
Consumer inside critical section
Consumed item = 109

So according to me, this causes race condition because of non-atomic operation of wait() function (Let me know if I am wrong). Is there any way by which I can avoid this ?

Answer 1

you could try replacing the s, full, empt integer by some pseudo semaphore:

struct pseudo_sem {
    int value; /// binary semaphore
    pthread_mutex_t mutex;
};
struct pseudo_sem s = { 1, PTHREAD_MUTEX_INITIALIZER };
struct pseudo_sem full = { 0, PTHREAD_MUTEX_INITIALIZER };
struct pseudo_sem empt = { BUFF_SIZ, PTHREAD_MUTEX_INITIALIZER };

void wait(struct pseudo_sem *s)
{
    pthread_mutex_lock(&s->mutex);
    while(s->value <= 0) {
        pthread_mutex_unlock(&s->mutex);
        usleep(100); // be kind, sleep a bit
        pthread_mutex_lock(&s->mutex);
    }   
    s->value--;
    pthread_mutex_unlock(&s->mutex);
}

void signal(struct pseudo_sem* s)
{
    pthread_mutex_lock(&s->mutex);
    s->value++;
    pthread_mutex_unlock(&s->mutex);
}

This could work but performs busy looping which a real semaphore does not do, that's why using a real semaphore would be far better IMHO