I need some help understanding the details of accessing QObject subclasses from other threads.

From Qt documentation:

"If you are calling a function on an QObject subclass that doesn't live in the current thread and the object might receive events, you must protect all access to your QObject subclass's internal data with a mutex; otherwise, you may experience crashes or other undesired behavior."

Consider the following classes:

// Created on stack in main() => thread affinity = main thread
class Model : public QObject {
    Q_OBJECT
public:
    int getValue() const {
        return m_value;
    }
signals:
    void error(QString err);
public slots:
    void incrementValue() { 
        m_value  ; 
    };
    void threadError(QString err) {
        emit error(err);
    }
private:
    std::atomic<int> m_value = 0;
}


class Thread : public QThread {
    Q_OBJECT
public:
    Thread(Model* model) : QThread(), m_model(model) {}
signals:
    void incrementValue();
    void error(QString err);
private:
    void run() override; // calls m_model->getValue(), emits incrementValue()
    Model* m_model;
};

Also assume that there is a Widget class with a QPushButton that when clicked calls the incrementValue() slot. Thread also occasionally emits the incrementValue signal and this is connected to the incrementValue() slot.

There is a potential data race: incrementValue() may be called simultaneously from the main thread by the user clicking the QPushButton as getValue() from a non main thread by Thread::run(). However since the m_value variable is atomic this is safe?

I guess it comes down to what constitutes the "QObject subclass's internal data". In my example m_value clearly is internal data. What else? Does the Q_OBJECT macro expand to variables that I need to mutex protect?

There is nothing in the above that is specific to QObject. In other words I have the same potential problems with multiple threads as if Model did not inherit from QObject. Qt does not fix my multithread problems in this case for me, but do not either introduce any new problems. Is this correct?

To simplify the discussion: both Model and Thread are as pure C as possible. I do not use QTimer, QTcpSocket, event loop inside Thread or anything else Qt. Nor do I call moveToThread. The only thing I use Qt for in this context is its signal/slot mechanism (and QString).

One possible, but cumbersome, solution that I see is: rewrite Model to NOT subclass QObject. Introduce:

class Gui2Model : public QObject {
Q_OBJECT
public:
   Gui2Model(Model* model) : m_model(model) {}
signals:
   void error(QString err);
public slots:
   void incrementValue() {
      m_model->incrementValue();
   }
   void errorSlot(QString err) {
     emit error(err);
   }
private:
   Model* m_model;
};

Connect the increment QPushButton clicked signal to the Gui2Model::incrementValue() slot.

Signal errors from Thread by calling:

void Model::error(QString err) {
    QMetaObject::invokeMethod(m_gui2model, "errorSlot", Qt::QueuedConnection, Q_ARG(QString, err));
}

That way I take the full burden of synchronizing Model, but at least it happens according to rules which I (should) know from standard C .

CodePudding user response：

Is it sufficient to rewrite the m_value variable to be std::atomic?

Assuming you join the thread before the model is destroyed, I think yes.

There is nothing in the above that is specific to QObject.

I think they wanted to stress "the object might receive events" part, which may not be a direct call in your code, but a signal/slot connection. And signal/slot connections would be specific to QObject.

In a very twisted scenario, you could have both a signal from you thread and a direct call from your thread, which would create a race condition :-)

As a rule it may perhaps be OK to pass const Model* to the Thread object. It should be safe to call const methods on the Model since these do not change the state of the Model?

If the model is created const, then yes. Actually, there wouldn't be much choice in such case. But if the model is created non-const, and the value could change from the main thread while accessing it from your thread, then the access to it would still need to be synchronized.