Why Qt UI Application goes to 'Not Responding' state while the job has already moved to a worker thread?

BAHRAMUDIN ADIL

I think there is no need for code to post because I have already mentioned above how did I do it, but I will post the code:

#include "mainwindow.h"
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);
    ui->progressBar->setVisible(false);
    connect(this, &MainWindow::setLabelText, ui->lbResult, &QLabel::setText);
    connect(this, &MainWindow::setText, ui->btGen, &QPushButton::setText);
    connect(this, &MainWindow::setValue, ui->progressBar, &QProgressBar::setValue);
    connect(this, &MainWindow::setVisiblePg, ui->progressBar, &QProgressBar::setVisible);

    connect(ui->btTest, &QPushButton::clicked, this, [this]() {
        if(data.size() == 0) {
            return;
        }
        if(ui->tfTestNumber->text().isEmpty()) {
            return;
        }
        setVisiblePg(true);
        ui->progressBar->setMaximum(data.size());
        QThread *thread = QThread::create([this]() {
            int p1 = 0;
            int p2 = 0;
            int p3 = 0;
            int p4 = 0;
            int p5 = 0;
            int p6 = 0;
            int p7 = 0;
            int p8 = 0;
            int p9 = 0;

            QString txt = ui->tfTestNumber->text();
            for (int i(0); i<data.size(); ++i) {
                // ....... loop through data
                emit setValue(i); // refresh progressBar
            }

            QString rst;
            rst.append("p1：").append(QString::number(p1)).append("\n");
            rst.append("p2：").append(QString::number(p2)).append("\n");
            rst.append("p3：").append(QString::number(p3)).append("\n");
            rst.append("p4：").append(QString::number(p4)).append("\n");
            rst.append("p5：").append(QString::number(p5)).append("\n");
            rst.append("p6：").append(QString::number(p6)).append("\n");
            rst.append("p7：").append(QString::number(p7)).append("\n");
            rst.append("p8：").append(QString::number(p8)).append("\n");
            rst.append("p9：").append(QString::number(p9)).append("\n");
            emit setLabelText(rst);
        });
        thread->connect(thread, &QThread::finished, thread, &QThread::deleteLater);
        thread->start();
    });

    connect(ui->btGen, &QPushButton::clicked, this, [this]() {
        if(started) {
           started = false;
        }
        int total = ui->tfTotal->text().toInt();
        if(total == 0) {
            return;
        }
        data.reserve(total);
        setVisiblePg(true);
        ui->progressBar->setMaximum(total);

        QThread *thread = QThread::create([this, total]() {
            for (int i(0); i<total; i++) {
                if(!started) {
                    break;
                }
                // loop and generate the data and push it to QStringList
                data.push_back(result);
		emit setValue(i); // refresh progressBar
            }
            setVisiblePg(false);
        });
        thread->connect(thread, &QThread::finished, thread, &QThread::deleteLater);
        thread->start();
        started = true;
    });
}

And I have also tried to make a new class and do everything there something like this:

// header
class GeneratorThread : public QObject
{
    Q_OBJECT
public:
    GeneratorThread();
    ~GeneratorThread();

    int getTotal() const;
    void setTotal(int value);

    bool getStarted() const;

    void setStarted(bool value);

    QStringList *getData() const;

signals:
    void setValue(int);
    void setVisiblePg(bool);
    void setText(const QString&);
    void setLabelText(const QString&);

public slots:
    void start();

private:
    QThread thread;
    int total;
    QStringList *data;
    bool started = false;
};

// source
GeneratorThread::GeneratorThread() : QObject()
{
    data = new QStringList;
    moveToThread(&thread);
    thread.start();
}

GeneratorThread::~GeneratorThread()
{
    thread.quit();
    thread.wait();
}

void GeneratorThread::start()
{
    started = true;
    for (int i(0); i<total; i++) {
        if(!started) {
            break;
        }
        data->push_back(result);
        emit setValue(i);
        qDebug() << i;
    }
    setVisiblePg(false);
}

QStringList *GeneratorThread::getData() const
{
    return data;
}

