3 Threads and Consumer-Producer problem with Semaphores

Question

Thread concurrency newbie here. So, I have a problem where my encrypt_thread sometimes hangs, and I'm pretty sure it is because of the way I've used my semaphores.

What My Program Does:

1. read_from_file is a thread that reads characters from a file into the input_buffer one char at a time. Every time it does, it reduces empty_in_input_buffer by 1, and increases full_in_input_buffer by 1. If empty_in_input is 0, it waits for a post(&empty_in_input_buffer) from the Encrypt thread.

2. Encrypt thread waits for a post(&full_in_input_buffer) from the Read thread to begin encryption of the character. It then calls post(&empty_in_input_buffer) and then wait(&empty_in_output_buffer). Once it places the encrypted character in the output_buffer, it calls post(&empty_in_output_buffer). Encrypt is the "middleman," waiting for a new character to be placed into the input_buffer and for a new slot to be free in the output_buffer.

3. Write thread calls wait(&full_in_output_buffer) to then resume execution and write the character in the output_buffer to a file. Once it does, it calls post(&empty_in_output_buffer).

I want to achieve maximum concurrency, so that, if read_from_file is placing a new character at slot 5 of input_buffer, then encrypt can still access slots 3 and 4 to encrypt and write to an output_buffer. I have not used a mutex to lock the input_buffer as the items in the buffer are not modified, only read, encrypted, and placed into an output_buffer, but I may be wrong with the assumption that I don't need one.

The program also prompts a user for a buffer_size at the very beginning. The thing is, my program hangs half of the time, and it works perfectly the other half, which indicates that I have a race condition I'm not accounting for. However, I have been unable to figure out where it happens.

Edit: fill_letter_arr simply creates a char array of the alphabet in uppercase and lowercase letters. is_in_letters just checks if a char is a letter in the alphabet. My encryption is very simple and it only encrypts letters.

Main thread:

int main(int argc, char *argv[]){
    //sem_init(&encrypt_signals_read, 0, 1); 
    int buf_size;
    file_in = fopen(argv[1], "r");
    file_out = fopen(argv[2], "w");

    take_input(&buf_size);

    struct thread_args args = {
        malloc(sizeof(char)*buf_size),
        malloc(sizeof(char)*buf_size),
        buf_size,
        0,
    };

    sem_init(&empty_in_input,0,buf_size); 
    sem_init(&empty_in_output,0,buf_size);
    sem_init(&full_in_input_buffer,0,0);
    sem_init(&full_in_output_buffer,0,0);

    //creating threads
    pthread_t read_thread,encrypt_thread,write_thread; 

    if(pthread_create(&read_thread,NULL,read_from_file,&args) != 0){
        printf("Error creating read thread!");
    }
    if(pthread_create(&encrypt_thread,NULL,encrypt,&args) != 0){
        printf("Error creating encrypt thread!");
    }
    if(pthread_create(&write_thread,NULL,write_to_file,&args) != 0){
        printf("Error creating write thread!");
    }

    pthread_join(read_thread,NULL);
    pthread_join(encrypt_thread,NULL);
    pthread_join(write_thread,NULL);

    fclose(file_in);
    fclose(file_out);
}

Read thread:

void* read_from_file(void* args){
    struct thread_args* shared = (struct thread_args*) args;
    char c = '0';
    int i = 0;
    int val,val1,val2,val3;
    if (file_in != NULL){

        do{

            c = fgetc(file_in);

            if(i >= shared->buffer_size)
                i = 0;

            sem_wait(&empty_in_input);
            shared->input_buffer[i] = c;
            sem_post(&full_in_input_buffer);
            i++;

        }while(c != EOF);

    }
     if (ferror(file_in) != 0 ) {
            fputs("Error reading file", stderr);
            exit(1);
    }
}

Encrypt thread:

void* encrypt(void* args){
    struct thread_args* shared = (struct thread_args*) args;
    int s = 1;
    int i = 0;
    char c = '0';
    int val,val1,val2,val3,val4;



    fill_letters_arr(0);

    do{

        if(i >= shared->buffer_size)
            i = 0;

        sem_wait(&full_in_input_buffer);
        c = shared->input_buffer[i];
        sem_post(&empty_in_input);

        if(is_in_letters(&c) == true){
            encrypt_letter(&s,&c);
        }

        sem_wait(&empty_in_output);
        shared->output_buffer[i] = c;
        sem_post(&full_in_output_buffer);
        i++;

    }while(c != EOF);
}

Write thread:

void* write_to_file(void* args){
    struct thread_args* shared = (struct thread_args*) args;
    char c = '0';
    int i = 0;
    int val,val1,val2,val3;


    if (file_out != NULL){

        while(c != EOF){

            if(i >= shared->buffer_size)
                i = 0;

            sem_wait(&full_in_output_buffer);
            c = shared->output_buffer[i];
            fputc(c,file_out);
            sem_post(&empty_in_output);
            i++;
        }

    }
     if (ferror(file_in) != 0 ) {
            fputs("Error reading file", stderr);
    }

}

Answer 1

You didn't post the complete, reproducible program, so guesses: 1) you have a dubious use of EOF. Conventionally, you view the return of getc as an int; this it can cover every valid character value as well as EOF. This also protects you against compilers that think unsigned chars are a reasonable default. At any rate, you should probably choose a different sentinal value to pass through your buffers to indicate EOF.

in :

if(is_in_letters(&c) == true){
    encrypt_letter(&s,&c);
}

2) Because you are giving it a negative value (EOF will truncate to (char)-1), so if you are using it as an index, you might get some surprising values.

3) If your encrypt_letter happens to generate a 0xff as the encrypted version, it will be interpreted by this and the final stages as EOF; but the first stage will still try to happily read characters.

Summary: EOF is not a character value, it is an int.