QML signal to C++ slot in worker thread

mzimmers

@jeremy_k I understand (I think) what you're saying, but I don't know how best to go about fixing this. Eventually, there will be dozens if not hundreds of signals coming from my QML code that need to be handled by the C++ back end.

So, should I give my worker object a parent, and if so, does it matter what the parent is?

Or, should I create the m_worker object within the context of the worker thread? This isn't difficult, as I can add a method to the Worker class to instantiate all the necessary members, but that seems a little bit of a hack.

I'm hoping for a clean solution that doesn't entail an intermediate object to relay signals and slots to the worker thread (this works, but it doubles the coding effort).

Thanks...

J.Hilk

@mzimmers don't invoke the worker slots directly, call SIGNALS.

The QObject::connect will handle the cross threading. If you expect a return value, change that to void and use signals and Qml Connections

JoeCFD

@mzimmers create a model(or qobject class) and connected it with your worker. Then call your model when the button is clicked. Emit a signal in the model in this call to worker. I guess your code communicates with worker using signal and slot

the order?

 worker->moveToThread(&thread);   
 thread.start();

mzimmers

@J-Hilk said in QML signal to C++ slot in worker thread:

don't invoke the worker slots directly, call SIGNALS.

I assume you mean emit a signal, right? Where and how do I make the call to QObject::connect() in this case?

mzimmers

@JoeCFD said in QML signal to C++ slot in worker thread:

create a model(or qobject class) and connected it with your worker.

Same question as above: how/where to create the connection?

JoeCFD

@mzimmers

