Why should I not pass a lambda with by-reference captures to the std::thread constructor?

Question

I wrote a timer class, but it does not work the way I needed it to. Can anyone tell me what is wrong with this?

template<typename D, typename C>
class timer
{
public:
    timer(D period, C&& callback)
    {
        std::thread t{[&](){
            std::this_thread::sleep_for(period);
            callback();
        }};

        t.detach();
    }
};

int main()
{
    timer t1(std::chrono::seconds(2), [](){
        std::cout << "hello from 1\n";
    });

    timer t2(std::chrono::seconds(3), [](){
        std::cout << "hello from 2\n";
    });

    std::this_thread::sleep_for(std::chrono::seconds(5));
}

The output is only:

hello from 1

Where is the 'hello from 2' line?

Answer 1

In your constructor:

timer(D period, C&& callback)
{
    std::thread t{[&](){
        std::this_thread::sleep_for(period);
        callback();
    }};

    t.detach();
}

period and callback are destroyed as timer returns, and timer returns way before callback is called, and possibly even before sleep_for is called.

The fix is to copy period and callback into the thread's functor so that those copies are still alive while the thread runs:

timer(D period, C&& callback)
{
    std::thread t{[=](){
        std::this_thread::sleep_for(period);
        callback();
    }};

    t.detach();
}