3
\$\begingroup\$

I am trying to get over my fear of multithreading programming and teaching myself to code using POSIX. I wrote a small version of the consumer producer problem. I am hoping I can get some feedback if it works (I think it does, as I traced the threads when my for loop was at 20 and it made sense) and what I should do to improve my code.

The semaphore starts at 4, the consumer waits one second, and then consumes a semaphore. The producer waits between 0 and 5 seconds, then produces (increments) the semaphore value. The consumer should wait if the semaphore is at 0 until the producer frees up the resources.

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

sem_t my_semaphore;
pthread_mutex_t count_mutex;
void * consumerUse(void * ptr);
void * producerUse(void * ptr);

int main() {

    int value;
    pthread_t consumer, producer;
    int valConsumer =0;
    int valProducer =0;

    sem_init(&my_semaphore, 0, 4);
    sem_getvalue(&my_semaphore, &value);
    printf("The initial value of the semaphore is %d\n", value);

    for(int i=0; i < 100; i++)
    {

        valConsumer = pthread_create(& consumer, NULL, consumerUse, (void*)(intptr_t) value);
        if(valConsumer != 0)
        {
            fprintf(stderr, "Error - pthread create () return code: %d\n", valConsumer);
            exit(EXIT_FAILURE);
        }

        valProducer = pthread_create(& producer, NULL, producerUse, (void*) (intptr_t)value);
        if(valProducer != 0)
        {
            fprintf(stderr, "Error - pthread create () return code: %d\n", valProducer);
            exit(EXIT_FAILURE);
        }

    }
    pthread_join( consumer, NULL);
    pthread_join( producer, NULL);
    printf("End program\n");
    return 0;
}

void *consumerUse(void * ptr)
{
    sleep(1);
    int value;
    sem_getvalue(&my_semaphore, &value);
    printf("Before wait: %d\n", value);
    pthread_mutex_lock(&count_mutex);
    sem_wait(&my_semaphore);
    pthread_mutex_unlock(&count_mutex);
    sem_getvalue(&my_semaphore, &value);
    printf("Consumer semaphore value: %d\n", value);
    return 0;
}

void *producerUse(void * ptr)
{

    int pause = rand() % 5;
    sleep(pause);
    int value;
    sem_getvalue(&my_semaphore, &value);
    printf("Before post: %d\n", value);
    if(value < 4)
    {

        sem_post(&my_semaphore);
        sem_getvalue(&my_semaphore, &value);
        printf("Producer semaphore value: %d\n", value);
    }
    return 0;
}
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

General note

It is very hard to tell if this program works or not: I don't know what is supposed to be a correct behaviour. I can only speculate that you didn't want to see a semaphore value ever to exceed 4. If it is the case, than it is a pure coincidence -- caused by the large delays (try to remove sleep()). Absolutely nothing in the code prevents two or more producers to "simultaneously" test the value, and proceed to post the semaphore.

Particular problems

There is no reason to serialize consumers' access to a sem_wait(). In a sense it defeats the purpose of the semaphore.

stdio is not thread safe. The fact that printouts haven't messed up is another coincidence, also caused by the sleep() delays.

The code creates a multitude of threads, but joins only two. Not a problem in a toy program; a bad resource leak in general.

A minor note: since the thread functions ignore the argument, you should pass NULL. Passing anything else (like (void *) (intptr_t) value) is misleading.

\$\endgroup\$
3
  • \$\begingroup\$ Can you be more specific about stdio not being thread safe. I have read on several posts the stdio is an atomic operation. Wouldn't that make it thread safe? \$\endgroup\$
    – Aaron
    Jul 16, 2014 at 21:28
  • \$\begingroup\$ My wrong choice of words, sorry. It is thread safe in a sense that calling them from different threads wouldn't cause bad things such as memory corruption. However, they are not reentrant and/or atomic. See for example this discussion: stackoverflow.com/questions/13190254/… \$\endgroup\$
    – vnp
    Jul 16, 2014 at 22:32
  • \$\begingroup\$ Makes sense, it seems that adding flockfile(stdout); before printf and funlockfile(stdout); after take care off that problem \$\endgroup\$
    – Aaron
    Jul 16, 2014 at 22:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.