int main(int argc, char *argv[])
{
   QGuiApplication app(argc, argv);
   Worker *worker;
   QThread thread(&app);
   worker = new Worker();
   auto model = new Model; /* whatever you call it */
   engine.rootContext()->setContextProperty("model", model);
   connect( model, signal, worker, slot );
   worker->moveToThread(&thread)
   thread.start();

class Model: QObject
{
public:
      void onButtonClicked() { /* call this from qml and you know how to do it */
           emit signal;
      }
}

J.Hilk

@mzimmers said in QML signal to C++ slot in worker thread:

@J-Hilk said in QML signal to C++ slot in worker thread:

don't invoke the worker slots directly, call SIGNALS.

I assume you mean emit a signal, right? Where and how do I make the call to QObject::connect() in this case?

No I mean literally call :D emit is just IDE sugar, it has no literal meaning.

Make your connect in the constructor. Threat affinity between signal and slot is done during execution, as long as you have the connect set to AutoConnection (the default) or QueuedConnection

mzimmers

@JoeCFD that is essentially what I did as a workaround. I created a class QmlStuff in the main thread and give it a slot:

void QmlStuff::relayConnect(QString host, quint16 port) {
    emit connectionRequested(host, port);
}

In main, the first thing I do after creating the Worker object is call a Worker function that does the connect.

This approach works, but it's going to double the amount of code I write. I'll do it if it's the best way; I was just hoping for something a little more straightforward.

mzimmers

@J-Hilk I'm still not sure I follow you. Are you suggesting:

create a signal in my QML
make the connection in the worker c'tor between the QML signal and a handler in my Worker object (*) EDIT: this is where I was having trouble referencing the QML object in my C++ code.
inside my onPressed(), call the QML signal

is that about right?

(*) I've been avoiding performing the connections in the c'tor, but if it's safe to do so, I can definitely do that.

Thanks...

mzimmers

So, I've made some progress, though it seems to be a limited solution. Following @J-Hilk 's suggestion (sort of), I've done this:

int main(int argc, char *argv[])
{
    QGuiApplication app(argc, argv);
    Worker *worker;
    QThread thread(&app);
    QQmlApplicationEngine engine;
    qmlRegisterType<Worker>("Worker", 1, 0, "Worker");
	
    worker = new Worker();
    engine.rootContext()->setContextProperty("worker", worker);
    engine.loadFromModule("restapi", "Main");

    QObject *rootObject = engine.rootObjects().constFirst();
    QObject::connect(rootObject, SIGNAL(sendRequested()), worker, SLOT(sendGet()));
    QObject::connect(&thread, &QThread::finished, worker, &Worker::deleteLater);
    worker->moveToThread(&thread);
    thread.start();

    pp.exec();
    thread.quit();
    return 0;
}

So, THAT connection is working. BUT: in my Worker class:

class Worker : public QObject
{
    Q_OBJECT
    Q_PROPERTY(QString reply READ reply WRITE setReply NOTIFY replyChanged)
public:
    explicit Worker(QObject *parent = nullptr);
    QString reply() {return m_reply; }
	void setReply(QString reply)
	{
	    if (m_reply != reply) {
		m_reply = reply;
		emit replyChanged(reply); // STILL GETTING ERROR HERE
	    }
	}
signals:
    void replyChanged(QString reply);
public slots:
    void sendGet();
private:
    QString m_reply;
private slots:
    void replyFinished(QNetworkReply *reply) {
	    QByteArray qba = reply->readAll();
	    setReply(QString(qba));
	    reply->deleteLater();
	}
}

I'm getting the original error (different parent object):

QObject: Cannot create children for a parent that is in a different thread.
(Parent is QQuickTextDocumentWithImageResources(0x1aafb094ef0), parent's thread is QThread(0x1aaf60b9b20), current thread is QThread(0x2243bff7d0)

So, evidently I still don't have this solved. Anyone have any ideas?

Thanks...

JoeCFD

@mzimmers said in QML signal to C++ slot in worker thread:

setReply

which object calls setReply? Any func called from outside of worked is better to /should be a slot. Otherwise, your GUI code could be blocked.

The error says the caller of setReply has a different thread id from the one (worker->moveToThread(&thread))used in your worker.

mzimmers

@JoeCFD

// worker.h
class Worker : public QObject
{
    Q_OBJECT
    Q_PROPERTY(QString reply READ reply WRITE setReply NOTIFY replyChanged)
    ...

// worker.cpp
Worker::Worker(QObject *parent) : QObject(parent)
{
    QObject::connect(&m_manager, &QNetworkAccessManager::finished, this, &Worker::replyFinished);
}
...
void Worker::replyFinished(QNetworkReply *reply) {
    QByteArray qba = reply->readAll();
    setReply(QString(qba));
    reply->deleteLater();
}
void Worker::setReply(QString reply)
{
    if (m_reply != reply) {
        m_reply = reply;
        emit replyChanged(reply);
    }
}

JoeCFD

@mzimmers
I can not run your code. I have some code like the following to make thread safe

void MyWorker::stop()
{
    /* make thread safe */
    if ( QThread::currentThread() != thread() ) {
        metaObject()->invokeMethod( this, "stop", Qt::QueuedConnection );
        return;
    }

   === do something ===
    emit finished();
}

is replyFinished a slot? If yes, it is called from a different thread. Then setReply(QString(qba)); is called from this thread as well. This thread is different from the thread inside your worker.

mzimmers

@JoeCFD said in QML signal to C++ slot in worker thread:

is replyFinished a slot? If yes, it is called from a different thread. Then setReply(QString(qba)); is called from this thread as well. This thread is different from the thread inside your worker.

Yes, it is a slot, and it seems to be called from within the worker thread (which makes sense, because it's a worker function).

I remember your suggestion about creating an intermediate object for this, but...that will double the code needed for handling every QML signal. There must be a better way to go about this.

J.Hilk

@mzimmers

something like this:

class Helper: public QObject{
    Q_OBJECT

public:
    explicit Helper(QObject *parent = nullptr) : QObject(parent), m_tUpdateProgress(this)
    {
        m_tUpdateProgress.setInterval(1000); // 1000ms -> evry second
        connect(&m_tUpdateProgress, &QTimer::timeout, this, &Helper::doWork);
        connect(this, &Helper::startTimerSignal, this, &Helper::start);
        connect(this, &Helper::stopTimerSignal, this, &Helper::stop);
        connect(this, &Helper::progressChanged, this, [=](int value)->void{qDebug() << value;});
    }

public slots:
    void start(){
        m_progress = 0;
        m_tUpdateProgress.start();
    }
    void stop(){
        m_tUpdateProgress.stop();
    }
signals:
    void progressChanged(int);
    void startTimerSignal();
    void stopTimerSignal();

private:
    void doWork(){
        m_progress++;
        emit progressChanged(m_progress);
    }

private:
    int32_t m_progress{0};
    QTimer m_tUpdateProgress;
};

//#include "clock.h"
//#include "sliderreporter.h"
#include <QThread>

int main(int argc, char *argv[])
{
    QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
//    SliderReporter sRep;
    QGuiApplication app(argc, argv);
    QQmlApplicationEngine engine;

    Helper helper;
    QThread thread;
    helper.moveToThread(&thread);
    thread.start();
    QObject::connect(&app, &QCoreApplication::aboutToQuit, &thread, &QThread::quit);

    engine.rootContext()->setContextProperty("CppThreadedObject", &helper);

    const QUrl url(QStringLiteral("qrc:/main.qml"));
    engine.load(url);



    return app.exec();
}

#include "main.moc"

Window {
    id:root
    visible: true
    width: 640
    height: 480
    title: qsTr("test")

    Item {
        id: mainItem
        anchors.fill: parent

        RowLayout {
            anchors.fill: parent
            spacing: 6

            Label {
                id:lbl

                //!Illeagal
//                Connections {
//                    target: CppThreadedObject
//                    function onProgressChanged(value) {lbl.text = value}
//                }
            }

            Button {
                text: "Start"

                onClicked: CppThreadedObject.startTimerSignal()
            }

            Button{
                text: "Stop"

                onClicked: CppThreadedObject.stopTimerSignal()
            }
        }

    }
}

If you want to receive signals from the worker, that, afaik, will only be possible via a wrapper/helper class like @JoeCFD suggested

PS: I knew this was somewhat similar:
https://forum.qt.io/topic/93912/queued-connection-from-qml

mzimmers

@J-Hilk said in QML signal to C++ slot in worker thread:

If you want to receive signals from the worker, that, afaik, will only be possible via a wrapper/helper class

I'm sorry, but I just don't get this. In my Worker class, I have this:

Q_PROPERTY(QString reply READ reply WRITE setReply NOTIFY replyChanged)

I've confirmed that READ and WRITE work. Moreover, I have a similar property for m_portNbr whose setter looks like this:

void Worker::setPortNbr(quint16 pn)
{
    if (m_portNbr != pn) {
        m_portNbr = pn;
        emit portNbrChanged(m_portNbr);
    }
}

And this works just fine (at least I don't get that error message. So, why doesn't THIS work?

void Worker::setReply(QString reply) {
    if (m_reply != reply) {
        m_reply = reply;
        emit replyChanged(m_reply);
    }
}

I realize that the portNbr change is initiated from the UI thread, while the reply change is initiated in the worker thread. Is this what's causing the problem?

jeremy_k

This conversation is getting bogged down in details. If the OP understands QObject thread affinity and auto or queued connection semantics, it should be easy to create an arbitrary interface. If not, the same situation is likely to occur for the next minor variation.

Fortunately this is explained in the documentation.
https://doc.qt.io/qt-6/qobject.html#thread-affinity

There are a few points in the documentation that are critical to this discussion:

setParent() will fail if the two QObjects involved live in different threads.

When a QObject is moved to another thread, all its children will be automatically moved too.

moveToThread() will fail if the QObject has a parent.

And, I believe this to be a fundamental part of the misunderstanding:

Note: A QObject's member variables do not automatically become its children. The parent-child relationship must be set by either passing a pointer to the child's constructor, or by calling setParent(). Without this step, the object's member variables will remain in the old thread when moveToThread() is called.

This means that creating a member object in the constructor or any other function is irrelevant. If the member is explicitly set as a child, it will follow the parent's thread affinity. Otherwise, the child will associate with whatever thread ran its constructor.

As an aside, the OP appears to be creating a thread to manage a QNetworkAccessManager instance. All of the member functions that return a QNetworkReply, as well as connectToHost*() are asynchronous. Unless the reply processing overhead is significant, it's usually not necessary to create another thread.

The QNetworkAccessManager doesn't appear in subsequent snippets.

mzimmers

@jeremy_k thank you for the detailed reply.

Regarding the need for a separate thread in this case, you're absolutely right that it's probably unnecessary. I was just trying to create a working example that I could transfer to my "real" application.

It appears that the takeaway from this discussion is that anything the QML will access (via Q_PROPERTY, etc.) should remain in the thread that is performing the Qt Quick activities. Right?

This would include the models that I create in C++. My application will have several models. A viable approach might be to put the network logic in a worker thread, and have that thread communicate to the UI thread via signals. The UI thread would then be responsible for parsing the messages and updating the model. I was hoping to perform this outside of the UI thread, but it appears that this just isn't viable.

If all this sounds about right, I'll close this topic. Of course, feel free to make any corrections or additions.

jeremy_k

@mzimmers said in QML signal to C++ slot in worker thread:

@jeremy_k thank you for the detailed reply.

Regarding the need for a separate thread in this case, you're absolutely right that it's probably unnecessary. I was just trying to create a working example that I could transfer to my "real" application.

I guess QNAM was an unfortunately complicated substitute, as it creates QObject children parented to itself. Or perhaps its complication was a benefit for the discussion.

It appears that the takeaway from this discussion is that anything the QML will access (via Q_PROPERTY, etc.) should remain in the thread that is performing the Qt Quick activities. Right?

Yes. Most functions in QObject are not thread safe. Check per class used, but by default everything should be presumed to not be.

This would include the models that I create in C++. My application will have several models. A viable approach might be to put the network logic in a worker thread, and have that thread communicate to the UI thread via signals. The UI thread would then be responsible for parsing the messages and updating the model. I was hoping to perform this outside of the UI thread, but it appears that this just isn't viable.

Reaffirming the answer in the previous block, presume QAbstractItemModel member functions are not thread safe. Work can happen in another thread outside of what these functions can view, or via explicit locking. See https://doc.qt.io/qt-6/qtquick-threading-example.html for a threaded ListModel example.

Revisiting the aside, an extra thread may be as unnecessary as with QNAM. Socket notifiers or timers and non-blocking polling can handle waiting for input. Timers and work queues can partition total workloads exceeding a reasonable execution period. I try to limit thread use to blocking system or library calls, situations where a unit of the workload can't be completed with reasonably latency, and cases where parallelism is expected to reduce the wall clock computation time.

Multiprocessing has a computation, memory bandwidth, and mental complexity overhead in most uses. To paraphrase some programmer wisdom of lore: Some people, when confronted with a problem, think "I know, I'll use threads." Now they have two problems.

mzimmers

@jeremy_k said in QML signal to C++ slot in worker thread:

Reaffirming the answer in the previous block, presume QAbstractItemModel member functions are not thread safe.

Understood. The idea would be to have a thread that contains the network manager, and handles the IO. My various models would all reside in the main thread, and message requests (and replies) would be conveyed through signals and slots. That should eliminate any thread-based issues regarding the model updates and attendant updates to the QML portion of the application.

I appreciate the comment about threads often causing more problems than they solve, and may well abandon the network thread, but I think it should work in this case.