January 5, 2012

Subin Sebastian Subin Sebastian
Lab Rat
33 posts

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.

  1. //mythread.h
  2. #ifndef MYTHREAD_H
  3. #define MYTHREAD_H
  4. #include <QtCore>
  5. #include <QtGui>
  6. class MyThread  : public QThread    {
  7. private:
  8.     QToolButton* _toolButton;
  9. public:
  10.     MyThread();
  11.     MyThread(QToolButton*);
  12.     void run();
  13. };
  14. #endif // MYTHREAD_H

  1. //mythread.cpp
  2. #include "mythread.h"
  3. MyThread::MyThread() {}
  4. MyThread::MyThread(QToolButton* toolButton) {
  5.     this->_toolButton = toolButton;
  6. }
  7. void MyThread::run()    {
  8.     _toolButton->setDisabled(true);
  9.     for(int i = 0; i < 99999; i++)
  10.         qDebug() << "Running : " << i;
  11.     _toolButton->setDisabled(false);
  12. }

  1. //settings.h
  2. #ifndef SETTINGS_H
  3. #define SETTINGS_H
  4. #include <QDialog>
  5. #include <QtCore>
  6. #include "mythread.h"
  7. namespace Ui {
  8.     class Settings;
  9. }
  10. class Settings : public QDialog {
  11.     Q_OBJECT
  12. public:
  13.     explicit Settings(QWidget *parent = 0);
  14.     ~Settings();
  15. private:
  16.     Ui::Settings *ui;
  17.     MyThread mThread;
  18. private slots:
  19.     void fetchDataNow();
  20. };
  21. #endif // SETTINGS_H

  1. //settings.cpp
  2. #include "settings.h"
  3. #include "ui_settings.h"
  4. Settings::Settings(QWidget *parent) : QDialog(parent), ui(new Ui::Settings) {
  5.     ui->setupUi(this);
  6.     mThread = new MyThread(ui->fetchNowButton); //This line causes the problem
  7. }
  8. Settings::~Settings()   {
  9.     delete ui;
  10. }
  11. void Settings::fetchDataNow()   {
  12.     mThread.start();
  13.     qDebug() << "Ok";
  14. }

But when I build this application, I get the following error.

  1. ../Convertr/settings.cpp: In constructor ‘Settings::Settings(QWidget*):
  2. ../Convertr/settings.cpp:6:46: error: no match for ‘operator=’ in ‘((Settings*)this)->Settings::mThread = (operator new(12u), (<statement>, ((MyThread*)<anonymous>)))
  3. ../Convertr/settings.cpp:6:46: note: candidate is:
  4. ../Convertr/mythread.h:6:7: note: MyThread& MyThread::operator=(const MyThread&)
  5. ../Convertr/mythread.h:6:7: note:   no known conversion for argument 1 from ‘MyThread*’ to ‘const MyThread&
  6. make: *** [settings.o] Error 1

Can someone help me to fix this?

Thanks in Advance

14 replies

January 5, 2012

Asperamanca Asperamanca
Robot Herder
649 posts

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 [developer.qt.nokia.com]

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.

January 5, 2012

Subin Sebastian Subin Sebastian
Lab Rat
33 posts

@Asperamanca,

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

January 5, 2012

dvez43 dvez43
Ant Farmer
317 posts

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!

January 5, 2012

Volker Volker
Ant Farmer
5428 posts

Napster wrote:

  1. //mythread.h
  2. #ifndef MYTHREAD_H
  3. #define MYTHREAD_H
  4. #include <QtCore>
  5. #include <QtGui>
  6. class MyThread  : public QThread    {
  7. private:
  8.     QToolButton* _toolButton;
  9. public:
  10.     MyThread();
  11.     MyThread(QToolButton*);
  12.     void run();
  13. };
  14. #endif // MYTHREAD_H

Thanks in Advance

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.

January 5, 2012

Asperamanca Asperamanca
Robot Herder
649 posts

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.

January 5, 2012

KA51O KA51O
Hobby Entomologist
478 posts

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.

  1. void Settings::fetchDataNow()   {
  2.     // disable the button here
  3.     yourButton->setDisabled(true);
  4.     // and connect the thread object signal doneWithWork() to
  5.     // a slot of the appropriate object in your GUI thread that will reenable the button
  6.     connect(mThread, SIGNAL(doneWithMyWork()), this, SLOT(reenableButton()));
  7.     mThread.start();
  8.     qDebug() << "Ok";
  9. }

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.

January 5, 2012

Subin Sebastian Subin Sebastian
Lab Rat
33 posts

Thanks guys. I’ve changed my code as per your suggestions. Please take a look.

mythread.h

  1. class MyThread  : public QThread    {
  2.     Q_OBJECT
  3. public:
  4.     MyThread();
  5. protected:
  6.     void run();
  7. signals:
  8.     void _finish();
  9. };

mythread.cpp

  1. MyThread::MyThread() {}
  2. void MyThread::run()    {
  3.     for(int i = 0; i < 99999; i++)
  4.         qDebug() << "Running : " << i;
  5.     emit _finish();
  6. }

