[Solved] QThread blocking GUI
-
Regarding mainwindow.cpp: How do you actually give the worker some work to do?
I see you start() a standard QThread, which will simply start the EventLoop inside the new thread.
Also you "moved" the Worker to the context of the new thread.
But where does the Worker object get the signal to actually begin doing some work?
Finally: Why inside the Worker you create a "Gui"? Do not do any GUI stuff in a non-main thread!
-
Use "QtSerialPort":http://qt-project.org/wiki/QtSerialPort.
-
[quote author="MuldeR" date="1357666209"]Regarding mainwindow.cpp: How do you actually give the worker some work to do?
I see you start() a standard QThread, which will simply start the EventLoop inside the new thread.
Also you "moved" the Worker to the context of the new thread.
But where does the Worker object get the signal to actually begin doing some work?[/quote]
The worker is given work by the signal start() emitted by the gui when the user press a "read" button.
[quote author="MuldeR" date="1357666209"]Finally: Why inside the Worker you create a "Gui"? Do not do any GUI stuff in a non-main thread![/quote]
As I said, I am not the original creator of this application and if possible, I would like to avoid rewriting everything.
This is used in a QTabWidget so that every QWidget has it's own class, then a *QWidget getWidget() method is used to set the gui in the main window QTabWidget's content QWidget.
@worker.cpp
QWidget *Worker::getWidget()
{
return (QWidget *) gui;
}@
@mainwindow.cpp
//...
ui->worker_widget = worker->getWidget();
//...
@What I actually thought is that when creating the gui with new the gui would remain in the main thread. When the Worker object is created, it is owned by the main threaded right ? Then so is his child Gui Object.
My real question would then be :
Will worker->moveToThread(thread) move also the child Gui QWidget in the new thread or will it stay in the main thread ?[quote author="kuzulis" date="1357667593"]Use "QtSerialPort":http://qt-project.org/wiki/QtSerialPort.[/quote]
It's a previous version than the one in Qt5 but it's actually what is used.
Thanks for helping.
-
Move everything you do in the constructor of the Worker into the MainWindow::createWorker() function. You are not allowed to do anything GUI related in another thread then the main thread.
Did you already see "this example":http://qt-project.org/wiki/QThreads_general_usage for a worker object approach with QThread ? -
[quote author="KA51O" date="1357718103"]Move everything you do in the constructor of the Worker into the MainWindow::createWorker() function. You are not allowed to do anything GUI related in another thread then the main thread.
Did you already see "this example":http://qt-project.org/wiki/QThreads_general_usage for a worker object approach with QThread ?[/quote]I already read this example from the original blog.
I also moved everything gui-related from my Worker to my MainWindow.
Nothing changed, I really don't understand : now the Gui object is owned by the MainWindow and everything is handled by signals...@mainwindow.cpp
void MainWindow::createWorker()
{worker = new Worker();
worker_gui = new Gui();
thread = new QThread();worker->moveToThread(thread);
ui->worker_widget = worker_gui;
connect(worker_gui, SIGNAL(start()), worker, SLOT(read_data()));
connect(worker_gui, SIGNAL(stop()), worker, SLOT(pause_processing()));
connect(worker, SIGNAL(emitData(QStringList)), worker_gui, SLOT(setRow(QStringList)));thread->start();
}@
-
process_data() recieves the data from the serial bus using a Communication class and adapts it to the desired format. It finally sends the data to the gui using emit_data(QStringList).
What surprises me is that the old design worked, except for the segfaults (probably coming from the non-thread-safe implementation).
The worker code wouldn't be relevant as there is nothing in the constructor now. The main concern would be the while(running) infinite loop in read_data().
-
Infinite loops in an event-driven framework are not really fitting. They block the event queue! Maybe in the old version this was no problem because the worker (or subclassed thread or whatever) didn't need to receive any signals or didn't rely on the eventloop.
So the last thing I can recommend would be to try and refactor the code so you don't need the inifinite loop (try solving it with signals and slots). -
Ok thank you very much.
My gui now reacts, I used the QCoreApplication::processEvents() that I found "here":http://qt-project.org/wiki/Threads_Events_QObjects. So now my signals are correctly sent.
But that does not seem to be the correct way of handling things and, as you said I still don't understand why it kept blocking the main thread. -
Do you call anything that is living in the context of the main thread directly (not using signal slot) from out of the process_data() function in your worker thread ?
Be aware that calling QCoreApplication::processEvents() in an infinite loop will raise the processor workload to 100 %.
EDIT: It almost seems as if the infinite loop in your worker is running in the context of the main thread (although you moved it to the worker thread).
-
I just finished refactoring the read and pause features (now I need to refactor all the others so they are complying to this model). Everything is now handled by signals.
I remplaced the infinite loop by a QTimer so now I can stop() and start() in an easy and most convenient way.