Multi-thread can't join properly

Question

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>
#include <condition_variable>
#include <atomic>
using namespace std;
mutex m;
condition_variable cov;
bool ready = false;
bool processed = false;
void showNum(int &f_, atomic_bool &alive_)
{
    while(alive_)
    {
        unique_lock<mutex> lk(m);
        cov.wait(lk,[]{ return ready;});
        f_++;
        ready = false;
        processed= true;
        lk.unlock();
        cout<<f_<<endl;
        cov.notify_one();
    }

}
int main() {
    vector<int> va;
    for (int i = 0; i < 10; ++i) {
        va.push_back(i);
    }
    int f = 0;
    atomic_bool alive{ true };


    std::thread t1(showNum,ref(f),ref(alive));
    auto sizeofVector = va.size();
    for (int j = 0; j < sizeofVector; ++j) {
        {
            lock_guard<mutex> lk0(m);
            f = va.back();
            cout<<f<<"    ";
            ready = true;
        }

        cov.notify_one();
        va.pop_back();
        {
            unique_lock<mutex> lk(m);
            cov.wait(lk,[]{return processed;});
            processed = false;
            lk.unlock();
        }

    }

    alive = false;
    t1.join();
    return 0;
}

I just want to test the condition variable in multi-thread. The code above is my test code.

the error is that the thread t1 can't join properly. I print the alive_, it is always true, can't be set to false by alive = false in the main thread.

I try to make alive a global variable, but still the same error.

Can you give me some advice?

