Finishing a thread in QThread

[Tio]

Hello over there.
I have a couple of threads running on my program. So, i need finish one of them when a click a button.
I've tried use quit() and exit(), but didn't work. For now, i am setting true to a bool in class Thread that reaches the end of code, but i need another method.
Thanks all
Here is code for now

#include "thread.h"
 
 
Thread::Thread(QObject *parent, QString strCaminhoOff, QString strCaminhoPP, int ind) : QThread(parent)
{
    this->strCaminhoOff = strCaminhoOff;
    this->strCaminhoPP = strCaminhoPP;
    stop = false;
    this->ind = ind;
}
 
void Thread::run()
{
        qDebug() << "Executing a new independant thread, GUI is NOT blocked" << ind;
        QTime tempo;
        tempo.start();
        int i;
        for(i=0; i<1000000000; i++){
            if(stop == false){
                i++; i--;
            }else break;
        }
 
        qDebug() << "\n" << i;
 
        emit nova(strCaminhoOff, tempo.elapsed(), ind);
 
        //exec();
}

#include "thread.h"


Thread::Thread(QObject *parent, QString strCaminhoOff, QString strCaminhoPP, int ind) : QThread(parent)
{
    this->strCaminhoOff = strCaminhoOff;
    this->strCaminhoPP = strCaminhoPP;
    stop = false;
    this->ind = ind;
}

void Thread::run()
{
        qDebug() << "Executing a new independant thread, GUI is NOT blocked" << ind;
        QTime tempo;
        tempo.start();
        int i;
        for(i=0; i<1000000000; i++){
            if(stop == false){
                i++; i--;
            }else break;
        }

        qDebug() << "\n" << i;

        emit nova(strCaminhoOff, tempo.elapsed(), ind);

        //exec();
}

To copy to clipboard, switch view to plain text mode 

[tbscope]

If you want to use quit(), you need to have an eventloop in your thread.
Otherwise, if you really really want to stop a thread no matter what: terminate()

[Tio]

yeah, i know, i've tried use the event loop with exec() and calling quit() or exit() when push the button, but didn't work.
I've tried terminate() too, but it's very unstable, sometimes crash the program.
I am doing anything wrong friend?
thanks

[SixDegrees]

Note, too, that calling terminate() is frowned upon. The problem is that it kills the thread immediately - there is no chance for the thread to delete locally allocated memory, close file handles or do any other cleanup that might be necessary. This may or may not be a problem, but it needs careful attention before you go down that road.

Why is the way you're doing it now - by checking the value of an external variable - bad? This is the approach most well-behaved threads use for premature termination.

[tbscope]

You start the eventloop when the thread is already finished.
What happens if you put it at the top of your run() function?

[Tio]

Tbscope
If i put it at the top, the threads stay on exec(), so do nothing

SixDegrees
Cause my thread will not realize this code, it's just to make something for now. Actually, the thread will call another function of the project, when ready. So, if i choose this way, i will need put this "bool stop" in that class and access from there. So, if there's a way to make the thread stop without the variable, it's better for me. Otherwise, i will keep this idea

[Coises]

the thread will call another function of the project, when ready. So, if i choose this way, i will need put this "bool stop" in that class and access from there. So, if there's a way to make the thread stop without the variable, it's better for me.

The thing is, it’s almost always a bad idea to force another thread to stop dead in its tracks... even more so if you don’t design and control the code. As C++ doesn’t provide a clean-up/recovery mechanism for asynchronous exceptions — if anything, it covers its ears and says, “Nan-nah-nah, I can’t hear you!” — it’s both difficult and inadvisable to try to do this if the thread to be halted is coded in C++.

Unless there is a really compelling reason to do otherwise, you should always think ask the other thread to stop, not force the other thread to stop.

If i put it at the top, the threads stay on exec(), so do nothing

If you use the event loop approach, then you have to program the thread as a sequence of events. This actually works quite well, as it allows you to use signals and slots, and QThread::exit(), to communicate; but it requires a bit of re-thinking about how the thread is programmed. You have to program it almost like a GUI thread, in small steps that are initiated in response to messages and don't block for long, except that each step finishes by queuing a message that will start the next step. You prime the event loop by queuing a message that starts the sequence of events.

[Tio]

Coises
Can you gimme a little example using exec() and quit()?

[Coises]

Can you gimme a little example using exec() and quit()?

background.h:

#include <QList>
#include <QThread>
 
class Background : public QThread {
    Q_OBJECT;
    QList<int> prime;
 public:
    Background();
    void run();
 signals:
    void gotNextPrime(QString);
    void proceed();
 private slots:
    void getNextPrime();
};

#include <QList>
#include <QThread>

class Background : public QThread {
    Q_OBJECT;
    QList<int> prime;
 public:
    Background();
    void run();
 signals:
    void gotNextPrime(QString);
    void proceed();
 private slots:
    void getNextPrime();
};

To copy to clipboard, switch view to plain text mode 

background.cpp:

#include "background.h"
 
Background::Background() {
    moveToThread(this);
    connect(this, SIGNAL(proceed()), SLOT(getNextPrime()), Qt::QueuedConnection);
    prime += 2;
}
 
void Background::run() {
    emit proceed();
    exec();
}
 
