0

Like in my title, my goal is to use the producer consumer model to mimic the linux cat command(ouput the contents of the file to screen). When I try running the program using lab3 5 animals.txt (lab3 the compiled/renamed executable, 5 being the size of the bounded buffer, and test_alarm.cpp being the name of the file to be printed), lines in the ouput seem to have been outputted multiple times. I am not sure what could be causing this(maybe I am not deleting the contents of the buffer properly? Though I'm not sure why that would cause multiple prints), so any help is appreciated.

An oddpoint is when I change the positions of 'ifstream infile' from global to the producer func, the output seems to change as well.

Animals.txt (output using linux's 'cat')
I have a dog
Dogs are large
I also have a cat
Animals.txt (output using my program)
I have a dog
Dogs are large
I also have a cat

I have a dog
Dogs are large
I also have a cat
I have a dog
Dogs are large
I have a dog
(Latest output)
counter in prod is: 1
counter in prod is: 2
counter in prod is: 3
I have a dog
counter in consumer is: 3
Dogs are large
counter in consumer is: 2

Segmentation fault (core dumped)


code


#include <iostream>
#include <string>
#include <semaphore.h>
#include <fstream>
#include <unistd.h>
#include <mutex>
#include <condition_variable>
#include <thread>

using namespace std;

int counter = 0; //number of words in buffer
int buffmax; //size of buffer
string filename;
mutex mu;
condition_variable cond;
string* buffer;
bool flag = false;
ifstream infile; 


void produce()
{
        
        infile.open(filename);

        while(getline(infile, temp)) //!infile.eof()
        {
                unique_lock<mutex> locker(mu);
                cond.wait(locker, []() {return counter < buffmax; }); //if true then produce, if false then wait
                buffer[counter] = temp;
                //infile >> buffer[counter];
                counter++;
                //cout << "counter in prod is: " << counter << endl;
                locker.unlock();
                cond.notify_one(); //notify consumer thread
        }
        infile.close();
        flag = true;
}


void consume()
{
        
        while(true)
        {
                if (flag == true && counter <= 0)
                        break;
        unique_lock<mutex> locker(mu);
                cond.wait(locker, []() {return counter > 0; });
           
                cout << buffer[0] << endl;
                string *x = new string[buffmax];
                for(int i = 0; i < counter-1; i++) // reason for seg fault?
                {
                   x[i]=buffer[i+1];
                }
                delete [] buffer;
                buffer = NULL
                buffer = x;
                x = NULL;
                //cout << "counter in consumer is: " << counter << endl;
                counter--;
                
                locker.unlock();
                cond.notify_one(); //notify producer thread
        }
}


int main(int argc, char* argv[])
{
    string s = argv[1];
    buffmax = stoi(s); //size of the buffer
    filename = string(argv[2]); // name of file to be parsed
    buffer = new string[buffmax]; // creating the bounded buffer
    
    thread t1(produce);
    thread t2(consume);

    t1.join();
    t2.join();

    return 0;
}

Viktor
  • 21
  • 5
  • 3
    Recommended reading: [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-i-e-while-stream-eof-cons) – user4581301 Nov 14 '20 at 21:09
  • I'll take a look – Viktor Nov 14 '20 at 21:14
  • Print out the value of counter when entering each function's critical section and again when exiting. You should see you're not reducing it enough when consuming. – user4581301 Nov 14 '20 at 21:22
  • I redid the loop by creating a global string temp and doing while(infile >> temp). I ran the program then and noticed the first word of each sentence got cut off. Ex: 'have a dog' . I am assuming this has something to do with the counter issue you mentioned. Will look into it. – Viktor Nov 14 '20 at 21:36
  • `while(infile >> temp)` reads a single whitespace-delimited token (effectively the first word) into `temp`. If you never use `temp` for anything it is wasted. This also does not protect `getline(infile, buffer[counter]);` from undetected failure. What you want is something closer to `while (getline(infile, buffer[counter]))`, but since this would place modification of the buffer outside the protection of the mutex, bad smurf will ensue. – user4581301 Nov 14 '20 at 21:45
  • The easy hack is `while (getline(infile, temp)` and then inside the mutex lock `buffer[counter] = temp;`. You can speed this up a bit by reading directly into `buffer[counter]` if you bundle the critical section into a function and return a bool. – user4581301 Nov 14 '20 at 21:51
  • yes that was what I was confused by, how can I fix my while loop without taking my buffer outside the critical section? – Viktor Nov 14 '20 at 21:52
  • will try thanks for the suggestion – Viktor Nov 14 '20 at 21:54
  • 1
    Side note: `bool flag = false;` is used by both threads outside of synchronization. This is somehting you'll probably get away with, but if you don't it's a real mind to figure out what went wrong. consider replacing it with `std::atomic_flag`. – user4581301 Nov 14 '20 at 22:13
  • will do. currently working on the counter sync issue : producer prints 1, 2, 3, while consumer prints 2, 1, 0 – Viktor Nov 14 '20 at 22:19
  • I have updated my code and the latest ouput along with it. However, I seem to be getting a seg fault error. Not sure what I am not allowed to access in this case. The misaligned counters in prod and consumer can be ignored mostly since they depend on the cout position. – Viktor Nov 14 '20 at 23:05
  • for any1 in the future the code is now fully working – Viktor Nov 14 '20 at 23:58

1 Answers1

2

In consume

    for (int i = 0; i < counter; i++)
        cout << buffer[i] << endl; //output contents of buffer
    counter--;

consumes counter buffers and only decreases counter by 1. This could be solved by reducing counter to zero, but that pretty much defeats the point of using the buffer in the first place.

A potential proper solution would be to replace the linear buffer with a circular buffer or a queue.

user4581301
  • 33,082
  • 7
  • 33
  • 54
  • the assignment details the use of only static or dynamic arrays no use of vectors queues etc. Otherwise I personally would have used a vector. – Viktor Nov 14 '20 at 21:45
  • 1
    Since this is a homework question I have to be careful how much I implement for you. `std::queue` wouldn't be the right tool for this job anyway because it grows. I think an unspoken part of the assignment is for you to write your own queue. Look around for papers and documentation on lock-free queues. Implement one correctly and it does almost all of your work for you. – user4581301 Nov 14 '20 at 22:02