Problem with Threading
-
Hi All,
I've been quite happy around with Qt these days. But yesterday, when I started with threading, some problems arose. The thing is, I have developed a simple application with a MainWindow, and I've added a Settings dialog for this application also. In one of the slots in settings dialog, I have a process which takes some time. So, I decided to thread it. For this, I have extended the QThread class (to MyThread) and I have my "time taking process" inside the run(). And finally inside the constructor of settings dialog, I've created an object of MyThread. The code follows.
@
//mythread.h
#ifndef MYTHREAD_H
#define MYTHREAD_H
#include <QtCore>
#include <QtGui>
class MyThread : public QThread {
private:
QToolButton* _toolButton;
public:
MyThread();
MyThread(QToolButton*);
void run();
};
#endif // MYTHREAD_H@@
//mythread.cpp
#include "mythread.h"
MyThread::MyThread() {}
MyThread::MyThread(QToolButton* toolButton) {
this->_toolButton = toolButton;
}
void MyThread::run() {
_toolButton->setDisabled(true);
for(int i = 0; i < 99999; i++)
qDebug() << "Running : " << i;
_toolButton->setDisabled(false);
}@@
//settings.h
#ifndef SETTINGS_H
#define SETTINGS_H
#include <QDialog>
#include <QtCore>
#include "mythread.h"
namespace Ui {
class Settings;
}
class Settings : public QDialog {
Q_OBJECT
public:
explicit Settings(QWidget *parent = 0);
~Settings();
private:
Ui::Settings *ui;
MyThread mThread;
private slots:
void fetchDataNow();
};
#endif // SETTINGS_H@@
//settings.cpp
#include "settings.h"
#include "ui_settings.h"
Settings::Settings(QWidget *parent) : QDialog(parent), ui(new Ui::Settings) {
ui->setupUi(this);
mThread = new MyThread(ui->fetchNowButton); //This line causes the problem
}
Settings::~Settings() {
delete ui;
}
void Settings::fetchDataNow() {
mThread.start();
qDebug() << "Ok";
}@But when I build this application, I get the following error.
@../Convertr/settings.cpp: In constructor ‘Settings::Settings(QWidget*)’:
../Convertr/settings.cpp:6:46: error: no match for ‘operator=’ in ‘((Settings*)this)->Settings::mThread = (operator new(12u), (<statement>, ((MyThread*)<anonymous>)))’
../Convertr/settings.cpp:6:46: note: candidate is:
../Convertr/mythread.h:6:7: note: MyThread& MyThread::operator=(const MyThread&)
../Convertr/mythread.h:6:7: note: no known conversion for argument 1 from ‘MyThread*’ to ‘const MyThread&’
make: *** [settings.o] Error 1@Can someone help me to fix this?
Thanks in Advance
-
I'm not sure this is related to the problem you mention, but are you aware that you must not simply manipulate a QObject from different threads?
See "This article":http://developer.qt.nokia.com/wiki/Threads_Events_QObjects
bq. It is very important to understand that QObject and all of its subclasses are not thread-safe (although they can be reentrant); therefore, you can not access a QObject from more than one thread at the same time, unless you serialize all accesses to the object’s internal data (for instance, by protecting it with a mutex).
Especially GUI object should only be manipulated from within the GUI thread.
A simple solution would be to write a slot in your settings dialog that disables the button, and connect it to the finished signal of the thread object.
-
Can you suggest code? What I want is to disable the button when the thread starts and re-enable it when the thread finishes. But is it possible to pass a QThread object as a parameter in connect()? As such QObject::connect(const QObject*, const char*, const QObject*, const char*, Qt::ConnectionType) will be expecting QObjects right?
Thanks
-
I do believe you would need to communicate between the two threads using the signals and slots methods. Heres some documentation on how to do so.
http://developer.qt.nokia.com/doc/qt-4.8/signalsandslots.html
You would instantiate the thread in your main code, create a signal in the header, and use the connect statement to link the two objects. Your slot would be your function that would change the state of the button.
Technically, creating a button in a thread is fine if your only going to be manipulating the button within that thread and ONLY that thread, unless you use signals and slots. Typically, two different threads / events trying to use the same button at the same time WILL cause an exception/crash.
So for your case instead of disabling and re-enabling the button from the button from the form, you would emit signals which is connected to the mainwindow button and using a function (slot) that disables and enables the button. You can also pass parameters between the two so only one signal needs to be created.
Goodluck!
-
[quote author="Napster" date="1325770837"]
@
//mythread.h
#ifndef MYTHREAD_H
#define MYTHREAD_H
#include <QtCore>
#include <QtGui>
class MyThread : public QThread {
private:
QToolButton* _toolButton;
public:
MyThread();
MyThread(QToolButton*);
void run();
};
#endif // MYTHREAD_H
@Thanks in Advance
[/quote]
You must not use GUI classes outside the main thread. Never ever.
Use signals and slots to communicate between your worker thread and the main/gui thread.
And do read the aforementioned "wiki article":/wiki/Threads_Events_QObjects.
-
Yes, it is perfectly fine to pass a QThread-object as a parameter of connect. Otherwise, the signals defined in QThread would make no sense.
As soon as signal and slot are in different threads, the signal is received by the receiving thread's event queue, and emitted from there. That means that the receiving thread must have an event queue for signals and slots to work.In your case, the receiving thread is the GUI thread, which has an event queue anyway, so it should work out of the box.
-
just disable the button right before you start your thread and connect the signal (which you need to add) indicating that your worker thread is done to a slot that will reenable the button.
Something like this.
@
void Settings::fetchDataNow() {
// disable the button here
yourButton->setDisabled(true);
// and connect the thread object signal doneWithWork() to
// a slot of the appropriate object in your GUI thread that will reenable the button
connect(mThread, SIGNAL(doneWithMyWork()), this, SLOT(reenableButton()));
mThread.start();
qDebug() << "Ok";
}
@BTW QThread is not the thread itself its just an interface object for threads. An instance of QThread is not itself running in a seperate thread only the stuff you do in its run() function will be in the context of another thread.
-
Thanks guys. I've changed my code as per your suggestions. Please take a look.
mythread.h
@class MyThread : public QThread {
Q_OBJECT
public:
MyThread();
protected:
void run();
signals:
void _finish();
};@mythread.cpp
@MyThread::MyThread() {}
void MyThread::run() {
for(int i = 0; i < 99999; i++)
qDebug() << "Running : " << i;
emit _finish();
}@concerned code in settings.h
@private:
Ui::Settings ui;
MyThread mThread;
private slots:
void buttonEnable();
void fetchDataNow();@concerned code in settings.cpp
@void Settings::buttonEnable() {
ui->fetchNowButton->setEnabled(true);
}
void Settings::fetchDataNow() {
ui->fetchNowButton->setDisabled(true);
QObject::connect(mThread, SIGNAL(_finish()), this, SLOT(buttonEnable()));
mThread->start();
qDebug() << "Ok";
}@Now another problem arises. When I click on the fetchNowButton in the settings dialog, the program finishes unexpectedly.
@The program has unexpectedly finished.
/home/napster/Programs/Learning-Qt/Convertr-build-desktop/Convertr exited with code 0@
See, the qDebug() << "Ok"; does have no effect. It breaks with the connect() itself.Thanks
-
Hi Volker,
But it worked strangely now, with very minor changes in the code. It still subclasses QThread.
In settings.h
@private:
Ui::Settings ui;
//MyThread mThread;
MyThread mThread;
public slots:
void enableButton();
void fetchDataNow();@In settings.cpp
@void Settings::fetchDataNow() {
ui->fetchNowButton->setDisabled(true);
//QObject::connect(mThread, SIGNAL(_finish()), this, SLOT(buttonEnable()));
QObject::connect(&mThread, SIGNAL(_finish()), this, SLOT(enableButton()));
mThread.start();
qDebug() << "Ok";
}@The commented lines are changed to the one very next to it in each file. The files mythread.h and mythread.cpp are unchanged. Its just a change from pointer object to normal object and it works out of the box. Why it happens? Also, can you explain me the way you suggested also? I mean without subclassing QThread. Also, I've gone through "this":http://developer.qt.nokia.com/wiki/Threads_Events_QObjects when you mentioned it for the first time itself. :)
Thanks in Advance
-
You didn't provide the implementation of your Settings class. As it works now, I would bet a penny that you forgot to actually create the MyThread class in the Settings constructor:
@
// -- PREFERRABLY --Settings::Settings( /* parameters... */, QObject *parent)
: QObject(parent),
mThread(new MyThread)
{
// ...
}// -- OR --
Settings::Settings( /* parameters... */, QObject *parent)
: QObject(parent)
{
mThread = new MyThread;
// ...
}
@ -
I think what Volker is suggesting is something along this lines. The code below was taken from one of the comments to "this":http://labs.qt.nokia.com/2010/06/17/youre-doing-it-wrong/ article.
@
class Producer : public QObject
{
Q_OBJECT
public slots:
void produce() { …emit produced(&data)…emit finished().. }
signals:
void produced(QByteArray *data);
void finished();
};class Consumer : public QObject
{
Q_OBJECT
public slots:
void consume(QByteArray *data) { …emit consumed()…emit finished().. }
signals:
void consumed();
void finished();
};int main(int argc, char **argv)
{
QCoreApplication app(argc, argv);
// create the producer and consumer and plug them together
Producer producer;
Consumer consumer;
producer.connect(&consumer, SIGNAL(consumed()), SLOT(produce()));
consumer.connect(&producer, SIGNAL(produced(QByteArray *)), SLOT(consume(QByteArray *)));// they both get their own thread
QThread producerThread;
producer.moveToThread(&producerThread);
QThread consumerThread;
consumer.moveToThread(&consumerThread);// start producing once the producer’s thread has started
producer.connect(&producerThread, SIGNAL(started()), SLOT(produce()));// when the consumer is done, it stops its thread
consumerThread.connect(&consumer, SIGNAL(finished()), SLOT(quit()));
// when consumerThread is done, it stops the producerThread
producerThread.connect(&consumerThread, SIGNAL(finished()), SLOT(quit()));
// when producerThread is done, it quits the application
app.connect(&producerThread, SIGNAL(finished()), SLOT(quit()));// go!
producerThread.start();
consumerThread.start();
return app.exec();
}
@ -
@Volker:
Calling subclassing QThread "wrong" seems a little strong to me, given even the official training material of less than 2 years ago provided this as the "official way".You can always get smarter, and it might not be the best way to do it, but code that subclasses QThread should work - at least if you do everything else right.
-
@Asperamanca
It's not "my" definition of wrong. It's more or less a "citation":http://labs.qt.nokia.com/2010/06/17/youre-doing-it-wrong/. I agree that the "new" sight on QThread is around for only a short time now - one and a half year since Bradley's blog post. Since Qt 4.4 (released May 2008) QThread::run() is no longer pure virtual, so subclassing isn't needed anymore since then. You can write code that works perfectly with subclassing QThread - that what was needed until 4.4 hit the world. But nowadays it isn't the recommended way anymore as it often led to buggy code.And as paradigms change, one should not adhere to old habits in case there are newer, less error prone ones around, especially when writing new code. So if you're subclassing QThread and do not give very good reasons for doing so, the expectable answer here in the forums is to change the code.