11

I'm attempting to solve the producer-consumer problem using pthreads and semaphores, but it looks like the producer threads aren't producing, and the consumer threads aren't consuming. It appears that the threads are being created:

  /* Do actual work from this point forward */
  /* Create the producer threads */
  for(c1=1; c1<=argarray[1]; c1++)
  {
    pthread_create(&tid, &attr, producer, NULL);
    printf("Creating producer #%d\n", c1);    
  }

  /* Create the consumer threads */
  for(c1=1; c1<=argarray[2]; c1++)
  {
    pthread_create(&tid, &attr, consumer, NULL);
    printf("Creating consumer #%d\n", c1);    
  }

because the "Creating producer #x" and "Creating consumer #x" are printed to the screen. It doesn't however, print from inside the threads themselves:

if(insert_item(item))
{
  fprintf(stderr, "Producer error.");
}
else
{
  printf("Producer produced %d\n", item);
}

likewise for consumer threads. Full code:

#include "buffer.h"
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <semaphore.h>

/* Create Buffer */
buffer_item buffer[BUFFER_SIZE];

/* Semaphore and Mutex lock */
sem_t cEmpty;
sem_t cFull;
pthread_mutex_t mutex;

/* Threads */
pthread_t tid; /* Thread ID */
pthread_attr_t attr; /* Thread attributes */

void *producer(void *param);
void *consumer(void *param);
void init();

/* Progress Counter */
int cg;

main(int argc, char *argv[])
{
  /* Variables */
  int argarray[3], c1;

  /* Argument counter checks */
  if(argc != 4)
  {
    fprintf(stderr, "usage: main [sleep time] [# of producer threads] [# of consumer threads]\n");
    return -1;
  }

  /* Get args from command line and change them into integers */
  argarray[0] = atoi(argv[1]); /* How long to sleep before ending */
  argarray[1] = atoi(argv[2]); /* Producer threads */
  argarray[2] = atoi(argv[3]); /* Consumer threads */

  /* Error check */
  if(argarray[1]<1)
  {
    fprintf(stderr, "argument 2 must be > 0\n");
    return -1;
  }
  if(argarray[2]<1)
  {
    fprintf(stderr, "argument 3 must be > 0\n");
    return -1;
  }    

  init();

  /* Do actual work from this point forward */
  /* Create the producer threads */
  for(c1=1; c1<=argarray[1]; c1++)
  {
    pthread_create(&tid, &attr, producer, NULL);
    printf("Creating producer #%d\n", c1);    
  }

  /* Create the consumer threads */
  for(c1=1; c1<=argarray[2]; c1++)
  {
    pthread_create(&tid, &attr, consumer, NULL);
    printf("Creating consumer #%d\n", c1);    
  }

  /* Ending it */
  sleep(argarray[0]);

  printf("Production complete.\n");
  exit(0);
}

void init()
{
  int c2;

  pthread_mutex_init(&mutex, NULL); /* Initialize mutex lock */
  pthread_attr_init(&attr); /* Initialize pthread attributes to default */
  sem_init(&cFull, 0, 0); /* Initialize full semaphore */
  sem_init(&cEmpty, 0, BUFFER_SIZE); /* Initialize empty semaphore */
  cg = 0; /* Initialize global counter */ 
  for(c2=0;c2<BUFFER_SIZE;c2++) /* Initialize buffer */
  {
    buffer[c2] = 0;
  }
}

void *producer(void *param)
{
  /* Variables */
  buffer_item item;

  while(1)
  { 
    sleep(rand());      
    item = (rand()); /* Generates random item */ 

    sem_wait(&cEmpty); /* Lock empty semaphore if not zero */
    pthread_mutex_lock(&mutex);

    if(insert_item(item))
    {
      fprintf(stderr, "Producer error."); 
    }
    else
    {
      printf("Producer produced %d\n", item); 
    }

    pthread_mutex_unlock(&mutex);
    sem_post(&cFull); /* Increment semaphore for # of full */
  }
}