void GeneratorThread::setStarted(bool value)
{
    started = value;
}

bool GeneratorThread::getStarted() const
{
    return started;
}

int GeneratorThread::getTotal() const
{
    return data->size();
}

void GeneratorThread::setTotal(int value)
{
    total = value;
    data->clear();
    data->reserve(value);
}

But it was the same.

jsulm

@BAHRAMUDIN-ADIL What does setVisiblePg(true) do?
You are accessing UI from your thread: ui->progressBar->setMaximum(total);!
It is not allowed/supported to access UI classes from other threads than UI thread!

BAHRAMUDIN ADIL

Till now I only one found one sulotion: I the worker thread, you must sleep:

QThread::usleep(1)

J.Hilk

@BAHRAMUDIN-ADIL besides what @jsulm said,
you're wantonly negligent with other aspects of a thread as well.

For example you access datawithout a mutex! Data can be changed at any time or it might crash when both threads try to access the container.
if you're using cpp17 pass your create arguments!

QThread *thread = QThread::create(myFunction, arg1, arg2);

Or use QtConCurrent the static function call on run allows arguments as well
also I see no reentrant checks. Multiple threaded functions could do exactly the same on the same data without mutex locks.

Depending on how fast your "calculation" is and how big the data.size() you're also flooding the main thread with signals, that can easily overwhelm it.

BAHRAMUDIN ADIL

@jsulm I am not accessing the UI from the thread, if you look carefully you will see that everything passing from the worker thread are using signals and slot, otherwise, the application will not start.

setVisiblePg(true)

this a signal which set the progress bar visible or hides it when accessing from the inside worker thread.

ui->progressBar->setMaximum(total);

This not from other thread. Look Carefully please!

J.Hilk

@BAHRAMUDIN-ADIL said in Why Qt UI Application goes to 'Not Responding' state while the job has already moved to a worker thread?:

@jsulm I am not accessing the UI from the thread, if you look carefully you will see that everything passing from the worker thread are using signals and slot,

that's a lie

QThread *thread = QThread::create([this]() {
            int p1 = 0;
            int p2 = 0;
            int p3 = 0;
            int p4 = 0;
            int p5 = 0;
            int p6 = 0;
            int p7 = 0;
            int p8 = 0;
            int p9 = 0;

            QString txt = ui->tfTestNumber->text(); //<<<<----------------
            for (int i(0); i<data.size(); ++i) {
                // ....... loop through data
                emit setValue(i); // refresh progressBar
            }

BAHRAMUDIN ADIL

@J.Hilk In the first, I don't need mutex because only one thread can access the data at the same time, my data is very big maybe 200,000,000.

But I don't know this: you're also flooding the main thread with signals, that can easily overwhelm it

If I don't use signal and slots, then what should I use to communicate between threads?
Thanks!

jsulm

@BAHRAMUDIN-ADIL You're emitting 10.000.000 signals, right? See "I am just generating 10,000,000 records of data".
If so, then you should reduce the amount of UI updates (your progress dialog). There is really no need to do it for each record.

J.Hilk

@BAHRAMUDIN-ADIL signal&slots is the way you should go.

Simply reduce the number of emits, via a modulo check for example, or a QElapsedTimer and only emit every 10 ms, etc.

BAHRAMUDIN ADIL

@J.Hilk You mean this:

emit setValue(i); // refresh progressBar

??
It is a signal not directly set the value of the progressbar.
If I directly set it will throw an exception.

jsulm

@BAHRAMUDIN-ADIL This is where you're accessing UI class:

QString txt = ui->tfTestNumber->text();

BAHRAMUDIN ADIL

@jsulm this is just getting the text, not setting text, and also it will be called the only once.

jsulm

@BAHRAMUDIN-ADIL You still should avoid it. It may work or not.
And as already suggested: reduce the amount of signals you're sending for UI updates.

BAHRAMUDIN ADIL

@jsulm Yes it will make sense, I will do this and see what happens.

BAHRAMUDIN ADIL

@jsulm Thanks! The problem is just with emitig the signals very quickly.