[Rubber ducky](https://en.wikipedia.org/wiki/Rubber_duck_debugging) would like to know what wakes up the thread after `alive` is set to false. — user4581301, Mar 06 '19 at 06:29
As the @Christophe said, the thread t1 is stuck on the code cov.wait(lk,[]{ return ready;}); when the main thread set the alive to be false. — tingda zhuang, Mar 07 '19 at 00:53

score 2 · Accepted Answer · answered Mar 06 '19 at 08:03

Can be changed

cov.wait(lk,[]{ return ready;});

to

cov.wait(lk,[&alive_]{ return ready || !alive_;});
if (!alive_)
    break;

And below alive_=false; add the line

cov.notify_one();

The complete code is as follows

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>
#include <condition_variable>
#include <atomic>
using namespace std;
mutex m;
condition_variable cov;
bool ready = false;
bool processed = false;
void showNum(int &f_, atomic_bool &alive_)
{
    while(alive_)
    {
        unique_lock<mutex> lk(m);
        cov.wait(lk,[&alive_]{return ready || !alive_;});
        if (!alive_)
            break;
        f_++;
        ready = false;
        processed= true;
        lk.unlock();
        cout<<f_<<endl;
        cov.notify_one();
    }

}
int main() {
    vector<int> va;
    for (int i = 0; i < 10; ++i) {
        va.push_back(i);
    }
    int f = 0;
    atomic_bool alive{ true };


    std::thread t1(showNum,ref(f),ref(alive));
    auto sizeofVector = va.size();
    for (int j = 0; j < sizeofVector; ++j) {
        {
            lock_guard<mutex> lk0(m);
            f = va.back();
            cout<<f<<"    ";
            ready = true;
        }

        cov.notify_one();
        va.pop_back();
        {
            unique_lock<mutex> lk(m);
            cov.wait(lk,[]{return processed;});
            processed = false;
            lk.unlock();
        }

    }

    alive = false;
    cov.notify_one();

    t1.join();
    return 0;
}

But there is some problem with your code. I find the predicate can't be the parameter that income to the `showNum()`, so the `cov.wait(lk,[&alive_]{return ready || !alive_;});` will occur error because of the `alive_` is the parameter of `showNom()`. When I modify it to the global parameter `alive`, it passes. — tingda zhuang, Mar 06 '19 at 14:36
What error was reported, is 'alive_' not capture? What version is your g++ — ccxxshow, Mar 07 '19 at 02:15
yes, 'alive_' not capture， g++ version: g++ (Ubuntu 5.4.0-6ubuntu1~16.04.11) 5.4.0 20160609 — tingda zhuang, Mar 07 '19 at 05:37

score 1 · Answer 2 · answered Mar 06 '19 at 07:22

1

In t1 the function doesn't constantly test alive. You have designed it so that every loop starts with waiting on the condition variable. It then go sleep and wakeup only when notified. Unfortunately, when main sets alive to false, the t1 thread is still in a waiting state.

You can observe this easily:

void showNum(int &f_, atomic_bool &alive_)
{
    while(alive_)
    {   cout<<"waiting..."<<endl;   
        unique_lock<mutex> lk(m);
        cout<<"waiting more..."<<endl;
        cov.wait(lk,[]{ return ready;});  ///<<<<< stuck here 
        cout<<"go..."<<endl;
        f_++;
        ready = false;
        processed= true;
        lk.unlock();
        cout<<"  sn:"<<f_<<endl;
        cov.notify_one();
    }
}

It will wake up only if main would ensure one more notifification on the condition variable. Only at this moment would it exit from its waiting state, and after processing find out that alive is false .

To avoid being stuck forever, you could change your code and make use of wait_for() so that the function can check on time out if it should still stay alive.

answered Mar 06 '19 at 07:22

Christophe

68,716
7
72
138

1

In addition you need to set `ready` to `true`. Otherwise T1 will immediately go back to sleep. Have a look at the [condition_variable::wait](https://de.cppreference.com/w/cpp/thread/condition_variable/wait) function with predicate. – julians Mar 06 '19 at 07:28
@julians good point ! There should be a predicate. But the spurious wakeup is a risk and not a feature (more [here](http://www.modernescpp.com/index.php/c-core-guidelines-be-aware-of-the-traps-of-condition-variables)). So the predicate is IMHO not a replacement for the timeout, is it? – Christophe Mar 06 '19 at 07:43
I don't think it is a replacement for a timeout. As far as I understood it, without the predicate returning true, T1 won't wake up. Even if it notified. (This is also what I could observe when testing the code) – julians Mar 06 '19 at 07:52
@Christophe,@julians, Thanks for providing me your advice. wait_for is a choice to solve the problem, but a suit time interval should be used. Maybe it is not that elegant. My humble opinion. – tingda zhuang Mar 06 '19 at 14:23
@tingdazhuang Indeed, the timeout is not the most elegant approach. The clean way would be to implement the paragraphe just before the one on wait_for: ensure that the main would notify once more the waiting thread so that it can realize that it’s over. – Christophe Mar 06 '19 at 14:49
@Christophe, yard, I add a notify_one() at the end of the main thread. I will answer the question again to show my code. – tingda zhuang Mar 07 '19 at 00:45

score 0 · Answer 3 · answered Mar 07 '19 at 00:50

#include <iostream>
#include <vector>
#include <thread>
#include <mutex>
#include <condition_variable>
#include <atomic>
#include <chrono>
using namespace std;
mutex m;
condition_variable cov;
bool ready = false;
bool processed = false;
atomic_bool alive{ true };
void showNum(int &f_, atomic_bool &alive_)
{
    while(alive)
    {
        unique_lock<mutex> lk(m);
        cov.wait(lk,[]{ return ready || !alive;});
        if(!alive)
            break;
        f_++;
        ready = false;
        processed= true;
        lk.unlock();
        cout<<f_<<endl;
        cov.notify_one();
    }

}
int main() {
    vector<int> va;
    for (int i = 0; i < 10; ++i) {
        va.push_back(i);
    }
    int f = 0;



    std::thread t1(showNum,ref(f),ref(alive));
    auto sizeofVector = va.size();
    for (int j = 0; j < sizeofVector; ++j) {
        {
            lock_guard<mutex> lk0(m);
            f = va.back();
            cout<<f<<"    ";
            ready = true;
        }

        cov.notify_one();
        va.pop_back();
        {
            unique_lock<mutex> lk(m);
            cov.wait(lk,[]{return processed;});
            processed = false;
            lk.unlock();
        }

    }
    alive = false;
    cov.notify_one();
    t1.join();
    return 0;
}

Integrating the advice, I modify my code to be above. It output as I expected. Thanks for all the people provide the advice, best regards.

Multi-thread can't join properly

3 Answers3