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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.