What is the problem with this QThread class?
-
I would say that using Singals & Slots with a queued connections, which makes sure the Slot is executed in the context of the receiver object's thread, is the recommended way to pass information from a thread to another one. Having the "worker" thread accessing variables that belong to your GUI class (or to some other thread) is kind of "strange" design. Better make it the other way around: GUI class starts the worker threads and receives the result, via Signal & Slots, when the thread is done. If you have huge data structures that you need to pass when the thread has finished (and that you don't want to pass via signal), why not simply make your GUI class wait for the QThread's finished() signal and then fetch the result via some getter function?
About using a Mutex: Yes, you can use a mutex to protect concurrent access to an object. But this works only if really all access to that object is protected by the same Mutex. If you only protect the access that is done by your "worker" thread, this doesn't prevent other threads, like the GUI thread, from accessing the object in an unprotected way! And remember that Qt itself will access GUI object, e.g. for redrawing. This happens in Qt "internal" code, so you cannot protect the access via Mutex there (except by modifying Qt).
--
Example:
@MyGuiClass::startThread(void)
{
m_thread = new MyWorkerThread();
connect(m_thread, SIGNAL(progressChanged(int)), this, SLOT(threadProgressChanged(int)), Qt::QueuedConnection);
connect(m_thread, SIGNAL(finished()), this, SLOT(threadFinished()), Qt::QueuedConnection);
m_thread->start();
}MyGuiClass::threadProgressChanged(int x)
{
ui->myLabel->setText(tr("Progress: %1").arg(QString::number(x)));
}MyGuiClass::threadFinished(void)
{
QString result = m_thread->getResult();
ui->myLabel->setText(tr("Result:: %1").arg(result);
delete m_thread;
}@ -
Thank you so much sir.Plz help me in my requirement.
Requirement: the helper thread will copy the Files(may be 1000s each with 5-7 mb) .At a given time GUI thread want to know last index of QStringList copied.( as GUI thread is faster and helper thread will be copying when GUI want to know how many files copied).*Approach1 *after copy of each file the helper thread will send a signal with the index no of the QStringList copied and main GUI thread will update its class variable in its slots and can retrieve the data from class variables.
PitFall-may be too much signal slot to process, at a given time GUI thread can not get updated data (last copied index) as it has to process slots to get update, but its busy in its own thread.
*Approach2 *the helper thread object has its own class variable to store last index copied, which is guarded by mutex.The main GUI thread now can access the variable of helper thread to get last index of QStringList copied.
Approach3 The Gui thread has own class variable to store last index copied, and guarded by mutex. After copy of each file, the helper thread updates the class variable of main GUI thread and GUI thread get updated data when needed.
pit fall- although a helper thread will access some non GUI data of GUI main thread, Its still not recommended.Is not it?
From above points 2 seems to be better approach than 1.Please give me your suggestion.Thank you in advance
[quote author="MuldeR" date="1383939351"]I would say that using Singals & Slots with a queued connections, which makes sure the Slot is executed in the context of the receiver object's thread, is the recommended way to pass information from a thread to another one. Having the "worker" thread accessing variables that belong to your GUI class (or to some other thread) is kind of "strange" design. Better make it the other way around: GUI class starts the worker threads and receives the result, via Signal & Slots, when the thread is done. If you have huge data structures that you need to pass when the thread has finished (and that you don't want to pass via signal), why not simply make your GUI class wait for the QThread's finished() signal and then fetch the result via some getter function?
About using a Mutex: Yes, you can use a mutex to protect concurrent access to an object. But this works only if really all access to that object is protected by the same Mutex. If you only protect the access that is done by your "worker" thread, this doesn't prevent other threads, like the GUI thread, from accessing the object in an unprotected way! And remember that Qt itself will access GUI object, e.g. for redrawing. This happens in Qt "internal" code, so you cannot protect the access via Mutex there (except by modifying Qt).
--
Example:
@MyGuiClass::startThread(void)
{
m_thread = new MyWorkerThread();
connect(m_thread, SIGNAL(progressChanged(int)), this, SLOT(threadProgressChanged(int)), Qt::QueuedConnection);
connect(m_thread, SIGNAL(finished()), this, SLOT(threadFinished()), Qt::QueuedConnection);
m_thread->start();
}MyGuiClass::threadProgressChanged(int x)
{
ui->myLabel->setText(tr("Progress: %1").arg(QString::number(x)));
}MyGuiClass::threadFinished(void)
{
QString result = m_thread->getResult();
ui->myLabel->setText(tr("Result:: %1").arg(result);
delete m_thread;
}@[/quote] -
Hi,
Approach 1 is the best. Approachs 2 & 3 need mutexes to protect shared data -- that's not as good.
[quote]PitFall-may be too much signal slot to process, at a given time GUI thread can not get updated data (last copied index) as it has to process slots to get update, but its busy in its own thread.[/quote]You don't need to worry about this. The signals and slots are queued. If the GUI thread is busy when the signal arrives, the signal will be stored in a queue. When the GUI thread has finished its current task, it will process the queued signal.
-
But when processing the data in main GUI(not after it finished), I need updated data .That is before main thread comes to rest,somewhere it needs updated information , how many files copied.then what to do?
[quote author="JKSH" date="1384062432"]Hi,Approach 1 is the best. Approachs 2 & 3 need mutexes to protect shared data -- that's not as good.
[quote]PitFall-may be too much signal slot to process, at a given time GUI thread can not get updated data (last copied index) as it has to process slots to get update, but its busy in its own threaromd.[/quote]You don't need to worry about this. The signals and slots are queued. If the GUI thread is busy when the signal arrives, the signal will be stored in a queue. When the GUI thread has finished its current task, it will process the queued signal.[/quote]
-
Why not simply pass the QStringList in the constructor to the thread, so the thread has its own "internal" copy? Then make a signal of type fileSelectedForCopy(QString) or fileHasBeenCopied(QString) that the worker thread will emit as soon as it starts or completes copying a specific file? And don't worry about passing strings via signals. Thanks to Qt's "implicit data sharing":http://harmattan-dev.nokia.com/docs/library/html/qt4/implicit-sharing.html, you pretty much are only passing a pointer - shallow copy (though with full guarantee that each String object can be used like an independent String object).
Of course you could as well pass an Integer that is in the index within the StringList. But then you need to worry whether the Thread's internal list and the GUI's list are still the same - which might not be the case...
--
BTW: If you think you are sending too many signals from worker thread to GUI, think about interrupt coalescing. It pretty much means you add an QElapsedTimer to the worker thread and check how long ago the last update signal was sent. If time is below a certain threshold, you simply don't send an update (yet). This avoids sending a zillion of update signals when processing very small files (that are copied very fast).
@WorkerThread::sendUpdate(const QString &fileName)
{
if(m_elapsedTimer.elapsed() > THRESHOLD)
{
emit fileHasBeenCopied(fileName);
m_elapsedTimer.restart();
}
}@ -
Dear Sir again thank you very much for your explanation.I need The gui thread will start the helper thread and need updates before it comes to rest.that means after starting the helper, it executes some code, where the proceedings of helper is taken as parameters.So if helper is sending signals, then the main GUI thread must stay ideal to proceed the internal events(signals).but how it is possible for the Gui thread to take updates from Helper before it comes to ideal?
[quote author="MuldeR" date="1384086961"]Why not simply pass the QStringList in the constructor to the thread, so the thread has its own "internal" copy? Then make a signal of type fileSelectedForCopy(QString) or fileHasBeenCopied(QString) that the worker thread will emit as soon as it starts or completes copying a specific file? And don't worry about passing strings via signals. Thanks to Qt's "implicit data sharing":http://harmattan-dev.nokia.com/docs/library/html/qt4/implicit-sharing.html, you pretty much are only passing a pointer - shallow copy (though with full guarantee that each String object can be used like an independent String object).Of course you could as well pass an Integer that is in the index within the StringList. But then you need to worry whether the Thread's internal list and the GUI's list are still the same - which might not be the case...
--
BTW: If you think you are sending too many signals from worker thread to GUI, think about interrupt coalescing. It pretty much means you add an QElapsedTimer to the worker thread and check how long ago the last update signal was sent. If time is below a certain threshold, you simply don't send an update (yet). This avoids sending a zillion of update signals when processing very small files (that are copied very fast).
@WorkerThread::sendUpdate(const QString &fileName)
{
if(m_elapsedTimer.elapsed() > THRESHOLD)
{
emit fileHasBeenCopied(fileName);
m_elapsedTimer.restart();
}
}@[/quote] -
WIth Signals & Slots you don't need to worry. GUI thread (and any thread running an event loop) will continuously process events. So it's all event based! If GUI thread needs some info from "worker" thread before it can proceed with function XYZ, simply call function XYZ from the Slot that will be invoked by the "worker" thread (via signal) as soon as that info is present.
Of course GUI thread can do other work in the meantime. Signals will never get lost! All pending signals are processed as soon as the GUI thread returns to the event loop. Nevertheless, you should avoid running lengthy (blocking) function calls in the GUI thread. This would "block" the GUI thread and your application window will appear to "freeze". Also no incoming signals (e.g. from "worker" thread) are processed while the GUI thread is in a "blocked" state!
(Though signals that arrive while the GUI thread is in "blocked" state are not lost. They are processed when the GUI thread returns to its event loop!)
__
Note: QApplication::processEvents() may be used to process all pending signals within a "long" function, but be very careful with it! It can happen that processEvents() processes an event that calls a Slot and that Slot in turn may call other functions. It's not easy to understand what can happen and you can even end up in an infinite recursion.
-
Of course Signals will not be lost. But, I need updated value in GUI thread, which ll be updated in Slot executed when the signal sent by helper.may be processEvents() is useful, but should use it again and again in the code to get updated values, before GUI thread returns to event loop.isn't it.So for a conclusion === SIGNAL/SLOT is the safest mechanism to implement than mutex in helper thread and accesing the value by main thread through helper Thread object.
[quote author="MuldeR" date="1384088661"]WIth Signals & Slots you don't need to worry. GUI thread (and any thread running an event loop) will continuously process events. So it's all event based! If GUI thread needs some info from "worker" thread before it can proceed with function XYZ, simply call function XYZ from the Slot that will be invoked by the "worker" thread (via signal) as soon as that info is present.Of course GUI thread can do other work in the meantime. Signals will never get lost! All pending signals are processed as soon as the GUI thread returns to the event loop. Nevertheless, you should avoid running lengthy (blocking) function calls in the GUI thread. This would "block" the GUI thread and your application window will appear to "freeze". Also no incoming signals (e.g. from "worker" thread) are processed while the GUI thread is in a "blocked" state!
(Though signals that arrive while the GUI thread is in "blocked" state are not lost. They are processed when the GUI thread returns to its event loop!)
__
Note: QApplication::processEvents() may be used to process all pending signals within a "long" function, but be very careful with it! It can happen that processEvents() processes an event that calls a Slot and that Slot in turn may call other functions. It's not easy to understand what can happen and you can even end up in an infinite recursion.[/quote]
-
I think your design to "actively" wait for the updated value is not good. Think of a different design! Instead of making a monolithic function that waits for the updated value, cut it down into several functions. Function (1) can start the "worker" thread. Function (2) can do some other work that is not dependent on result from "worker" thread. Function (3) does the stuff that depends on result from "worker" thread. Now function (2) might be invoked immediately after function (1), but function (3) will not be invoked before the "worker" thread sends the signal. You can invoke function (3) directly from the Slot that receives the signal. There is no need to actively "wait" here.
__
In case you really need to wait for a signal in a "synchronous" way (but without making your application freeze!), you can still do this:
@QEventLoop loop;
connect(myThread, SIGNAL(someSignal()), &loop, SLOT(quit()));
loop.exec(); //<-- Event processing until thread is done@But you should ONLY use this as a "last resort" solution, as it can have some unexpected behavior!
-
yes. i think I ve to change my design. and in slot i ve to update value, and when needed updated value, the GUI thread will processEvents(), then access the value.
but my aim to create a helper thread is-to copy files(time taking operation), and my GUI ll show only few (6) widgets having one of the properties as the new file name.till the file is not set to individual widgets,no small widgets will be displayed. hence I need updated value and then only i can show widgets. So any how i ve to wait, whether the image is set to widget/not. but i can set individual image to small widgets by GUI thread in a slot,without depending on helper thread to set image on small widget(which cause of freezing).
Thank you so much sir for sharing some valuable suggestion.Thanks a lot.
[quote author="MuldeR" date="1384091572"]I think your design to "actively" wait for the updated value is not good. Think of a different design! Instead of making a monolithic function that waits for the updated value, cut it down into several functions. Function (1) can start the "worker" thread. Function (2) can do some other work that is not dependent on result from "worker" thread. Function (3) does the stuff that depends on result from "worker" thread. Now function (2) might be invoked immediately after function (1), but function (3) will not be invoked before the "worker" thread sends the signal. You can invoke function (3) directly from the Slot that receives the signal. There is no need to actively "wait" here.__
In case you really need to wait for a signal in a "synchronous" way (but without making your application freeze!), you can still do this:
@QEventLoop loop;
connect(myThread, SIGNAL(someSignal()), &loop, SLOT(quit()));
loop.exec(); //<-- Event processing until thread is done@But you should ONLY use this as a "last resort" solution, as it can have some unexpected behavior![/quote]
-
You don't have to call processEvents(). Especially don't call it in order to "access" some value, because there is absolutely no guarantee that the desired Signal will arrive during your processEvents() call. And also please do not call processEvents() in a loop or something. That's really bad design.
Instead: Simply do nothing at all! As soon as the desired Signal arrives, the Slot connected to that signal will be invoked - fully automatically. Then, whatever needs to be done with the "new" information can be done in the that Slot - or in some function that is now invoked from that Slot.
There really should be no need to "actively" wait for the signal...
-
Is it possible for the main thread to process events while running some other codes?If not then how it is possible to get updated data while running.please tell me how to design it?please go through the previous reply, i ve edited my requirement in previous post.
may be I need to implement mutex in helper thread and access data from GUI thread,and wait for specific value and then proceed.
[quote author="MuldeR" date="1384093010"]You don't have to call processEvents(). Especially don't call it in order to "access" some value, because there is absolutely no guarantee that the desired Signal will arrive during your processEvents() call. And also please do not call processEvents() in a loop or something. That's really bad design.Instead: Simply do nothing at all! As soon as the desired Signal arrives, the Slot connected to that signal will be invoked - fully automatically. Then, whatever needs to be done with the "new" information can be done in the that Slot - or in some function that is now invoked from that Slot.
There really should be no need to "actively" wait for the signal...[/quote]
-
No, if your GUI thread is doing something else, it obviously can NOT process signals at the same time, but it will do so, as soon as it returns to the event loop. Also, with a proper design, the GUI thread will spent about 99% of its time in the event loop, so signals should be caught up very quickly under normal circumstances....
__Why not simply:
@void MyMainWindow::init()
{
doSomeInitializationStuffFirst();m_thread = new MyWorkerThread(/*some input data*/); connect(m_thread, SIGNAL(newInfoAvail(Foo)), this, SLOT(processNewInfo(Foo))); m_thread.start(); doOtherStuffHereThatDoesNotNeedTheThreadResult();}
//SLOT
void MyMainWindow::processNewInfo(Foo bar)
{
//following function can update the GUI
doWhateverNeedsToBeDoneWithTheNewInfo(bar);
}@NOTE: After init() function exists, your GUI thread will simply keep on processing events until a Signal form the worker thread arrives - in which case the Slot will be invoked for further processing.
No processEvents() required anywhere...
-
thank you very much sir. I ll redesign according to my requirement and yur suggestions.
__Why not simply:
@void MyMainWindow::init()
{
doSomeInitializationStuffFirst();m_thread = new MyWorkerThread(/*some input data*/); connect(m_thread, SIGNAL(newInfoAvail(Foo)), this, SLOT(processNewInfo(Foo))); m_thread.start(); doOtherStuffHereThatDoesNotNeedTheThreadResult();}
void MyMainWindow::processNewInfo(Foo bar)
{
doWhateverNeedsToBeDoneWithTheNewInfo(bar);
}@No processEvents() required anywhere...[/quote]
-
[quote]Is it possible for the main thread to process events while running some other codes?If not then how it is possible to get updated data while running.please tell me how to design it?please go through the previous reply, i ve edited my requirement in previous post.
may be I need to implement mutex in helper thread and access data from GUI thread,and wait for specific value and then proceed.[/quote]I don't see how this should be a problem ???
Example:
@void MyMainWindow::init()
{
QStringList filesToBeCopied = generateFileList();m_thread = new MyFileCopyThread(filesToBeCopied); connect(m_thread, SIGNAL(copyStarted(QString)), this, SLOT(copyStarted(QString))); connect(m_thread, SIGNAL(copyCompleted(QString)), this, SLOT(copyCompleted(QString))); m_thread.start(); doOtherStuffHereThatDoesNotNeedTheThreadResult();}
//SLOT
void MyMainWindow::copyStarted(QString name)
{
//Just an example. You can do whatever needs to be done!
myStatusLabel->setText(tr("Now Copying: %1").arg(name));
}//SLOT
void MyMainWindow::copyCompleted(QString name)
{
//Just an example. You can do whatever needs to be done!
myStatusLabel->setText(tr("Copy complete: %1").arg(name));
}@NOTE: After init() function exists, your GUI thread will simply keep on processing events until a Signal form the worker thread arrives – in which case the Slot will be invoked for further processing.
No processEvents() required anywhere…
-
Sir tell me one thing. is there any problem if i access the HelperThread object data through my main thread with ReadWritemutex in helper thread function. Is there any problem here?
[quote author="MuldeR" date="1384094383"][quote]Is it possible for the main thread to process events while running some other codes?If not then how it is possible to get updated data while running.please tell me how to design it?please go through the previous reply, i ve edited my requirement in previous post.
may be I need to implement mutex in helper thread and access data from GUI thread,and wait for specific value and then proceed.[/quote]I don't see how this should be a problem ???
Example:
@void MyMainWindow::init()
{
QStringList filesToBeCopied = generateFileList();m_thread = new MyFileCopyThread(filesToBeCopied); connect(m_thread, SIGNAL(copyStarted(QString)), this, SLOT(copyStarted(QString))); connect(m_thread, SIGNAL(copyCompleted(QString)), this, SLOT(copyCompleted(QString))); m_thread.start(); doOtherStuffHereThatDoesNotNeedTheThreadResult();}
//SLOT
void MyMainWindow::copyStarted(QString name)
{
//Just an example. You can do whatever needs to be done!
MyStatusLabel->setText(tr("Now Copying: %1").arg(name));
}//SLOT
void MyMainWindow::copyCompleted(QString name)
{
//Just an example. You can do whatever needs to be done!
MyStatusLabel->setText(tr("Copy complete: %1").arg(name));
}@NOTE: After init() function exists, your GUI thread will simply keep on processing events until a Signal form the worker thread arrives – in which case the Slot will be invoked for further processing.
No processEvents() required anywhere…[/quote]
-
[quote author="blue_sky" date="1384095586"]Sir tell me one thing. is there any problem if i access the HelperThread object data through my main thread with ReadWritemutex in helper thread function. Is there any problem here?[/quote]
Not quite sure what you mean?
But you can add a getter and setter function to your Worker thread like this:
@Foo MyWorkerThread::getData(void) const
{
QReadLocker readLock(&m_readWriteLock);
//real assignment operator below, NOT pointer assignment!
Foo data = m_data;
return data;
}MyWorkerThread::setData(const Foo &data)
{
QWriteLocker writeLock(&m_readWriteLock);
//real assignment operator below, NOT pointer assignment!
m_data = data;
}@Now make sure that all access to m_data is done via getData() and setData(). This applies to the GUI thread and the WorkerThread itself!
BUT: If communication between GUI thread and WorkerThread is done solely with Signals & Slots and queued connections, there is no need for such getter/setter functions. Just pass data "by value" in the Signal.