concerned code in settings.h

  1. private:
  2.     Ui::Settings *ui;
  3.     MyThread* mThread;
  4. private slots:
  5.     void buttonEnable();
  6.     void fetchDataNow();

concerned code in settings.cpp

  1. void Settings::buttonEnable()  {
  2.     ui->fetchNowButton->setEnabled(true);
  3. }
  4. void Settings::fetchDataNow()   {
  5.     ui->fetchNowButton->setDisabled(true);
  6.     QObject::connect(mThread, SIGNAL(_finish()), this, SLOT(buttonEnable()));
  7.     mThread->start();
  8.     qDebug() << "Ok";
  9. }

Now another problem arises. When I click on the fetchNowButton in the settings dialog, the program finishes unexpectedly.

  1. The program has unexpectedly finished.
  2. /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

January 6, 2012

Volker Volker
Ant Farmer
5428 posts

You’re doing it wrong.™

Read Threads, Events and QObjects wiki article and change your code (hint: do not subclass QThread).

January 6, 2012

Subin Sebastian Subin Sebastian
Lab Rat
33 posts

Hi Volker,

But it worked strangely now, with very minor changes in the code. It still subclasses QThread.

In settings.h

  1. private:
  2.     Ui::Settings *ui;
  3.     //MyThread* mThread;
  4.     MyThread mThread;
  5. public slots:
  6.     void enableButton();
  7.     void fetchDataNow();

In settings.cpp

  1. void Settings::fetchDataNow()   {
  2.     ui->fetchNowButton->setDisabled(true);
  3.     //QObject::connect(mThread, SIGNAL(_finish()), this, SLOT(buttonEnable()));
  4.     QObject::connect(&mThread, SIGNAL(_finish()), this, SLOT(enableButton()));
  5.     mThread.start();
  6.     qDebug() << "Ok";
  7. }

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 [developer.qt.nokia.com] when you mentioned it for the first time itself. :)

Thanks in Advance

January 6, 2012

Volker Volker
Ant Farmer
5428 posts

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:

  1. // -- PREFERRABLY --
  2.  
  3. Settings::Settings( /* parameters... */, QObject *parent)
  4.     : QObject(parent),
  5.       mThread(new MyThread)
  6. {
  7.     // ...
  8. }
  9.  
  10. // -- OR --
  11.  
  12. Settings::Settings( /* parameters... */, QObject *parent)
  13.     : QObject(parent)
  14. {
  15.     mThread = new MyThread;
  16.     // ...
  17. }

January 9, 2012

KA51O KA51O
Hobby Entomologist
478 posts

I think what Volker is suggesting is something along this lines. The code below was taken from one of the comments to this [labs.qt.nokia.com] article.

  1. class Producer : public QObject
  2. {
  3.    Q_OBJECT
  4.    public slots:
  5.       void produce() { …emit produced(&data)…emit finished().. }
  6.    signals:
  7.       void produced(QByteArray *data);
  8.       void finished();
  9. };
  10.  
  11. class Consumer : public QObject
  12. {
  13.    Q_OBJECT
  14.    public slots:
  15.       void consume(QByteArray *data) { …emit consumed()…emit finished().. }
  16.    signals:
  17.       void consumed();
  18.       void finished();
  19. };
  20.  
  21. int main(int argc, char **argv)
  22. {
  23.    QCoreApplication app(argc, argv);
  24.    // create the producer and consumer and plug them together
  25.    Producer producer;
  26.    Consumer consumer;
  27.    producer.connect(&consumer, SIGNAL(consumed()), SLOT(produce()));
  28.    consumer.connect(&producer, SIGNAL(produced(QByteArray *)), SLOT(consume(QByteArray *)));
  29.  
  30.    // they both get their own thread
  31.    QThread producerThread;
  32.    producer.moveToThread(&producerThread);
  33.    QThread consumerThread;
  34.    consumer.moveToThread(&consumerThread);
  35.  
  36.    // start producing once the producer’s thread has started
  37.    producer.connect(&producerThread, SIGNAL(started()), SLOT(produce()));
  38.  
  39.    // when the consumer is done, it stops its thread
  40.    consumerThread.connect(&consumer, SIGNAL(finished()), SLOT(quit()));
  41.    // when consumerThread is done, it stops the producerThread
  42.    producerThread.connect(&consumerThread, SIGNAL(finished()), SLOT(quit()));
  43.    // when producerThread is done, it quits the application
  44.    app.connect(&producerThread, SIGNAL(finished()), SLOT(quit()));
  45.  
  46.    // go!
  47.    producerThread.start();
  48.    consumerThread.start();
  49.    return app.exec();
  50. }

January 9, 2012

Asperamanca Asperamanca
Robot Herder
649 posts

@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.

January 9, 2012

Volker Volker
Ant Farmer
5428 posts

@Asperamanca
It’s not “my” definition of wrong. It’s more or less a citation [labs.qt.nokia.com]. 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.

January 21, 2012

Subin Sebastian Subin Sebastian
Lab Rat
33 posts

Thanks a lot Volker, Asperamanca & KA51O

 
  ‹‹ qt checksum      Updating QListView from aggrigation thread make the GUI stuck ››

You must log in to post a reply. Not a member yet? Register here!