passing bool by reference and use its latest value

Question

I have the following code:

MyClass::aMethod()
{
    ...
    bool isStarted = false;

    boost::thread(boost::bind(&MyClass::CheckTimeoutThread, this, isStarted));

    ...//some time later
    isStarted = true;
    ...
}

MyClass::checkTimeoutThread(bool &isStarted)
{
    ...//some code here
    while(anotherFlag)
    {
        ...//do something
        if(isStarted)//ALWAYS THE INITIAL VALUE OF FALSE
        {
        }
        ...
    }
}

I was expected the isStarted variable can be used as a flag but I am wrong or I was doing something wrong.

juanchopanza · Accepted Answer · 2013-12-04T06:12:31.243

3

The boost::thread will store copy of its arguments. But you can simulate a reference by passing a reference_wrapper with the help of boost::ref. Also note that you don't need to call boost::bind:

boost::thread t(&MyClass::CheckTimeoutThread, this, boost::ref(isStarted));

But note that you now have a race condiiton on isStarted. You need to either synchrise access to it with, say, mutexes, or use an atomic type, if available on your platform (if you have C++11, then you can use std::atomic<bool>, as suggested by @PeteBecker.)

Edit Obviously all of the above assumes that isStated lives at least as long as the thread that uses it. In practice, this means the thread must be done by the time aMethod returns. Alternatively, you can make isStarted a data member. But you still have to make sure to join the thread before the MyClass destructor destroys it.

edited Dec 04 '13 at 06:12

answered Dec 03 '13 at 21:40

juanchopanza

223,364
34
402
480

Deleting my answer and upvoting yours: didn't notice the absence of [tag:c++11]. – Casey Dec 03 '13 at 21:42
when to call boost::bind and when not? we call it all over the place. – 5YrsLaterDBA Dec 03 '13 at 21:51
That takes care of the first problem. The second is that the write to `isStarted` in one thread and the read in another are not synchronized, so there is no guarantee that the second thread will see the updated value. In C++11 this would call for an `std::atomic`. – Pete Becker Dec 03 '13 at 21:52
@5YrsLaterDBA For example if you need a callable object, `boost::function f = boost::bind.....` – juanchopanza Dec 03 '13 at 21:53
@PeteBecker You are right. In my case, that check is not critical but it should be synchronized anyway. If it is not synchronized, the value may not be the latest one is the ONLY issue or it may cause crash when both threads try to access the same location at the same time? – 5YrsLaterDBA Dec 03 '13 at 22:11
@5YrsLaterDBA - in C++11, the behavior without synchronization is undefined. In practice, there's a possibility that the thread will wait a **long** time for the updated value. – Pete Becker Dec 03 '13 at 23:02
@sehe I was assuming that amongst all the `...` there was a join in `aMethod`. – juanchopanza Dec 04 '13 at 06:07
Yes, **checkTimeoutThread** will exit before **isStarted** will be out of the scope. Otherwise I will use a class variable instead. – 5YrsLaterDBA Dec 04 '13 at 14:24

score 1 · Answer 2 · edited May 23 '17 at 10:34

The biggest problem (besides std::ref or boost::ref required to pass references to thread), is that you are passing a reference to a temporary

MyClass::aMethod()
{
    ...
    bool isStarted = false;

}

/// Oops at the end of the method, `isStarted` no longer lives

Any access through the stale reference is Undefined Behaviour after control leaves aMethod

May I suggest using proper synchronization: See it Live on Coliru

#include <thread>
#include <mutex>
#include <condition_variable>
#include <iostream>

struct X
{
    std::condition_variable cv;
    std::mutex mx;
    bool isStarted;
    std::thread worker;

    X() : isStarted(false) {}

    void aMethod()
    {
        worker = std::thread(&X::Worker, this);

        std::this_thread::sleep_for(std::chrono::seconds(1));

        {
            std::cout << "signalling worker\n";
            std::unique_lock<std::mutex> lock(mx);
            isStarted = true;
            cv.notify_one();
        }

        worker.join();
        isStarted = false;
    }

    void Worker()
    {
        std::unique_lock<std::mutex> lock(mx);
        std::cout << "worker ready\n";
        cv.wait(lock, [this] { return isStarted; });
        std::cout << "worker started\n";

        std::cout << "worker done\n";
    }
};

int main()
{
    X o;
    o.aMethod();
}

Note that

if you s/std/boost you should be able to use this on C++03 compilers
you almost don't need isStarted anymore in this case. I added it because of spurious wake-ups: Why does pthread_cond_wait have spurious wakeups?

passing bool by reference and use its latest value

2 Answers2