c++ using condition_variable as class member for a class that spawns threads

Question

im trying to synchronize a producer consumer case as following (the small reproducible instance of my scenario)

#include<thread>
#include<condition_variable>
#include<deque>


class Multi_Thread_Test{
public:

    std::mutex mu;
    std::condition_variable cond;

    std::deque<int>stored;
    Multi_Thread_Test(){

    }
    void Producer(){
        std::unique_lock<std::mutex>locker(mu);
        srand(time(0));
        stored.push_back(rand());
        locker.unlock();
        cond.notify_one();
    }
    void Producer_loop(){

        while(true){
            std::thread t1([&]{Producer();});
            std::thread t2([&]{Producer();});
            t1.join();
            t2.join();
            std::this_thread::sleep_for(std::chrono::seconds(5));
        }

    }
    void Consumer() {
        std::thread pl([&]{this->Producer_loop();});
        while (true) {

            std::unique_lock<std::mutex> locker(mu);
            cond.wait(locker, [&] { return !stored.empty(); });
            int temp = stored.front();
            stored.pop_front();
            locker.unlock();
        }
    }
};


int main() {
    
    Multi_Thread_Test test;
    test.Consumer();
    
    std::cin.get();
    std::cin.get();
    return 0;
}

the program compiles successfully but getting the following error when executing the program

Segmentation fault (core dumped)

Debug:SIGSEGV (signal SIGSEGV: invalid address (fault address: 0x0))

and when debugging it i found out that it occurs on cond.wait and cond.notify_one calls. ive captured by reference in lambada thread spawns but it seems the condition_variable might not captured the correct way. what is the cause of this error and how can i fix this or implement this logic in the correct way?

What should `std::unique_lock locker;` do? What does it lock and what are you trying to unlock with `locker.unlock();`? — Evg, Oct 05 '21 at 18:14
its main rule is to prevent busy waiting on the deque for the consumer — Amir Rasti, Oct 05 '21 at 18:18
`std::unique_locklocker(mu);` is more appropriate. Right now your 'unique lock' locks absolutely nothing. Or did it not occur to you that your mutex `mu` is utterly unused. — WhozCraig, Oct 05 '21 at 18:19
there is some operation on the temp variable which is the front of the deque and needed to be completed before getting the next front ,but ive didn't mention it as i thought it might been irrelevant — Amir Rasti, Oct 05 '21 at 18:21
It is entirely irrelevant. Smart scope locks need something to lock/unlock. You're have nothing. Just declaring a `std::unique_lock` locker; without attaching it to a `std::mutex` does absolutely nothing in the way of concurrency protection, and that is exactly what you're doing. Unrelated, after properly attaching the mutex to the unique-lock as I showed earlier, your end-scope `locker.unlock()` becomes pointless; the unique-lock will release it's contained mutex on destruction via scope exit anyway. More code != better code. — WhozCraig, Oct 05 '21 at 18:25
sorry about that , yes i noticed that now , i have accidentally forget that in the sample code (but it was attached in the main code), but ive test it now with the mutex attached but getting the same error . ill edit the post — Amir Rasti, Oct 05 '21 at 18:29
Make sure you compile exactly the code you provide here, i.e. that this is a [mcve]! Providing code X and asking about code Y isn't helpful. — Ulrich Eckhardt, Oct 05 '21 at 18:34
@Ulrich Eckhardt ,thanks but its almost the exact case and im getting the same segmentation fault on both the main code and the sample ive provided . — Amir Rasti, Oct 05 '21 at 18:36
In the producer, you are unlocking before you use the conditional var to notify one. Do not do so. In fact, just remove your unlock statements. The mutex will be unlocked when the locker object goes out of scope. — kalyanswaroop, Oct 05 '21 at 18:46
You are launching a new thread for each `push_back()`, and immediately join it. This is not how it is supposed to work. You are supposed to have each producer running its own loop in its own thread. You are not supposed to join anything until you you are completely done. — n. m. could be an AI, Oct 05 '21 at 18:46
im actually trying to allocate two threads for creating some content that is pushed into the deque ' Producers ' and one thread that use it 'Consumer',so that the maximum threads will be three — Amir Rasti, Oct 05 '21 at 18:50
im calling join to prevent the threads destruction before its finished — Amir Rasti, Oct 05 '21 at 18:52
Here is how **all** of your threads are supposed to look: `std::thread p1(this, Producer_Loop); std::thread p2(this, Producer_Loop);` That's it. No more threads. Adjust `Producer_Loop` accordingly (to run Producer in a loop, without launching any threads). — n. m. could be an AI, Oct 05 '21 at 18:56
@n. 1.8e9-where's-my-share m if you have implemented this without getting the segmentation fault please post it — Amir Rasti, Oct 05 '21 at 19:03
im statically linking on Linux ,could this be a possible reason? — Amir Rasti, Oct 06 '21 at 06:22

c++ using condition_variable as class member for a class that spawns threads

0 Answers0