void *consumer(void *param)
{
  buffer_item item;

  while(1)
  {
    sleep(rand());
    sem_wait(&cFull); /* Lock empty semaphore if not zero */
    pthread_mutex_lock(&mutex);
    if(remove_item(&item))
    {
      fprintf(stderr, "Consumer error."); 
    }
    else
    {
      printf("Consumer consumed %d\n", item);
    }

    pthread_mutex_unlock(&mutex);
    sem_post(&cEmpty); /* Increments semaphore for # of empty */
  }
}

int insert_item(buffer_item item)
{
  if(cg < BUFFER_SIZE) /* Buffer has space */
  {
    buffer[cg] = item;
    cg++;
    return 0;
  }
  else /* Buffer full */
  {
    return -1;
  }
}

int remove_item(buffer_item *item)
{
  if(cg > 0) /* Buffer has something in it */
  {
    *item = buffer[(cg-1)];
    cg--;
    return 0;
  }
  else /* Buffer empty */
  {
    return -1;
  }
}

Terminal output:

user@isanacom:~/Desktop/PCthreads$ ./main 10 10 10
Creating producer #1
Creating producer #2
Creating producer #3
Creating producer #4
Creating producer #5
Creating producer #6
Creating producer #7
Creating producer #8
Creating producer #9
Creating producer #10
Creating consumer #1
Creating consumer #2
Creating consumer #3
Creating consumer #4
Creating consumer #5
Creating consumer #6
Creating consumer #7
Creating consumer #8
Creating consumer #9
Creating consumer #10
Production complete.

As a beginner to multithreading, I'm sure its probably something simple I'm overlooking, and I appreciate the help.

Mike
  • 662
  • 3
  • 13
  • 27

3 Answers3

7

Both the consumer and producer does a sleep(rand()) which will sleep for a random number of seconds between 0 and MAX_INT, in the example you give the main thread will terminate after 10 seconds. If the rand() value of the producers are above 10 they will never have the chance produce anything.

Rickard
  • 7,239
  • 1
  • 18
  • 11
  • Nice catch. Actually though I think they've already invoked UB by calling `rand` (which is not thread-safe) from more than one thread, possibly at the same time. – R.. GitHub STOP HELPING ICE May 19 '11 at 17:43
  • Could I use the argument I pass in for sleep be used for the threads as well, to solve this problem? – Mike May 19 '11 at 17:48
  • You could try to just replace the sleep(rand()) with sleep(1) – Rickard May 19 '11 at 17:51
  • Could I call join function at the end of main function as a work around to this problem? – VaM999 Dec 17 '19 at 23:29
  • 1
    @VaM999 Yes, pthread_join() is one way of waiting for a thread to finish what it's doing. However in the example above, neither consumers or producers ever finish. – Rickard Dec 18 '19 at 09:29
3

You should start by using an array of threads...

pthread_t tid[argarray[1] + argarray[2]];

for(c1 = 0; c1 < argarray[1]; c1++)
{
    pthread_create(&tid[c1], &attr, producer, NULL);
    printf("Creating producer #%d\n", c1); 
}
for(c1 = 0; c1 < argarray[2]; c1++)
{
    pthread_create(&tid[c1 + argarray[1]], &attr, consumer, NULL);
    printf("Creating consumer #%d\n", c1); 
}

There may be other problems but this is the first one I see ...

malcolm
  • 31
  • 1
  • If the thread ID is not used in the app, there is no point in keeping it. Effectively dumping it by using one '&tid' in all the create loops is fine. – Martin James Mar 06 '13 at 17:33
  • It may even be that you can pass NULL as the pthread_t pointer, but I haven't tried it. – Martin James Mar 06 '13 at 17:36
2

You should not directly call exit at the end of your main function. You should first call pthread_join for the threads you created at the end, to keep the main thread alive until the other threads die.

gunan
  • 917
  • 5
  • 11
  • 1
    ..or use some other means of preventing main thread exit. Joining onto all those threads means keeping a reference to them for that one reason and a join() loop on them all. Big PITA. Easier to just wait for an input char, or use a sleep() loop, or something, than a messy join() thing. – Martin James Mar 06 '13 at 17:38