0

I have a custom Timer class in C++ that uses std::async to run a timer in a separate thread. The problem I'm facing is that when I call start() on the same Timer object consecutively with no delay, the timer does not execute the second time. However, if I add a small delay between the two start() calls, it works fine.

Here is the code I'm using:

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

class Timer {
public:
    Timer() : running(false), stopRequested(false), interval_(0), singleShot(false) {}

    void start(std::chrono::milliseconds interval) {
        stop();
        wait_for_thread_finish();

        {
            std::lock_guard<std::mutex> lock(mutex);
            interval_ = static_cast<int>(interval.count());
            running = true;
            stopRequested = false;
            startTime = std::chrono::steady_clock::now();
        }

        timerFuture = std::async(std::launch::async, &Timer::run, this);
    }

    void stop() {
        {
            std::lock_guard<std::mutex> lock(mutex);
            stopRequested = true;
            running = false;
        }
        cv.notify_all();
    }

    void wait_for_thread_finish() {
        if (timerFuture.valid()) {
            // Check thread status using wait_for
            auto status = timerFuture.wait_for(std::chrono::milliseconds(0));
            if (status == std::future_status::timeout) {
                // Thread is still running, wait for it to finish
                timerFuture.wait();
            }
        }
    }

    void run() {
        try {
            auto nextWakeUp = std::chrono::steady_clock::now() + std::chrono::milliseconds(interval_);
            while (true) {
                {
                    std::unique_lock<std::mutex> lock(mutex);
                    if (cv.wait_until(lock, nextWakeUp, [this] { return stopRequested.load(); }))
                        break;

                    if (!running.load()) break;
                }

                timeoutCallback();

                if (singleShot.load()) {
                    std::lock_guard<std::mutex> lock(mutex);
                    running = false;
                    break;
                }
                nextWakeUp = std::chrono::steady_clock::now() + std::chrono::milliseconds(interval_);
            }

            {
                std::lock_guard<std::mutex> lock(mutex);
                running = false;  // Ensure running is set to false after the loop exits
            }
        }
        catch (const std::exception& e) {
            // Handle exceptions
            std::cerr << "Error: " << e.what() << std::endl;
        }
        running = false;
    }

    void timeoutCallback() {
        std::cout << "Timer triggered!" << std::endl;
    }

private:
    std::atomic<bool> running;
    std::atomic<bool> stopRequested;
    int interval_;
    std::chrono::steady_clock::time_point startTime;
    std::atomic<bool> singleShot;

    std::mutex mutex;
    std::condition_variable cv;
    std::future<void> timerFuture;
};

int main() {
    Timer timer1;

    std::cout << "Starting timer 1 immediately...\n";
    
    timer1.start(std::chrono::milliseconds(1000));  // Timer 1 with 1-second interval
    timer1.start(std::chrono::milliseconds(1099));  // Starting again immediately
    
    // Wait for a while to observe the output
    std::this_thread::sleep_for(std::chrono::seconds(3));

    std::cout << "Stopping timers...\n";
    timer1.stop();

    // Ensure both timers finish execution before main exits
    timer1.wait_for_thread_finish();

    std::cout << "Main thread exiting...\n";
    return 0;
}

Issues:

  • When I start the timer1 immediately twice with no delay between the calls, the second timer does not seem to trigger.
  • However, when I add a small delay (e.g., std::this_thread::sleep_for(std::chrono::milliseconds(100));) between the two start() calls, both timers trigger as expected.

What I've tried:

  • I am calling stop() and wait_for_thread_finish() before starting the timer again to ensure the previous thread has finished, but the issue still occurs.
  • I thought it might be related to the state of the std::future or the mutex, but I am unsure why the second start does not work without the delay.

When I say "the second timer does not seem to trigger," I mean that the timeoutCallback() function (which simply prints "Timer triggered!") is not being called the second time, even though the start() method is called immediately after the first one.

What I observe:

  • When I call start() on timer1 with a 1-second interval, the first "Timer triggered!" message is printed after 1 second.
  • When I call start() again immediately with a 1.099-second interval, nothing happens—there is no subsequent output after the first message.
  • The second timer doesn't seem to trigger even though it should trigger after 1.099 seconds.

What I expect to observe:

  • After the first "Timer triggered!" message at 1 second, I expect to see another message at approximately 1.099 seconds, which should indicate that the second timer has been triggered.
  • If I add a small delay between the two start() calls (for example, std::this_thread::sleep_for(std::chrono::milliseconds(100));), both timers work as expected, and both messages are printed.

Has anyone faced this issue or can explain why this happens?

4
  • 1
    Add more debugging output in all your functions, to make tracing easier. Commented Jan 14 at 16:19
  • 1
    I don't think I understand your design. When you call start() twice on the same Timer object, you start two threads, but they're both using the same variables for interval_ and the other members, and the second call overwrites the values given to the first. In fact I think you have a data race, since the main thread writes to interval_, and the run() function reads it without holding the mutex. Commented Jan 14 at 16:25
  • 1
    It seems more like you want to create two separate Timer objects for your two timers. Commented Jan 14 at 16:26
  • 1
    I would be bringing out "address sanitizer", "thread sanitizer" and "undefined behaviour sanitizer" as a first attempt to debug this. Commented Jan 14 at 16:45

0

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.