CODEWITHSUNDEEP
c++qtsignals-slotsqthread
johnco3
johnco3

Reputation: 2642

QT slot not getting called on main thread

From the thread context of a slot in my QT GUI application (upon button push), I am trying to launch a worker thread to update other another part of the GUI with the results of a CPU intensive calculation - these results will update a table or a google like map widget - so this needs to occur in the main QT application thread where it is safe to update these widgets.

The problem I have is that the updateGUIWidget slot never gets called unless I change the connection type to Qt::DirectConnection - in which case it gets called in the worker thread (where it is unsafe to update the GUI). I checked the results of each of the connect calls and they are fine, it seems that there is some issue with the event loop somewhere. I'm not sure if I need to allocate the thread and the worker objects as members of the mainwindow or if its OK to do so from stack variables in the slot.

void
mainwindow::on_importSimulatedFlight_clicked()
{
    // experimental worker thread model adapted from YouTube tutorial
    // by Giuseppe di Angelo https://www.youtube.com/watch?v=BgqT6SIeRn4
    // auto thread = new QThread;
    // note worker created in gui thread here - we will move its thread
    // affinity to 'thread' below before starting it.
    auto thread = new QThread;
    auto worker = new Worker;
    connect(thread, &QThread::started, worker, &Worker::doWork);
    // connect(worker, &Worker::progressUpdate, this, &mainwindow::updateGUIWidget, Qt::DirectConnection);
    connect(worker, &Worker::progressUpdate, this, &mainwindow::updateGUIWidget, Qt::QueuedConnection);
    connect(worker, &Worker::workDone, thread, &QThread::quit);
    connect(thread, &QThread::finished, worker, &Worker::deleteLater);
    // move worker to separate thread
    worker->moveToThread(thread);
    thread->start();
}

The mainwindow has a slots declared in mainwindow.h as follows:

class mainwindow : public QMainWindow
{
    Q_OBJECT
public:
    explicit mainwindow(QWidget *parent = Q_NULLPTR);
    ~mainwindow();
    ...
public slots:
    void on_importSimulatedFlight_clicked();
    void updateGUIWidget(const param& rParam);
    ...
}

and implemented in mainwindow.cpp as follows:

void
mainwindow::updateGUIWidget(const param& rParam)
{
... update widget components with rParam partial result here
}

and my worker is as follows:

class Worker : public QObject
{
    Q_OBJECT
public slots:
    void doWork() {
        const QString result;
        for (int i=0; i<5; i++) {
            const MMRTimedRecord foo;
//            QThread::sleep(1);
            emit progressUpdate(foo);
        }
        emit workDone(result);
    }
signals:
    void progressUpdate(const MMRTimedRecord&);
    void workDone(const QString &result);
};

Upvotes: 1

Views: 1302

Answers (3)

Kuba hasn&#39;t forgotten Monica
Kuba hasn&#39;t forgotten Monica

Reputation: 98425

It’s really easy – and you shouldn’t be managing any threads manually:

void Ui::slot() {
  QtConcurrent::run([this]{
    auto result = compute();
    QMetaObject::invokeMethod(this, [this, r = std::move(result)]{
      m_widget.setSomething(r);
    });
  });
}

The type of the data you compute should be movable.

Upvotes: 0

user1095108
user1095108

Reputation: 14603

You can check this answer for a possible solution. You can do 

MainThreadEvent::post([&]()
  {
    // gui update stuff
  }
);

in your slot to do the gui update in the main thread, but it is a crude approach to be sure. Despite this, I do something like this all the time. Be careful of dangling pointers and references (use QPointer)..., as the issued event is independent of the issuing object. Alternatively, use the timer approach.

Upvotes: 0

PlinyTheElder
PlinyTheElder

Reputation: 1494

The reason it isn't working is because there's a serious flaw in your code: you are trying to emit a reference to a local variable to be handled in an slot on a different thread. That is a recipe for disaster.

When you are using Qt::QueuedConnection, you MUST emit by value, like this:

void progressUpdate(MMRTimedRecord val);

That means that your MMRTimedRecord must be copyable, and correspondingly, your slot must also accept by value. And why is there a mismatch between the signal progressUpdate(const MMRTimedRecord&) and the slot updateGUIWidget(const param& rParam); ?

Upvotes: 2

Related Questions