void Background::getNextPrime() {
    for (int p = prime.last() + 1;; ++p) for (int i = 0;; ++i) {
        int j = prime.at(i);
        int k = p / j;
        if (p == j * k) break;
        if (j < k) continue;
        prime += p;
        emit gotNextPrime(QString("Prime %1 is %2.").arg(prime.count()).arg(p));
        emit proceed();
        return;
    }
}

#include "background.h"

Background::Background() {
    moveToThread(this);
    connect(this, SIGNAL(proceed()), SLOT(getNextPrime()), Qt::QueuedConnection);
    prime += 2;
}

void Background::run() {
    emit proceed();
    exec();
}

void Background::getNextPrime() {
    for (int p = prime.last() + 1;; ++p) for (int i = 0;; ++i) {
        int j = prime.at(i);
        int k = p / j;
        if (p == j * k) break;
        if (j < k) continue;
        prime += p;
        emit gotNextPrime(QString("Prime %1 is %2.").arg(prime.count()).arg(p));
        emit proceed();
        return;
    }
}

To copy to clipboard, switch view to plain text mode 

mainwindow.h:

#include <QGridLayout>
#include <QLabel>
#include <QMainWindow>
#include <QPushButton>
#include <QWidget>
#include "background.h"
 
class MainWindow : public QMainWindow {
    Q_OBJECT
public:
    MainWindow(QWidget *parent = 0);
private:
    QGridLayout *grid;
    QWidget     *centralWidget;
    QLabel      *displayLabel;
    QPushButton *stopButton;
    QPushButton *startButton;
    Background  *bgnd;
};

#include <QGridLayout>
#include <QLabel>
#include <QMainWindow>
#include <QPushButton>
#include <QWidget>
#include "background.h"

class MainWindow : public QMainWindow {
    Q_OBJECT
public:
    MainWindow(QWidget *parent = 0);
private:
    QGridLayout *grid;
    QWidget     *centralWidget;
    QLabel      *displayLabel;
    QPushButton *stopButton;
    QPushButton *startButton;
    Background  *bgnd;
};

To copy to clipboard, switch view to plain text mode 

mainwindow.cpp:

#include "mainwindow.h"
 
MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent) {
    centralWidget = new QWidget(this);
    displayLabel = new QLabel(centralWidget);
    displayLabel->setAlignment(Qt::AlignCenter);
    displayLabel->setText("Click Start to begin.");
    startButton = new QPushButton(centralWidget);
    startButton->setText("Start");
    stopButton = new QPushButton(centralWidget);
    stopButton->setText("Stop");
    grid = new QGridLayout(centralWidget);
    grid->addWidget(displayLabel, 0, 0, 1, 2);
    grid->addWidget(startButton, 1, 0);
    grid->addWidget(stopButton, 1, 1);
    setCentralWidget(centralWidget);
    bgnd = new Background();
    connect (startButton, SIGNAL(clicked()), bgnd, SLOT(start()), Qt::DirectConnection);
    connect (stopButton, SIGNAL(clicked()), bgnd, SLOT(quit()));
    connect (bgnd, SIGNAL(gotNextPrime(QString)), displayLabel, SLOT(setText(QString)));
}

#include "mainwindow.h"

MainWindow::MainWindow(QWidget *parent) : QMainWindow(parent) {
    centralWidget = new QWidget(this);
    displayLabel = new QLabel(centralWidget);
    displayLabel->setAlignment(Qt::AlignCenter);
    displayLabel->setText("Click Start to begin.");
    startButton = new QPushButton(centralWidget);
    startButton->setText("Start");
    stopButton = new QPushButton(centralWidget);
    stopButton->setText("Stop");
    grid = new QGridLayout(centralWidget);
    grid->addWidget(displayLabel, 0, 0, 1, 2);
    grid->addWidget(startButton, 1, 0);
    grid->addWidget(stopButton, 1, 1);
    setCentralWidget(centralWidget);
    bgnd = new Background();
    connect (startButton, SIGNAL(clicked()), bgnd, SLOT(start()), Qt::DirectConnection);
    connect (stopButton, SIGNAL(clicked()), bgnd, SLOT(quit()));
    connect (bgnd, SIGNAL(gotNextPrime(QString)), displayLabel, SLOT(setText(QString)));
}

To copy to clipboard, switch view to plain text mode 

main.cpp:

#include <QtGui/QApplication>
#include "mainwindow.h"
 
int main(int argc, char *argv[]) {
    QApplication a(argc, argv);
    MainWindow w;
    w.show();
    return a.exec();
}

#include <QtGui/QApplication>
#include "mainwindow.h"

int main(int argc, char *argv[]) {
    QApplication a(argc, argv);
    MainWindow w;
    w.show();
    return a.exec();
}

To copy to clipboard, switch view to plain text mode 

Note that the connect in the Background constructor must specify Qt::QueuedConnection — otherwise, since the signal and slot are on the same thread, the emit proceed(); would become effectively a recursive call to getNextPrime(); we need it to go through the queue so a quit() can interrupt the sequence.

The connect for StartButton has to specify Qt::DirectConnection because until bgnd is started, there is no event loop to process a queued connection.

[Tio]

Thank you very much Coises, i will see this code and try it
when i reach a conclusion, i will post here =D
And thanks to all people that help me until here ^^