std::chrono::milliseconds(2000) vs 2000ms

Question

I am implementing a timer library,

In 2nd parameter of std::condition_variable::wait_until, std::chrono::steady_clock::now() + 2000ms is working fine, but std::chrono::steady_clock::now() + std::chrono::milliseconds(2000) is not.

Please guide me what I am missing, and how to use later.

/*
 * Inspired by:
 * https://www.fluentcpp.com/2018/12/28/timer-cpp/
 * */

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

using namespace std;

class Timer {
    std::condition_variable cv;
    mutable std::mutex m;
    bool is_killed = false;
    bool is_reset = false;
    std::thread active_thread;


    void dispose_thread() {
        if (active_thread.joinable()) {
            kill();
            active_thread.join();
        }
        is_killed = false; //safe, because all threads are dead
        is_reset = false;
    }

    auto lock() const {
        return std::unique_lock(m);
    }

public:
    [[maybe_unused]] void kill() {
        auto l = lock();
        is_killed = true;
        is_reset = false;
        cv.notify_all();
    }

    [[maybe_unused]] void reset() {
        auto l = lock();
        is_killed = false;
        is_reset = true;
        cv.notify_all();
    }

    ~Timer() { dispose_thread(); }

    template<class Function, class... Args, class Rep, class Period>
    [[maybe_unused]] void once(const std::chrono::duration<Rep, Period> &sleep_duration, Function &&f, Args &&... args);

    template<class Function, class... Args>
    [[maybe_unused]] void fix_time_once(Function &&f, Args &&... args);

};

template<class Function, class... Args, class Rep, class Period>
void Timer::once(const std::chrono::duration<Rep, Period> &sleep_duration, Function &&f, Args &&... args) {
    dispose_thread();
    active_thread = std::thread(
            [&, f = std::forward<Function>(f), args = std::make_tuple(std::forward<Args>(args)...)]() {
                auto l = lock();
                do {
                    is_reset = false;
                    cv.wait_until(l, std::chrono::steady_clock::now() + sleep_duration, [&] { return is_killed or is_reset; });
                    if (is_killed) return;
                } while (is_reset);

                std::apply(f, args);
            });
}


template<class Function, class... Args>
void Timer::fix_time_once(Function &&f, Args &&... args) {
    dispose_thread();
    active_thread = std::thread(
            [&, f = std::forward<Function>(f), args = std::make_tuple(std::forward<Args>(args)...)]() {
                auto l = lock();
                do {
                    is_reset = false;
                    cv.wait_until(l, std::chrono::steady_clock::now() + 2000ms, [&] { return is_killed or is_reset; });
                    if (is_killed) return;
                } while (is_reset);

                std::apply(f, args);
            });
}

using namespace std;

#define LOGTIME std::time(nullptr)
void func2(const std::string_view str, const std::string_view str2) {
    std::cout << LOGTIME << ": " << str << str2 << std::endl;
}


int main() {

    Timer t = Timer();
    Timer t2 = Timer();
    std::cout << LOGTIME << ": " << "Program Starting" << std::endl;
    t.once(std::chrono::milliseconds(2000), func2, "INCORRECT", " This should be std::chrono::milliseconds(2000) apart"); // not working as expected
    t2.fix_time_once(func2, "CORRECT", " This is running as expected"); // working as expected


    std::this_thread::sleep_for(std::chrono::seconds(10));
    t.kill();
    t2.kill();
    return 1;
}

Output:

1611935262: Program Starting                                                                                                                                                       
1611935262: INCORRECT This should be std::chrono::milliseconds(2000) apart                                                                                                         
1611935264: CORRECT This is running as expected  

Answer 1

You're passing a reference to a temporary std::chrono::milliseconds(2000) into Timer::once(), and subsequently into the lambda.

The temporary stops existing at the end of the full-expression (the ;), so sleep_duration becomes a dangling reference when the lambda is executed in another thread.

Pass sleep_duration by-value instead

1. Here:
   void Timer::once(std::chrono::duration<Rep, Period> sleep_duration, ...

2. And here:
   [&, sleep_duration, f = std::forward<Function>(f), ...]() {