start and programmatically stop a thread

gbettega

Hi,
a very simple question for this forum.

A "long" operation is embedded in a thread, handled with the QThread strategy.
Two buttons in a widget: START, and STOP.
The thread starts when START is pressed, through the signal operate().
Now I would like to stop the thread, when STOP is pressed.

Consider that, in general, the "long" operation is not a for cycle written by me
as in this case, but a call to an external dll, which does its works: so I cannot
access any for through a flag controlling the cycle execution, and I cannot re-write
the ,dll code

Obviously the current solution for stopping does not work, since stop is executed at the end of the QThread main loop.

Read some tents of posts on this topic, new and old, and obviously the documentation, but for a lack of understanding from my side had no success.

Grazie mille
Giovanni

testThreads.pro

QT += gui core \
    widgets

CONFIG += c++11
TEMPLATE = app


# The following define makes your compiler emit warnings if you use
# any Qt feature that has been marked deprecated (the exact warnings
# depend on your compiler). Please consult the documentation of the
# deprecated API in order to know how to port your code away from it.
DEFINES += QT_DEPRECATED_WARNINGS
e
# You can also make your code fail to compile if it uses deprecated APIs.
# In order to do so, uncomment the following line.
# You can also select to disable deprecated APIs only up to a certain version of Qt.
#DEFINES += QT_DISABLE_DEPRECATED_BEFORE=0x060000    # disables all the APIs deprecated before Qt 6.0.0

SOURCES += \
        main.cpp \
        widget.cpp

# Default rules for deployment.
qnx: target.path = /tmp/$${TARGET}/bin
else: unix:!android: target.path = /opt/$${TARGET}/bin
!isEmpty(target.path): INSTALLS += target

HEADERS += \
    testWorker.h \
    threadController.h \
    threadWorker.h \
    widget.h

main.cpp

#include <QApplication>
#include <QTimer>

#include "widget.h"

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

    widget w;
    w.show();

    return a.exec();
}

widget.h

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QWidget>

class widget : public QWidget
{
    Q_OBJECT

public:

    widget(QWidget* parent = nullptr);
};

#endif // MAINWINDOW_H

widget.cpp


#include <QPushButton>
#include <QPlainTextEdit>
#include <QHBoxLayout>
#include <QVBoxLayout>

#include "widget.h"

#include "threadController.h"
#include "threadWorker.h"
#include "testWorker.h"
#include "widget.h"

widget::widget(QWidget* parent) : QWidget(parent)
{
    testWorker* customWorker = new testWorker(10000);
    threadController* tc = new threadController(customWorker,this);

    QVBoxLayout* vl = new QVBoxLayout();
    this->setLayout(vl);

    QPlainTextEdit* te = new QPlainTextEdit(this);
    te->setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOn);
    te->ensureCursorVisible();
    vl->addWidget(te);

    QHBoxLayout* hl =  new QHBoxLayout();
    vl->addLayout(hl);

    QPushButton* bs =  new QPushButton("START",this);
    QPushButton* bss = new QPushButton("STOP",this);
    hl->addWidget(bs);
    hl->addWidget(bss);

    QObject::connect(bs,  &QPushButton::pressed, this, [=]() { tc->operate();    });
    QObject::connect(bss, &QPushButton::pressed, this, [=]() { tc->stop(); });
    QObject::connect(customWorker,&testWorker::newText,this,[=](QString text) { te->insertPlainText(text);} );
}

threadWorker.h

#ifndef THREADWORKER_H
#define THREADWORKER_H

#include <QObject>

class threadWorker : public QObject
{
    Q_OBJECT

public:

    threadWorker() {}

public slots:

    virtual void doWork(){}

signals:

    void resultReady();
};

#endif // THREADWORKER_H

testWorker.h

#ifndef TESTWORKER_H
#define TESTWORKER_H

//! ----------------
//! custom includes
//! ----------------
#include "threadWorker.h"

//! ----
//! C++
//! ----
#include <iostream>
#include <string>

//! ---
//! Qt
//! ---
#include <QString>
#include <QPlainTextEdit>

class testWorker : public threadWorker
{
    Q_OBJECT

private:

    size_t m_cycles;
    void* m_p;

public:

    testWorker(size_t cycles, void* p = nullptr) : m_cycles(cycles), m_p(p) {}

    void doWork() override
    {
        size_t S = 0;
        for(size_t i = 0; i <= m_cycles; i++)
        {
            S += i;
            std::string s = "* calcolo somma parziale: " + std::to_string(i) + "\n";
            emit newText(QString::fromStdString(s));
        }

        emit resultReady();
    }

signals:

    void newText(QString);
};

#endif // TESTWORKER_H

threadController.h

#ifndef THREADCONTROLLER_H
#define THREADCONTROLLER_H

//! ---
//! Qt
//! ---
#include <QObject>
#include <QThread>

//! ----------------
//! custom includes
//! ----------------
#include "testWorker.h"

class threadController : public QObject
{
    Q_OBJECT

private:

    QThread workerThread;

public:

    threadController(threadWorker* worker, QObject* parent = nullptr): QObject(parent)
    {
        worker->moveToThread(&workerThread);
        QObject::connect(&workerThread, &QThread::finished, worker, &QObject::deleteLater);
        QObject::connect(this, &threadController::operate, worker, &threadWorker::doWork);
        QObject::connect(this, &threadController::stop, this, [=]()
        {
            workerThread.quit();
            workerThread.wait();
        });

        QObject::connect(worker, &threadWorker::resultReady, this, &threadController::handleResults);
        workerThread.start();
    }

