Producer Consumer with Circular Buffer and Suspected Race Condition

Hey all,

I am relatively new to C programming and need to write a program that uses semaphores to control access to shared memory that places items into a circular buffer. The problem I have is one where the consumer runs out of things to consume and the producer doesn't wake up to place anything new into the buffer. I have the following code, which gives the output below it. I have been looking at this now for days and just seem to be going around in circles. Ay help would be appreciated.

Thanks,

Terry

Code:

#define _SVID_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <semaphore.h>
#include <fcntl.h>
#include <sys/uio.h>
#include <sys/shm.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
#include <pthread.h>

#define BUFFER_SIZE 10
#define ERROR_CHAR (-1)
#define MAX_FILE_SIZE 256

#define TRUE 1
#define FALSE 0

void buffer_char(char *c);
char* unbuffer_char(void);

typedef struct
{
    char file_path[BUFFER_SIZE][MAX_FILE_SIZE];
    int head;
    int tail;
    sem_t full;
    sem_t empty;
    sem_t mutex;
} Buffer;

Buffer *buffer_ptr;

int main(int argc, char** argv)
{
    int shmid;
    pid_t prod_pid, con_pid;

    /* shmid is the id of the shared memory address for our buffer */
    shmid = shmget(IPC_PRIVATE, getpagesize(), IPC_CREAT | 0666);

    /* get a pointer to our buffer in shared memory */
    buffer_ptr = (Buffer*) shmat(shmid, NULL, 0);

    /* initialise the buffer */
    buffer_ptr->head = 0;
    buffer_ptr->tail = 0;

    /* initialise our semaphores (2nd param 1 means shared betweeen processes */
    sem_init(&buffer_ptr->empty, 1, BUFFER_SIZE);
    sem_init(&buffer_ptr->full, 1, 0);
    sem_init(&buffer_ptr->mutex, 1, 1);

    /* spawn our child processes */
    prod_pid = fork();
    if (prod_pid == 0)
    {
        /* this is the producer process */
        int i;
        char items[15][10] = {"File 1", "File 2", "File 3", "File 4", "File 5", "File 6", "File 7", "File 8", "File 9", "File 10", "File 11", "File 12", "File 13", "File 14", "File 15"};

        for (i = 0; i < 15; i++)
        {
            sem_wait(&buffer_ptr->empty);
            sem_wait(&buffer_ptr->mutex);

            printf("Buffering %s\n", items[i]);
            buffer_char(items[i]);

            sem_post(&buffer_ptr->mutex);
            sem_post(&buffer_ptr->full);
        }
        exit(0);
    }

    con_pid = fork();
    if (con_pid == 0)
    {
        /* this is the consumer process */
        while (TRUE)
        {
            sem_wait(&buffer_ptr->full);
            sem_wait(&buffer_ptr->mutex);

            printf("Unbeffering %s\n", unbuffer_char());

            sem_post(&buffer_ptr->mutex);
            sem_post(&buffer_ptr->empty);
        }
        exit(0);
    }


    if (prod_pid != 0 && con_pid != 0)
    {
        /* this is the main process so wait here for both processes to finish before closing out and finishing */
        wait(prod_pid);
        wait(con_pid);
        printf("************************************");

        /* detach the shared memory and deallocate the memory segment */
        shmdt(&buffer_ptr);
        shmctl(shmid, IPC_RMID, 0);

        /* finally, close the signature file */

        return (EXIT_SUCCESS);
    }
}

void buffer_char(char* c)
{
    /* Use modulo as a trick to wrap around the end of the buffer back to the beginning */
    if ((buffer_ptr->tail + 1) % BUFFER_SIZE != buffer_ptr->head)
    {
        strcpy(buffer_ptr->file_path[buffer_ptr->tail], c);
        buffer_ptr->tail = (buffer_ptr->tail + 1) % BUFFER_SIZE;
    }
}

char* unbuffer_char(void)
{
    if (buffer_ptr->tail != buffer_ptr->head)
    {
        char *temp = buffer_ptr->file_path[buffer_ptr->head];
        buffer_ptr->head = (buffer_ptr->head + 1) % BUFFER_SIZE;

        printf("Head: %d\n", buffer_ptr->head);
        printf("Tail: %d\n", buffer_ptr->tail);

        return (temp);
    }
    else
    {
        printf("Problem is here!\n");
        printf("Head: %d\n", buffer_ptr->head);
        printf("Tail: %d\n", buffer_ptr->tail);
    }
}

Output:

Code:

Buffering File 1
Buffering File 2
Buffering File 3
Buffering File 4
Buffering File 5
Buffering File 6
Buffering File 7
Buffering File 8
Buffering File 9
Buffering File 10
Head: 1
Tail: 9
Unbeffering File 1
Head: 2
Tail: 9
Unbeffering File 2
Head: 3
Tail: 9
Unbeffering File 3
Head: 4
Tail: 9
Unbeffering File 4
Head: 5
Tail: 9
Unbeffering File 5
Head: 6
Tail: 9
Unbeffering File 6
Head: 7
Tail: 9
Unbeffering File 7
Head: 8
Tail: 9
Unbeffering File 8
Head: 9
Tail: 9
Unbeffering File 9
Problem is here!
Head: 9
Tail: 9

Does it have anything to do with falling off the end of a function which expects to return a result?

Code:

$ gcc -Wall -c foo.c
foo.c: In function ‘main’:
foo.c:42: warning: implicit declaration of function ‘getpagesize’
foo.c:99: warning: implicit declaration of function ‘wait’
foo.c: In function ‘unbuffer_char’:
foo.c:141: warning: control reaches end of non-void function
foo.c: In function ‘main’:
foo.c:111: warning: control reaches end of non-void function

You'll get a garbage answer, which you then try and "print", and no doubt, it blows up.

You should try to understand the code that you're copying:
http://en.wikipedia.org/wiki/Circula..._One_Slot_Open

You have a 9 slot queue, and a 10 count semaphore.

getpagesize() is no longer part of Posix. "sizeof(Buffer)" is sufficient.

"wait()" doesn't do what you think it does. You should include <sys/wait.h> and call waitpid() in a loop.
http://www.opengroup.org/onlinepubs/...s/waitpid.html

unbuffer_char() should return 0 if the queue is empty.
buffer_char() should log and return an error also if the queue is full.

Normally, the semaphore's would prevent dequeue-on-empty and enque-on-full. However, error checking and logging provides the first indication that things aren't "normal".

You mostly have a Posix-compliant source - so instead of "_SVID_SOURCE", I would use "-D_XOPEN_SOURCE=700" when compiling instead.

Here's how I would compile that code using gcc:
gcc foo.c -Wall -pedantic -std=c99 -D_XOPEN_SOURCE=700 -pthread

Even though you're not using pthreads, "-pthread" will link the libs needed for semaphores as well as any necessary platform-specific defines and libraries used in mult-threading. Alternatively, "-lrt" will provide the semaphore routines.

gg

Thanks for the feedback guys.

T.