pthread not giving expected output

Question

I am trying to implement the Producer-Consumer problem operating system using semaphore and pthread. But my output is totally different from expected. Here is my code:

    #include<iostream>
    #include<pthread.h>
    #include<fstream>
    #include<unistd.h>
    #include<queue>

    // define queue size
    #define QUEUE_SIZE 5

    // declare and initialize semaphore and read/write counter
    static int semaphore = 1;
    static int counter = 0;

    // Queue for saving characters
    static std::queue<char> charQueue;

    // indicator for end of file
    static bool endOfFile = false;

    // save arrays
    char consumerArray1[100];
    char consumerArray2[100];

    // function to wait for semaphore
    void wait()
    {
        while(semaphore<=0);
        semaphore--;
    }

    // function to signal the wait function
    void signal()
    {
        semaphore++;
    }

    void *Producer(void *ptr)
    {
        int i=0;
        std::ifstream input("string.txt");
        char temp;
        while(input>>temp)
        {
            wait();
            charQueue.push(temp);
            //std::cout<<"Producer:\nCounter: "<<counter<<" Semaphore: "<<semaphore<<std::endl;
            counter++;
            std::cout<<"Procuder Index: "<<i<<std::endl;
            i++;
            signal();
            sleep(2);
        }
        endOfFile = true;
        pthread_exit(NULL);
    }

    void *Consumer1(void *ptr)
    {
        std::cout<<"Entered consumer 1:"<<std::endl;
        int i = 0;
        while(counter<=0);
        while(!endOfFile)
        {
            while(counter<=0);
            wait();
            //std::cout<<"Consumer1:\nCounter: "<<counter<<" Semaphore: "<<semaphore<<std::endl;
            consumerArray1[i] = charQueue.front();
            charQueue.pop();
            i++;
            counter--;
            std::cout<<"Consumer1 index:"<<i<<" char: "<<consumerArray1[i]<<std::endl;
            signal();
            sleep(2);
        }
        consumerArray1[i] = '\0';
        pthread_exit(NULL);
    }

    void *Consumer2(void *ptr)
    {
        std::cout<<"Entered consumer 2:"<<std::endl;
        int i = 0;
        while(counter<=0);
        while(!endOfFile)
        {
            while(counter<=0);
            wait();
            //std::cout<<"Consumer2:\nCounter: "<<counter<<" Semaphore: "<<semaphore<<std::endl;
            consumerArray2[i] = charQueue.front();
            charQueue.pop();
            i++;
            counter--;
            std::cout<<"Consumer2 index: "<<i<<" char: "<<consumerArray2[i]<<std::endl;
            signal();
            sleep(4);
        }
        consumerArray2[i] = '\0';
        pthread_exit(NULL);
    }

    int main()
    {
        pthread_t thread[3];
        pthread_create(&thread[0],NULL,Producer,NULL);
        int rc = pthread_create(&thread[1],NULL,Consumer1,NULL);
        if(rc)
        {
            std::cout<<"Thread not created"<<std::endl;
        }
        pthread_create(&thread[2],NULL,Consumer2,NULL);
        pthread_join(thread[0],NULL);pthread_join(thread[1],NULL);pthread_join(thread[2],NULL);
        std::cout<<"First array: "<<consumerArray1<<std::endl;
        std::cout<<"Second array: "<<consumerArray2<<std::endl;
        pthread_exit(NULL);
    }

The problem is my code, in some runs freezes(probably in an infinite loop) after the entire file has been read. And also both of the consumer functions read the same words even though I am popping it out after reading. Also the part of printing the array element that has been read just prints blank. Why are these problems happening? I am new to threads(as in coding using threads, I know theoretical concepts of threads) so please help me with this problem.

Answer 1

The pthreads standard prohibits accessing an object in one thread while another thread is, or might be, modifying it. Your wait and signal functions violate this rule by modifying semaphore (in signal) while a thread calling wait might be accessing it. You do this with counter as well.

If what you were doing in signal and wait were legal, you wouldn't need signal and wait. You could just access the queue directly the same way you access semaphore directly. If the queue needs protection (as I hope you know it does) then semaphore needs protection too and for exactly the same reason.

The compiler is permitted to optimize this code:

while(semaphore<=0);

To this code:

if (semaphore<=0) { while (1); }

Why? Because it knows that no other thread can possibly modify semaphore while this thread could be accessing it since that is prohibited by the standard. Therefore, there is no reason to read more than once.

You need to use actual sempahores and/or locks.