    ~threadController()
    {
        workerThread.quit();
        workerThread.wait();
    }

public slots:

    virtual void handleResults() {}

signals:

    void operate();
    void stop();
};

#endif // THREADCONTROLLER_H

gbettega

this is not a camping, but, unfortunately I cannot change tents in tens, since the post content has been flagged as spam for unknowkn reasons

gbettega

sorry, remove "this" from lambda functions

JonB

@gbettega said in start and programmatically stop a thread:

Now I would like to stop the thread, when STOP is pressed.

You need to set, check and act on bool QThread::isInterruptionRequested() const, or emit a signal on which the thread has placed a slot and code in that causes the thread to some to an end. Either way you need the thread's event loop to be running which won't happen if you do a long, busy loop in the thread, so you need it to do its work in "chunks". [Actually I am not certain that the isInterruptionRequested() requires the event loop to be entered (which the signal/slot does), you may be able to test it intermittently without. But it still means that the worker thread needs to test this regularly during its work.]

Consider that, in general, the "long" operation is not a for cycle written by me
as in this case, but a call to an external dll, which does its works: so I cannot
access any for through a flag controlling the cycle execution, and I cannot re-write
the ,dll code

Then you are utterly stymied! If you cannot change the DLL code then, obviously, it cannot enter the thread's event loop for a signal nor check for isInterruptRequested(). If the DLL itself does not offer some call to interrupt and terminate it then it is not intended to be interruptible! You might leave it running and ignore it. You might "kill" it forcefully, but almost certainly that will leave things in a bad state.

gbettega

ok thank you very much.
Terminate(); the function within the thread does not used shared resources, and does not read/write to/from disk
Have a nice day
G

JonB

@gbettega
That is what I meant by

You might "kill" it forcefully, but almost certainly that will leave things in a bad state.

As per docs:

Warning: This function is dangerous and its use is discouraged. The thread can be terminated at any point in its code path. Threads can be terminated while modifying data. There is no chance for the thread to clean up after itself, unlock any held mutexes, etc. In short, use this function only if absolutely necessary.

Use at your own risk!

gbettega

@JonB Thank you!
What does exactly "bad state" means?
Grazie mille
Giovanni

Pl45m4

@gbettega said in start and programmatically stop a thread:

What does exactly "bad state" means?

UB (undefined behavior), for example...
If you stop/terminate the thread while processing it might leave garbage values behind, which then cause errors elsewhere.
You can never be sure that everything is working as it should after "killing" the thread forcefully.
terminate() IS a solution, but not a good one.

All depends on what your thread/your app is doing.

@gbettega said in start and programmatically stop a thread:

the thread does not used shared resources, and does not read/write to/from disk

It could potentially work for you... but could also result in errors later.

JonB

@gbettega
All as @Pl45m4 has written. It means "It works OK, if you're lucky, till some time it does not, if you are unlucky".

Depending on what happens to happen when, you can kill it lots of times with no ill effects, then perchance one time it gets killed at a different state and that does have an effect. It does help mitigation that your thread at least "does not access anything else", but there are other things which can be affected.

What is it that this DLL code does which takes so long to compute and does not allow for interruption/termination? Would you consider, maybe, that when user asks for "stop" you allow the thread to run to normal termination but disconnect/ignore any signals or output it sends you? That might be safer than killing.

gbettega

@JonB thank you for your reply.
The code sent to the thread performs a geometry/topology operation performed on a BREP file. E.g. a defeatuting operation done on a complex shape geometry. It is one among the typical operations a geometry kernel of a CAD software performs onto n input geometry. If the required simplification is "simple" the dll takes a fraction of second, or less, but, if the defeaturing operation is made of multiple simplifications onto a complex shape, or multiple shapes, the kernel (here OpenCascade) could take serveral minutes, and in the meantime the GUI should remain unlocked. There is also the possibility that the code inside the library gets stuck: the user cannot distinguish between a stuck state (infinite loop) or a very long operation, so he generally "quits". Ignoring all it is happening in this case is not feasible, since these operations when running are intensive, both in terms of memory, both in terms of CPU.
Have a nice day
GIovanni

JonB

@gbettega
I understand. Then you seem to have little choice other than to "kill/terminate" the thread. Hopefully in practice it will go OK.

jeremy_k

A safer option is to run the calculation in a separate process. Termination there is easier to understand for a seasoned programmer.

JonB

@jeremy_k
I had thought about suggesting that to the OP. But they will then need to write some IPC code to get the information back to the calling process which may be a bit of work, depending on the quantity and format of the results.

jeremy_k

Stdin and stdout of the child process work for low volumes. Shared memory for large data sets. It's unusual to want a multithreaded algorithm and not understand some form of IPC.

The risk of having a memory allocation abandoned, or a library dependency leaving a mutex locked dissuades me from considering QThread::terminate() in any circumstance.

J.Hilk

Like previously stated, the only real option you have is to start a separate process from your Qt application. This process loads the DLL and performs the calculations.

You can then terminate that process at any time, and the operating system will take care of cleaning up memory, handles, and other resources. The only potential downside is the risk of corrupted files, if the DLL performs file operations.

Communication between your main application and the second process can be handled via QSharedMemory, QLocalSocket, or standard input/output piping.