c++11 - C++ 11: Calling a C++ function periodically

Question

I have put together a simple c++ timer class that is supposed to call a given function periodically from various examples on SO as follows:

#include <functional>
#include <chrono>
#include <future>
#include <cstdio>

class CallBackTimer
{
public:
    CallBackTimer()
    :_execute(false)
    {}

    void start(int interval, std::function<void(void)> func)
    {
        _execute = true;
        std::thread([&]()
        {
            while (_execute) {
                func();                   
                std::this_thread::sleep_for(
                std::chrono::milliseconds(interval));
            }
        }).detach();
    }

    void stop()
    {
        _execute = false;
    }

private:
    bool            _execute;
};

Now I want to call this from a C++ class as followsL

class Processor()
{
    void init()
    {
         timer.start(25, std::bind(&Processor::process, this));
    }

    void process()
    {
        std::cout << "Called" << std::endl;
    }
};

However, this calls with the error

terminate called after throwing an instance of 'std::bad_function_call'
what():  bad_function_call

Answer 1

The problem in your code is that your lambda expression inside your "start" function captures the local variables by reference, using the [&] syntax. This means that the lambda captures the interval and func variables by reference, which are both local variables to the start() function, and thus, they disappear after returning from that function. But, after returning from that function, the lambda is still alive inside the detached thread. That's when you get the "bad-function-call" exception because it tries to call func by reference to an object that no longer exists.

What you need to do is capture the local variables by value, with the [=] syntax on the lambda, as so:

void start(int interval, std::function<void(void)> func)
{
    _execute = true;
    std::thread([=]()
    {
        while (_execute) {
            func();                   
            std::this_thread::sleep_for(
            std::chrono::milliseconds(interval));
        }
    }).detach();
}

This works when I try it.

Or, you could also list out the values you want to capture more explicitly (which I generally recommend for lambdas):

void start(int interval, std::function<void(void)> func)
{
    _execute = true;
    std::thread([this, interval, func]()
    {
        while (_execute) {
            func();                   
            std::this_thread::sleep_for(
            std::chrono::milliseconds(interval));
        }
    }).detach();
}

EDIT

As others have pointed out, the use of a detached thread is not a great solution because you could easily forget to stop the thread and you have no way to check if it's already running. Also, you should probably make the _execute flag atomic, just to be sure it doesn't get optimized out and that the reads / writes are thread-safe. You could do this instead:

class CallBackTimer
{
public:
    CallBackTimer()
    :_execute(false)
    {}

    ~CallBackTimer() {
        if( _execute.load(std::memory_order_acquire) ) {
            stop();
        };
    }

    void stop()
    {
        _execute.store(false, std::memory_order_release);
        if( _thd.joinable() )
            _thd.join();
    }

    void start(int interval, std::function<void(void)> func)
    {
        if( _execute.load(std::memory_order_acquire) ) {
            stop();
        };
        _execute.store(true, std::memory_order_release);
        _thd = std::thread([this, interval, func]()
        {
            while (_execute.load(std::memory_order_acquire)) {
                func();                   
                std::this_thread::sleep_for(
                std::chrono::milliseconds(interval));
            }
        });
    }

    bool is_running() const noexcept {
        return ( _execute.load(std::memory_order_acquire) && 
                 _thd.joinable() );
    }

private:
    std::atomic<bool> _execute;
    std::thread _thd;
};