Collectives™ on Stack Overflow

Find centralized, trusted content and collaborate around the technologies you use most.

Learn more about Collectives

Teams

Q&A for work

Connect and share knowledge within a single location that is structured and easy to search.

Learn more about Teams

I'm currently thinking about the threadsafety of the QTimer implementation.

In my application I use the bool isActive() method to check if a timer is running or not. As I plan to use this method also from other threads, my thoughts went over to threadsafety considerations.

According to my research the method bool isActive() is not threadsafe.

Here are my assumptions:

The implementation of QTimer ( QTimer source code ) shows that bool isActive() just checks if the member-variable int id; is greater than 0:

inline bool isActive() const { return id >= 0; }
This member variable is initialized at the constructor with INV_TIMERwhich is a define to -1. When the timer is started, it will be set to the return-value of the int QObject::startTimer(int interval).
/*! \overload start()
    Starts or restarts the timer with the timeout specified in \l interval.
    If \l singleShot is true, the timer will be activated only once.
void QTimer::start()
    if (id != INV_TIMER)                        // stop running timer
        stop();
    nulltimer = (!inter && single);
    id = QObject::startTimer(inter);
When a call to isActive()is executed during QTimer::start()from another thread, in my opinion the returned value of bool isActive()could be invalid.
I would appreciate the opinion of someone who is able to verify my assumptions.
To reach thread safety, I would just wrap my call to the timer with a mutex, like in the code snippet shown below.
class SensorControl : public QObject
    Q_OBJECT
public:
    SensorControl();    // inits and interval-settings are done at implementation
    bool Start()
        QMutexLocker lock(&m_mutexTimer);
        return m_pTimer->start();
    void Stop()
        QMutexLocker lock(&m_mutexTimer);
        return m_pTimer->stop();
    bool IsMeasuring() const
        QMutexLocker lock(&m_mutexTimer);
        return m_pTimer->isActive();
private:
    QMutex m_mutexTimer;
    QTimer* m_pTimer;
                QTimer is not threadsafe, and not even reentrant. So not even a mutex in your code will make it safe.
– peppe
                May 8, 2017 at 17:24
If you want to only call QTimer::isActive from another thread, then your solution looks safe. isActive only accesses the id member variable, so you need to mutex-protect all writes to id, and the read of id from your thread. You did that for isActive and stop, so that looks good.
Note that if you ever call other methods of QTimer that write to id, you will get undefined behaviour. So take care to not call things like QTimer::setInterval(), QTimer::~QTimer() (!) and so on. Also don't use a singleshot timer, as that will write to id in QTimer::timerEvent().
In general wrapping an existing class and adding mutexes is dangerous, whether that works depends on the internals of said class, and those are hard to check for all cases. Also, internals might change in the next Qt version, maybe in the next version QTimer::timerEvent() will unconditionally change the id, and your solution is not thread safe anymore.
So while your approach works, in general I would recommend against it.
        
Thanks for contributing an answer to Stack Overflow!
Please be sure to answer the question. Provide details and share your research!
But avoid …
Asking for help, clarification, or responding to other answers.
Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.