closing QtConcurrent::run created thread

randdengineer

Hello,

I create a thread with QtConcurrent::run method. Here are the codes:
main.cpp:

void StateMachineThread(MainWindow *w)
{
    QThread::currentThread()->setObjectName("state machine thread");
    QScopedPointer<stateMachine> states(new stateMachine);
    states->run(w);
}

int main(int argc, char *argv[])
{
    int returnVal;

    QApplication a(argc, argv);
    QThread::currentThread()->setObjectName("main thread");
    MainWindow w;
    w.show();
    QFuture<void> threadStateMachine = QtConcurrent::run(StateMachineThread, &w);
    return a.exec();
}

In mainwindow.cpp i defined the destructor as this:

MainWindow::~MainWindow()
{
    qInfo() << "mainWindow destructed in" << QThread::currentThread();
    emit this->finished();
    delete ui;
}

Here is the run method of stateMachine class:

void stateMachine::run(MainWindow *w)
{
    QScopedPointer<QEventLoop> loop(new QEventLoop);
    connect(w, &MainWindow::finished, loop.data(), &QEventLoop::quit);
    qInfo() << "state machine starting in" << QThread::currentThread();
    loop->exec();
}

The threads are closed and the objects are deleted in the order I intended. So far I haven't run into any issues but I'm not sure if I've done something wrong that could cause problems in the future. Do you have any comments about this?

Chris Kawa

Hi, welcome to the forum.

Do you have any comments about this?

You don't need to allocate everything dynamically:

QScopedPointer<stateMachine> states(new stateMachine);
states->run(w);

You can just do

stateMachine states;
states.run(w);

or even

stateMachine().run(w);

Same with

QScopedPointer<QEventLoop> loop(new QEventLoop);

where you can just do

QEventLoop loop;

Dynamic allocations are heavy. Stack's your friend.

As for the system design itself it's kinda pointless.
First - it's not nice that a state machine class has knowledge about main window. Those classes don't have anything in common, so shouldn't know about each other. The connection between them should be made one level up, where you have context and access to both objects.
Second - all this work basically recreates this simple code:

MainWindow w;
w.show();

QThread stateMachineThread;
stateMachineThread.start();

QObject::connect(&w, &MainWindow::finished, &stateMachineThread, &QThread::quit);

What's the point of using QtConcurrent, all that setup and manually running an event loop when QThread does that for you out of the box?

randdengineer

Hi @Chris-Kawa
I am new to Qt and there is no specific point in using QtConcurrent. I was looking for a thread method and i choosed it after watching the video in the link:
https://www.youtube.com/watch?v=QcL8Sob5shk
Thank you for the recommendation. I will return to QThread. It seems more efficient as you said.