Creating a ticker thread

A new question about a revised version of the code in this question, based on the advice in the accepted answer can be found here.

I need to signal my program to do something every X seconds. So I decided to make a separate thread that sends such a signal using ZeroMQ. All other behaviour is also triggered by ZeroMQ messages, so this seems like a safe way to do it, with respect to multithreading.

I have never used threads in C++ before though, so I'd like some feedback on my code. Note that in the code below I have replaced the actual task with a simple cout statement.

EDIT: I just realized the thread could very well terminate between setting run = false and calling join, which causes a std::system_error to be thrown. Putting the join statement in an if-block with isjoinable still suffers from the same problem. How should I handle this? try-catch? Using a locking mechanism instead of an atomic bool?

Ticker.hpp

#include 
#include 
#include 

class Ticker {
private:
    const unsigned int interval;
    std::atomic_bool run;
    std::thread tickthread;
public:
    Ticker(unsigned int interval);
    Ticker(const Ticker& orig) = delete;
    virtual ~Ticker();
    void stop();
private:
    void tickfunction();
};

Ticker.cpp

#include "Ticker.hpp"
#include 

using namespace std;

Ticker::Ticker(unsigned int interval)
: interval(interval), run(true), tickthread(&Ticker::tickfunction, this) {
}

Ticker::~Ticker() {
    if (tickthread.joinable()) {
        stop();
    }
}

void Ticker::stop() {
    cout << "stopping..." << endl;
    run = false;
    // interrupt sleep?
    tickthread.join();
    cout << "joined" << endl;
}

void Ticker::tickfunction() {
    while (true) {
        this_thread::sleep_for(chrono::seconds(interval));
        if (run) {
            cout << "tick" << endl;
        } else {
            break;
        }
    }
}

There are also two specific things I'd like recommendations on:

-
How can I change this class to accept

I see some things that may help you improve your program.

Use include guards

You probably already know this, but the Ticker.hpp file should have include guards to allow it to be included in multiple files within the same project. The ordinary form would be something like this:

#ifndef TICKER_HPP
#define TICKER_HPP
// contents of Ticker.hpp go here
#endif //TICKER_HPP

Avoid data races

The tickfunction accesses std::cout but that's a single resource that might simultaneously be used by other threads. One fix for this is to use a std::mutex like this:

std::mutex cout_mutex;

// wherever cout is used:
some_function() {
    std::lock_guard lock(cout_mutex);
    std::cout << "Now we can do this safely!\n";
}

Note that std::lock_guard is intended to be used via RAII so that the lock is obtained when the object is created and released when it's destroyed, making it easy to avoid the bug of forgetting to unlock the mutex.

If the actual function called within your code each "tick" uses shared resources, you should apply the same technique.

Use standard durations

To answer your question, one way to use a standard duration would be to specify a particular one. For example, if you decided that millisecond resolution was enough for this program (and it probably is), the class declaration might look like this:

class Ticker {
private:
    const std::chrono::milliseconds interval;
    std::atomic_bool run;
    std::thread tickthread;
public:
    Ticker(std::chrono::milliseconds intval);
    Ticker(const Ticker& orig) = delete;
    virtual ~Ticker();
    void stop();
private:
    void tickfunction();
};

Use the chrono literals for more readable code

With the code modified to use a standards time interval, we can now use it with std::chrono_literals and make the code even easier to write and to read.

int main()
{
    using namespace std::chrono_literals;
    Ticker t(500ms);
    Ticker t10(10s);
    std::this_thread::sleep_for(15s);
}

Allow for thread cancellation

For various reasons, there is no simple standard way of cancelling a thread. However, there are almost always ways you can accomplish the equivalent in your own code. I'll show how this might be done in the next suggestion.

Prefer async to raw threads

C++ has a std::async mechanism that is a bit higher level than the more fundamental thread and mutex mechanism. The advantage to using it is that you can spend more time thinking about the problem you're trying to solve rather than the details of threads and locks. Here's one way of refactoring your code that doesn't use thread explicitly:

#include 
#include 
#include 

static std::mutex cout_mutex;

class Ticker 
{
public:
    // create but don't execute yet
    Ticker(std::shared_future f, std::chrono::milliseconds interval) 
    : interval{interval}, done{f}
    { }
    // run the thread
    std::future run() {
        return std::async(std::launch::async, &Ticker::tickWrapper, this);
    }
private:
    void tickWrapper() {
        std::future_status status;
        do {
            status = done.wait_for(interval); // waits for the signal from main()
            if (status == std::future_status::timeout) {
                tickfunction();
            }
        } while (status != std::future_status::ready);
    }
    void tickfunction() {
        std::lock_guard lock(cout_mutex);
        std::cout  done;
};

Note that I've separated tickWrapper from tickfunction. The latter is now just a simple function that doesn't contain any references to timers or futures. This allows us to separate the actual function from the mechanics of a recurring timer implementation. Here's a main function showing how to create and drive this new Ticker object:

int main()
{   
    // for convenience in specifying time durations
    using namespace std::chrono_literals;

    // create a promise and associated shared_future
    std::promise done;
    std::shared_future done_future(done.get_future());

    // create the Ticker objects 
    Ticker t1(done_future, 300ms);
    Ticker t2(done_future, 500ms);

    // start the threads
    auto x = t1.run();
    auto y = t2.run();

    // delay for a while 
    std::this_thread::sleep_for(3s);

    // tell the other threads to stop
    done.set_value();

    std::cout << "Done\n";
}

StackExchange Code Review Q#125579, answer score: 3