c++multithreadingc++17stdthread

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


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?


Solution

  • 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();
    }