Timer with std::thread-CodePudding

I'm working on a timer class to perform operations on a different thread, sample code below is a copy from another SO question HERE

#include <thread>
#include <chrono>
#include <functional>

class Timer
{
public:
    ~Timer();
    Timer() noexcept;

    typedef std::chrono::milliseconds Interval;
    typedef std::function<void(void)> Timeout;

public:
    void start(const Interval& interval, const Timeout& timeout);
    void stop();

private:
    std::thread mThread;    /** Timer thread */
    bool mRunning = false;  /** Timer status */
};

Implementation with a comment where the problem will occur:

Timer::~Timer()
{
}

Timer::Timer() noexcept
{
}

void Timer::start(const Interval& interval, const Timeout& timeout)
{
    mRunning = true;

    mThread = std::thread([&]()
        {
            while (mRunning == true)
            {
                std::this_thread::sleep_for(interval);

                // std::abort will be called here
                timeout();
            }
        });
}

void Timer::stop()
{
    mRunning = false;
    mThread.join();
}

Sample to test the timer:

#include <iostream>

int main()
{
    Timer tm;

    tm.start(std::chrono::milliseconds(1000), []
        {
            std::cout << "Hello!" << std::endl;
        });

    std::this_thread::sleep_for(std::chrono::seconds(4));
    tm.stop();
}

I'm not able to understand why std::abort is called while executing the std::function within lambda and how do I resolve this?

CodePudding user response：

Arguments to your start function are passed by reference. In your lambda, you capture them by reference. By the time you come around calling that lambda, everything you've captured is destroyed, thus you're causing undefined behavior.

Additionally, make sure to either use atomic<bool> instead of a regular bool:

#include <thread>
#include <chrono>
#include <functional>
#include <cstdio>
#include <atomic>

class Timer {
public:
    ~Timer() {
        if (mRunning) {
            stop();
        }
    }
    typedef std::chrono::milliseconds Interval;
    typedef std::function<void(void)> Timeout;

    void start(const Interval &interval, const Timeout &timeout) {
        mRunning = true;

        mThread = std::thread([=]() {
            while (mRunning) {
                std::this_thread::sleep_for(interval);

                timeout();
            }
        });
    }
    void stop() {
        mRunning = false;
        mThread.join();
    }

private:
    std::thread mThread{};
    std::atomic_bool mRunning{};
};

int main() {
    Timer tm;

    tm.start(std::chrono::milliseconds(1000), [] {
        std::puts("Hello!");
    });

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

P.S. you might want to look into coroutines depending on where this idea is going.