[Solved] Help needed in Thread Coding !!!
-
It's because of this line:
@
timer = new QTimer(this);
@
MyThread is an object that is created and lives in the main thread. timer is created in the worker thread and is given a parent from main thread. This scenario is unsupported by Qt.
Since you take care of destruction of timer yourself anyway you can simply skip giving it a parent:
@
timer = new QTimer();
@Some other comments about your code:
- You create MyThread object in the constructor of ClientApp and destroy it in the destructor. Since lifespan of both objects is the same why not simply make it a member(instead of pointer to dynamic object)?
- This check
@ if(timer != 0) delete timer;@ is pointless. delete 0 is a no-op (standard explicitly states that) so you can simply write delete timer (although see my first point). - QThread::exit() is very unclean way to terminate a thread and is very error prone, especially if you have any reasources created in there (like your timer). It's a straight way to memory leaks.
- It's a question of design, but instead of inheriting QThread and dynamically creating a timer in the worker thread consider creating a custom worker object, use moveToThread, and use startTimer and timerEvent. This will decouple the actual worker code from the boilerplate thread handling code and make it clearer and easier to maintain (eg. switch QThread to QThreadPool or QtConcurrent in the future).
- I know this is just an example but managing app state via reading of text on a button is not very nice. Create proper state variables and enums. Text changes and compiler won't help you if you slip!
- I cringe every time I see (somePointer == 0). Either use (!somePointer) or consider C++11/14 nullptr. It's time.
-
Many thanks for your valuable reply chrisaverage, can you give a lime light on using ui components in a thread in any way possible please..!!
-
- You create MyThread object in the constructor of ClientApp and destroy it in the destructor. Since lifespan of both objects is the same why not simply make it a member(instead of pointer to dynamic object)?
Hello Chrisaverage,
Am a newbie for this C++ Qt application development, regarding your above comment can you mention how can I make a thread a member of the ClientApp. -
[quote author="srikanth.koorma" date="1411912740"]can you give a lime light on using ui components in a thread[/quote]
Sure: don't ;)
To be more specific - widgets are to be created, updated and handled only in the main (UI) thread. If you need to update a widget from a worker thread use signals/slots.
More on the topic "here":http://qt-project.org/doc/qt-5/thread-basics.html#gui-thread-and-worker-thread -
[quote author="srikanth.koorma" date="1411913107"]can you mention how can I make a thread a member of the ClientApp.[/quote]
Istead of this:
@
class ClientApp : public QMainWindow, public Ui::ClientApp
{
...
private:
MyThread* threads;
};ClientApp::ClientApp(QWidget *parent):QMainWindow(parent),Ui::ClientApp()
{
...
threads = new MyThread(10, this);
}@
simply do this:
@
class ClientApp : public QMainWindow, public Ui::ClientApp
{
...
private:
MyThread threads;
};//if you need to initialize it to something other than the defaults then also
ClientApp::ClientApp(QWidget *parent):QMainWindow(parent),Ui::ClientApp(), threads(10, this)
{ ... }
@
Btw. Why is it called "threads" if it's only a single thread object? :) -
Haha actually I got a code from online, there the coder used a Qlist for multiple threads in the following way
@QList<MyThread*> threads;@
So i just deleted Qlist and used the same code as a sample code for my application.
Once again thanks for your valuable suggestions Chrisaverage, can you give me a snippet code for using ui components for a worker thread.??
-
[quote author="srikanth.koorma" date="1411914284"]can you give me a snippet code for using ui components for a worker thread.??[/quote]
Something like this (I'm writing as I go, assume some minor errors):
@
class SomeWorker: public QObject {
...
public slots:
void doSomeWork();
signals:
somethingHappened(int);
}SomeWorker::doSomeWork() {
...
//worker signals change in the ui needed
emit somethingHappened(42);
}class SomeWidget : public QWidget {
...
public slots:
void updateUi(int value);
private:
QThread thread;
SomeWorker worker;
}SomeWidget::SomeWidget(QWidget* parent) : QWidget(parent) {
worker.moveToThread(&thread);
connect(&thread, &QThread::started, &worker, &SomeWorker::doSomeWork);
connect(&worker, &SomeWorker::somethingHappened, this, &SomeWidget::updateUi);
thread.start();
}void SomeWidget::updateUi(int value) {
//ui update is done in the ui thread
ui->someWidget->setValue(value);
}
@ -
I'll try and will get back to you soon Chrisaverage. Once again very thanks for your valuable suggestions and snippet codes. You made my day. Am really greatful to you..!! :)
-
Error in Application output:
@
Starting /home/chocky/Desktop/mtech_project/build-ClientApp-Desktop_Qt_5_3_GCC_64bit-Debug/ClientApp...
QObject::connect: No such signal MyThread::display(QString data) in ../ClientApp/clientapp.cpp:34
QObject::connect: (receiver name: 'ClientApp')
/home/chocky/Desktop/mtech_project/build-ClientApp-Desktop_Qt_5_3_GCC_64bit-Debug/ClientApp exited with code 0
@Header file:
@
#ifndef CLIENTAPP_H
#define CLIENTAPP_H#include <QMainWindow>
#include "ui_clientapp.h"#include <QThread>
#include <QTimer>namespace Ui {
class ClientApp;
}class MyThread : public QThread
{
Q_OBJECT;
public:
explicit MyThread(int interval, QObject* parent = 0);
~MyThread();signals:
void valueChanged(int);
void display(QString);private slots:
void count(void);protected:
void run(void);private:
int i;
int inc;
int intvl;QTimer* timer;
};
class ClientApp : public QMainWindow, public Ui::ClientApp
{
Q_OBJECTpublic:
explicit ClientApp(QWidget *parent = 0);
~ClientApp();signals:
private slots:
void test(int value);
void on_pushButton_clicked();
void updateUi(QString);
private:
MyThread thread;
};#endif // CLIENTAPP_H
@Source file:
@
#include "clientapp.h"
#include "ui_clientapp.h"ClientApp::ClientApp(QWidget *parent):QMainWindow(parent),Ui::ClientApp(),thread(10, this)
{
setupUi(this);
connect(&thread, SIGNAL(valueChanged(int)),this, SLOT(test(int)));
connect(&thread, SIGNAL(display(QString data)),this, SLOT(&ClientApp::updateUi(QString data)));
}ClientApp::~ClientApp()
{
if(thread.isRunning())
thread.exit(0);
}void ClientApp::test(int value)
{
label->setText(QString("Current Thread Processing Status : %1").arg(value));
}MyThread::MyThread(int interval, QObject* parent):QThread(parent),i(0),inc(-1),intvl(interval),timer(0)
{}
MyThread::~MyThread()
{
delete timer;
}void MyThread::run(void)
{
if(timer == 0)
{
timer = new QTimer();
connect(timer, SIGNAL(timeout()), this, SLOT(count()));
}
timer->start(intvl);
exec();
}void ClientApp::updateUi(QString data)
{
label_2->setText(data.toUtf8());
}void MyThread::count(void)
{
if((i >= 100) or ( i <= 0))
inc = -inc;i += inc; emit valueChanged(i); emit display(QString::fromUtf8("Hello"));
}
void ClientApp::on_pushButton_clicked()
{
if(pushButton->text() == "Start Searching")
{
pushButton->setText("Stop Searching");
thread.start();
}
else
{
pushButton->setText("Start Searching");
thread.exit(0);
}
}
@ -
@connect(&thread, SIGNAL(display(QString data)),this, SLOT(&ClientApp::updateUi(QString data)));
@ should be
@connect(&thread, SIGNAL(display(QString)),this, SLOT(updateUi(QString)));
@
Note no explicit parameter name (data) and class member pointer (&ClientApp::)
The latter is only used in the new Qt5 connection syntax (like in my examples). If you're using the old SIGNAL and SLOT macros syntax omit it. -
Chrisaverage, you are a star pal. You made my half of the PG project work. Thanks for your great support..!! :)