CODEWITHSUNDEEP
c++multithreadingmutex
TheEngineer
TheEngineer

Reputation: 832

Communication between threads not working - C++

I am trying to write a simple example of the classic communication between two threads, producer and consumer. The producer locks the mutex, produces random string messages and push them up to the queue and release the lock. Then consumer locks the mutex and prints that data on the screen. For some reason, after running the code I get blank terminal for some times and then program terminates without any outputs! Here is my code:

#include <iostream>
#include <stdlib.h>
#include <thread>
#include <mutex>
#include <queue>
#include <random>
#include <string>
#include <cstdlib>

using namespace std;

static mutex mmutex;
static condition_variable mcond;
static queue <string> mqueue;

void consumer() {
    while (true) {
        unique_lock<mutex> lck{mmutex};
        mcond.wait(lck);
        string new_string = "producer has not produced yet ";
        string m = "";
        if (!mqueue.empty()) {
            m = mqueue.front();
            mqueue.pop();
            string new_string = "producer produced " + m;
        }   
        cout << new_string << endl;
        lck.unlock();
    }
}

void producer() {
    while (true) {
        string new_msg = NULL;
        unique_lock<mutex> lck{mmutex};
        int random = rand() % 40 + 40;
        new_msg = "New Random Char is "+static_cast <char> (random);
        mqueue.push(new_msg);
        mcond.notify_one();
    }
}




int main() {

    thread t1{ producer };
    thread t2{ consumer };


    t1.join();
    t2.join();
    cout << "exiting"<<endl;
    system("PAUSE");
    exit(0);
}

Upvotes: 0

Views: 446

Answers (2)

Geezer
Geezer

Reputation: 5710

Overall you're getting the synchronization scheme just fine. Other than that the code has one run-time error, a few unintended consequences of working with std::strings and an unnecessary (and potentially misleading to the reader) call to: unlock() on a std::unique_ptr.

after running the code I get blank terminal for some times and then program terminates without any outputs

It hangs and terminates due to assigning pointer to null to an std::string:

string new_msg = NULL;

As you can see here, this will cause the std::string instance to try access this address of zero: (

Second, you cannot get what you wanted by concatenating a string literal with a char as in these lines:

string new_string = "producer produced " + m;

and

new_msg = "New Random Char is "+static_cast <char> (random);

Here's a working, slightly better written version of your thread procedures, in which you can see various valid ways to initialize and assign to an std::string to get what you intended. Note again the removal of lck.unlock(); as std::unique_lock is an RAII object that will release the mutex upon its destruction at the while scope's exit as is:

void consumer() {
    while (true) {
        unique_lock<mutex> lck{ mmutex };
        mcond.wait(lck);
        string new_string;
        if (!mqueue.empty()) {
            string m = mqueue.front();
            mqueue.pop();
            new_string = string("producer produced ") + m;
        }
        else
        {
            new_string = "producer has not produced yet ";
        }
        cout << new_string << endl;
        //lck.unlock(); // <--- Not the intended usage of `unique_lock`
    }
}

void producer() {
    while (true) {
        string new_msg("New Random Char is ");
        unique_lock<mutex> lck{ mmutex };
        int random = rand() % 40 + 40;
        new_msg += static_cast<char>(random);
        mqueue.push(new_msg);
        mcond.notify_one();
    }
}

Output:

producer produced New Random Char is F
producer produced New Random Char is =
producer produced New Random Char is K
producer produced New Random Char is N
producer produced New Random Char is *
producer produced New Random Char is :
producer produced New Random Char is 3
producer produced New Random Char is O
producer produced New Random Char is @
producer produced New Random Char is E
producer produced New Random Char is I
...

Upvotes: 2

mksteve
mksteve

Reputation: 13073

This code is not in great shape. If you run it in a debugger, you quickly find why it stops, which is ....

string new_msg = NULL;

This is an access violation where the bytes at 0 (NULL) are read to get a string.

Corrected :

string new_msg;  // default value is empty.

With that removed, there are a couple of necessary changes to get the code closer to intended behavior..

new_msg = "New Random Char is "+static_cast <char> (random);

This doesn't work, as it takes the address of the string, and adds 40 to 80 bytes to it. Which moves out of the string to some "random location". The original C compatibility is hitting here, and the correct way of doing what ( I believe ) is intended is ....

new_msg = string("New Random Char is ") +static_cast <char> (random);

When converted to a std::string, the + now behaves as the append operator.

Finally in the consumer...

string new_string = "producer produced " + m;

needs to be

new_string = "producer produced " + m;

Otherwise the variable new_string which is used to cout the results is not the same as the variable which reads the queue.

Finally in my testing the consumer was not keeping up with the producer, and there needs to be some form of throttling and end condition.

Upvotes: 1

Related Questions