PDA

View Full Version : Qt multithreading and let the second thread update the main (UI) thread



Cupidvogel
17th April 2015, 22:05
So I have a view (say QFrame) in the main thread (currently there is only a single thread where the entire application resides), where I need to show some query results when the user enters some query term in the text box and presses Enter. This is how it works, I have class called QueryFinder which I instantiate with the query term. Then I call execute(), which polls some internal database for some time to get query results. While instantiating QueryFinder, I also pass the view (the QFrame from which the user queried) and that is held as a view object in the QueryFinder class. Whenever any query result match is found, I call something like fView->updateView(searchResult), and the view has the appropriate method update to show the result. The results keep on coming until the database polling is complete.

Now the database polling is pretty memory intensive, so while the querying goes on, the UI becomes kind of unresponsive. A loading gif image, for example, which signifies the ongoing querying, often stalls and freezes. So I want to separate out the entire querying part into a separate thread, and that thread should poll, and update the view (which is in a separate thread, the main one). How do I do this in Qt/C++? Here is my pseudocode:



//view code
//after user presses enter
if (!finder)
{
finder->stop();
}
finder = new QueryFinder(this, queryTerm);
finder->execute();

void updateView(QueryResult &result)
{
//display the query result properly
}

//QueryFinder code, header file
class QueryFinder: public QObject
{
public:
QUeryFinder(QWidget *, QueryTerm&);
void execute();
void stop();

private:
bool shouldStop;
QWidget *fView;
QueryTerm fTerm;
}


//QueryFinder code, implementation file
QueryFinder::QueryFinder(QWidget *w, QueryTerm& term): fView(w), fTerm(term)
{
shouldStop = false;
}

void QueryFinder::execute()
{
//do the polling and if any result found, say newResult, update view, but only if no new query request has come in
//the meantime, else abnson this object altogether
while (cursor->hasNext()) {
newResult = cursor->getCurResult();
if (!shouldStop)
fView->updateView(newResult);
else
return;
}
}

void QueryFinder::stop()
{
shouldStop = true;
}


If you observe, not only the updating the view, even the stop method needs to go from one thread to another. How do I accomplish this? I have never done any multithreading in Qt/C++.

pixaeiro
17th April 2015, 23:35
Yes, if your task takes a long time and you don't want to freeze the UI thread you need a second worker thread to do the job, and you need to implement the communication between these two threads using signals and slots.

This article will give you an excellent introduction with examples to QThread:

http://blog.debao.me/2013/08/how-to-use-qthread-in-the-right-way-part-1/

Cupidvogel
18th April 2015, 00:31
Yes, I have studied it, but it gives very simplistic examples, I have almost zero idea how to implement it here.

jefftee
18th April 2015, 03:55
My personal preference is to use the "move to thread" method, which is described here:

https://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/

Design your worker object the like a normal class that inherits QObject, add signals/slots necessary to communicate with your main GUI thread, etc. Qt will automatically do the right thing if you connect signals/slots across different threads. Qt::QueuedConnection is used if the sender and receiver are in different threads.

Cupidvogel
18th April 2015, 10:19
Can you provide some pseudo code to show how to at least structure it? I am feeling totally lost..

jefftee
18th April 2015, 10:45
The example I referred you to had example code, no?

anda_skoa
18th April 2015, 11:29
Well, you just have to make updateView() a slot and add a matching signal to the QueryFinder class and connect the two.
Then, instead of calling the view directly, you just emit the signal.

Cheers,
_

Cupidvogel
18th April 2015, 11:36
Okay, I got that part. Here two thread communicate via signals and slots. But in this case, the slot must receive an object of type QueryResult. Qt greatly limits how signals and slots can use parameters, and the one way I can see, is to use a Signal Mapper. But even with Signal Mapper, I don't know how to properly use it to say, emit a signal from one object with a QString as param, and capture and use the sent QString in a slot in another object. I browsed net heavily, I could not find a complete working example of it. Secondly, even Signal Mapper allows only certain basic types of params to be exchanged, like QString. Here the object which is required to update the view is of a custom class, QueryResult. How will the Action class and the View class communicate?


The example I referred you to had example code, no?


Again, this was a pretty basic example. Which class here should inherit from QThread? I couldn't see any threading in your example at all..

anda_skoa
18th April 2015, 12:43
But in this case, the slot must receive an object of type QueryResult.

Yes, I should have written updateView(const QueryResult&), but I thought it would be pretty obvious that the signal would need to transport the data as its argument.



Qt greatly limits how signals and slots can use parameters

In what way?



and the one way I can see, is to use a Signal Mapper

Why on earth would you need a signal mapper?



But even with Signal Mapper, I don't know how to properly use it to say, emit a signal from one object with a QString as param, and capture and use the sent QString in a slot in another object.

You would not use a signal mapper when you can use a trivial signal/slot connection between two objects.



I browsed net heavily, I could not find a complete working example of it.

Heavly for how many seconds?
The first Google hit on signal/slots is the Qt documentation on that topic (which should be the primary source even without search engine help) and it does naturally have an example that transports a value through the connection.



Which class here should inherit from QThread?
The suggestion was to use moveToThread() of the worker object which does not require subclassing QThread.

Cheers,
_

Cupidvogel
18th April 2015, 12:49
Can you give an example of how without using signal mapper I can transport a parameter, say, an object belonging to a custom class?

Which object do I need to move to thread? My Query Action class?

anda_skoa
18th April 2015, 14:14
Can you give an example of how without using signal mapper I can transport a parameter, say, an object belonging to a custom class?

What is it with you and the signal mapper? There is no indication of any need for a signal mapper.
Declare a signal with the respective type as its argument type.



Which object do I need to move to thread? My Query Action class?

Well, the object that you want to run in the other thread.
Probably the instance of your QueryFinder class.

Cheers,
_

jefftee
18th April 2015, 16:43
Can you give an example of how without using signal mapper I can transport a parameter, say, an object belonging to a custom class?

Which object do I need to move to thread? My Query Action class?
Look at the Q_DECLARE_METATYPE macro, which you can used to declare any custom types and the qRegisterMetaType macro. These two macros are needed to register a custom metatype that can be used for signal/slot parameters.

The class you moveToThread is the one that has the signals/slots and code that you wish to execute in a separate thread.

Added after 6 minutes:


Qt greatly limits how signals and slots can use parameters, and the one way I can see, is to use a Signal Mapper.
That is a wildly inaccurate statement. Qt supports all native types, many Qt types, and allows you to register any custom type to the meta object system. How can you say that Qt greatly limits the use of parameters?

anda_skoa
18th April 2015, 19:36
Look at the Q_DECLARE_METATYPE macro, which you can used to declare any custom types and the qRegisterMetaType macro. These two macros are needed to register a custom metatype that can be used for signal/slot parameters.

Just as a clarification: only because this is a cross-thread signal/slot connection (Qt::QueuedConnection).
A direct connection does not require any thing from the argument types.

Cheers,
_

jefftee
18th April 2015, 19:46
Just as a clarification: only because this is a cross-thread signal/slot connection (Qt::QueuedConnection).
A direct connection does not require any thing from the argument types.
Thanks for clarifying. I will usually register my custom types whether or not I am using threads and cross-thread signals/slots. I believe the other beneficial reason to do so is that you can use QVariant for custom types that have been registered, which may come in handy too.

Cupidvogel
18th April 2015, 21:27
Okay. First of all, I am very sorry about my statement about signals and slots. I experimented with them, and I can see that they jolly well accept all sorts of parameters, be standard types or custom objects. Sorry about that.

As for the subject of this thread, I tried the example 2.1 of this article: http://blog.debao.me/2013/08/how-to-use-qthread-in-the-right-way-part-1/

I created a separate WorkerThread class like this:



//header: WorkerThread.hpp

#include <QObject>
#include <QThread>
#include <QDebug>

class WorkerThread : public QObject
{
Q_OBJECT

public:
WorkerThread();

private slots:
void onTimeout()
{
qDebug() << "Worker::onTimeout get called from?: "<< QThread::currentThreadId();
}
};


//implementation: WorkerThread.cpp

#include "WorkerThread.hpp"

WorkerThread::WorkerThread()
{
}


Then in my View class, where I instantiate the action and call its execute method to start the database polling, I added the timer part:



//view class

connect(queryButton, SIGNAL(clicked()), this, SLOT(doQuery()));

void QueryView::doQuery()
{
/*---------------previous code
if (!finder)
{
finder->stop();
}
finder = new QueryFinder(this, queryTerm);
finder->execute();
-----------------*/

//new code:

qDebug() << "From main thread: " << QThread::currentThreadId();

QThread t;
QTimer timer;
WorkerThread worker;

QObject::connect(&timer, SIGNAL(timeout()), &worker, SLOT(onTimeout()));
timer.start(1000);

timer.moveToThread(&t);
worker.moveToThread(&t);

t.start();
}


Now when I click the button, contrary to what is given as output in the example, I get this output:



From main thread: 0xa06771d4
QThread: Destroyed while thread is still running
The program has unexpectedly finished.


followed by the app crashing. The Apple system generated crash report shows this message:



Thread 0:: Dispatch queue: com.apple.main-thread
0 libsystem_kernel.dylib 0x9a4709cf mach_absolute_time + 1
1 com.apple.HIToolbox 0x9bef7957 GetCurrentEventTime + 18
2 com.apple.HIToolbox 0x9beee570 ReceiveNextEventCommon + 301
3 com.apple.HIToolbox 0x9beee42c _BlockUntilNextEventMatchingListInModeWithFilter + 99
4 com.apple.AppKit 0x95da7721 _DPSNextEvent + 742
5 com.apple.AppKit 0x95da6dc5 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 350
6 com.apple.AppKit 0x95d9b77c -[NSApplication run] + 907
7 libqcocoa.dylib 0x05878bf1 0x5858000 + 134129
8 QtCore 0x02d85181 QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 401
9 QtCore 0x02d881ed QCoreApplication::exec() + 349
10 com.<product_id>.<project> 0x000a2d08 main + 344
11 com.<product_id>.<project> 0x000a1c15 start + 53

Thread 1:: Dispatch queue: com.apple.libdispatch-manager
0 libsystem_kernel.dylib 0x9a4788ce kevent64 + 10
1 libdispatch.dylib 0x953a673f _dispatch_mgr_invoke + 245
2 libdispatch.dylib 0x953a63a2 _dispatch_mgr_thread + 52

Thread 2:
0 libsystem_kernel.dylib 0x9a477e6a __workq_kernreturn + 10
1 libsystem_pthread.dylib 0x904b82b1 _pthread_wqthread + 939
2 libsystem_pthread.dylib 0x904b5e2e start_wqthread + 30

Thread 3:
0 libsystem_kernel.dylib 0x9a477e6a __workq_kernreturn + 10
1 libsystem_pthread.dylib 0x904b82b1 _pthread_wqthread + 939
2 libsystem_pthread.dylib 0x904b5e2e start_wqthread + 30

Thread 4:
0 libsystem_kernel.dylib 0x9a477e6a __workq_kernreturn + 10
1 libsystem_pthread.dylib 0x904b82b1 _pthread_wqthread + 939
2 libsystem_pthread.dylib 0x904b5e2e start_wqthread + 30

Thread 5:
0 libsystem_kernel.dylib 0x9a4719ce mach_msg_trap + 10
1 libsystem_kernel.dylib 0x9a470a70 mach_msg + 68
2 com.apple.CoreFoundation 0x9321bef6 __CFRunLoopServiceMachPort + 214
3 com.apple.CoreFoundation 0x9321b309 __CFRunLoopRun + 1529
4 com.apple.CoreFoundation 0x9321aaa6 CFRunLoopRunSpecific + 390
5 com.apple.CoreFoundation 0x9321a90b CFRunLoopRunInMode + 123
6 com.apple.AppKit 0x95e76ea0 _NSEventThread + 283
7 libsystem_pthread.dylib 0x904b7e13 _pthread_body + 138
8 libsystem_pthread.dylib 0x904b7d89 _pthread_start + 162
9 libsystem_pthread.dylib 0x904b5e52 thread_start + 34

Thread 6:: Qt bearer thread
0 libsystem_kernel.dylib 0x9a47784e __select + 10
1 QtCore 0x02dd9732 qt_safe_select(int, fd_set*, fd_set*, fd_set*, timespec const*) + 546
2 QtCore 0x02dda6ef QEventDispatcherUNIXPrivate::doSelect(QFlags<QEventLoop::ProcessEventsFlag>, timespec*) + 975
3 QtCore 0x02ddb89d QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) + 269
4 QtCore 0x02d85181 QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) + 401
5 QtCore 0x02b9ab7c QThread::exec() + 108
6 QtCore 0x02b9e66c QThreadStorageData::finish(void**) + 3020
7 libsystem_pthread.dylib 0x904b7e13 _pthread_body + 138
8 libsystem_pthread.dylib 0x904b7d89 _pthread_start + 162
9 libsystem_pthread.dylib 0x904b5e52 thread_start + 34

Thread 7:: QProcessManager
0 libsystem_kernel.dylib 0x9a47784e __select + 10
1 QtCore 0x02d2677d QLockFile::unlock() + 2397
2 QtCore 0x02b9e66c QThreadStorageData::finish(void**) + 3020
3 libsystem_pthread.dylib 0x904b7e13 _pthread_body + 138
4 libsystem_pthread.dylib 0x904b7d89 _pthread_start + 162
5 libsystem_pthread.dylib 0x904b5e52 thread_start + 34

Thread 8:: com.apple.CFSocket.private
0 libsystem_kernel.dylib 0x9a47784e __select + 10
1 com.apple.CoreFoundation 0x9326bb3a __CFSocketManager + 906
2 libsystem_pthread.dylib 0x904b7e13 _pthread_body + 138
3 libsystem_pthread.dylib 0x904b7d89 _pthread_start + 162
4 libsystem_pthread.dylib 0x904b5e52 thread_start + 34


What am I doing wrong?

jefftee
18th April 2015, 21:53
What am I doing wrong?
For starters, your QThread instance t will be destroyed right after you do the t.start() because it's allocated on the stack and will go out of scope as sooon as doQuery() returns. Allocate it on the heap (new QThread()) and save a pointer to it for future use or make it a class member variable.

Same for your timer and worker object. Make them member variables or allocate from the heap, etc.

Edit: Not sure why you are trying to move the timer to the thread too. In my experience I have only moved the worker object to the QThread. I would also recommend that you rename your WorkerThread class to WorkerObject as it is misleading to name it as a thread.

Cupidvogel
18th April 2015, 22:11
Cool. That worked. Yes, no need to move the timer, they just gave it by way of an example, in the example following it, they commented that line out.. So along these footsteps, is this design correct for my code? Will this be enough, or am I doing something wrong?



//inside the slot

//get the query term, and pass it to the action, it will update view through signal/slot, so longer need to pass view as param

std::shared_ptr<QueryAction> action = std::make_shared<QueryAction>(queryTerm);

//this in turn will run a while loop polling the database, and finish gracefully once polling is complete
action->execute();

QThread *t = new QThread();
action->moveToThread(t);

anda_skoa
18th April 2015, 22:11
Thanks for clarifying. I will usually register my custom types whether or not I am using threads and cross-thread signals/slots. I believe the other beneficial reason to do so is that you can use QVariant for custom types that have been registered, which may come in handy too.

Right, Q_DECLARE_METATYPE is required for encapsulating the type in QVariant. qRegisterMetaType() is needed, if the QVariant needs to be created without calling fromValue().
Which is the case of a signal/slot connection using Qt::QueuedConnection (such as a cross-thread one).

Cheers,
_

jefftee
18th April 2015, 22:36
//inside the slot

//get the query term, and pass it to the action, it will update view through signal/slot, so longer need to pass view as param

std::shared_ptr<QueryAction> action = std::make_shared<QueryAction>(queryTerm);

//this in turn will run a while loop polling the database, and finish gracefully once polling is complete
action->execute();

QThread *t = new QThread();
action->moveToThread(t);

Not sure what action->execute() is intended to do in line 8. If your intent is to execute something in the new thread, that's not the correct way to accomplish that. For starters, you haven't created the new QThread yet, nor has it been started (QThread::start). Typically you would emit a signal that is connected to a slot in your Worker object, which can be easily done with a QTimer::singleShot.

The pointer on line 10 is a local pointer, so you will lose the ability to address your QThread after your slot returns. Make the pointer a class member variable, which will allow you to use it later on.

You should also make all of your signal/slot connections for the Worker object and QThread after line 11.

Lastly, after you have created the QThread, moved your Worker object to the QThread, and made your signal/slot connections, you need to start the QThread so that it begins running its event loop.

After you have done all of that, then QTimer::singleShot with a 0ms wait to queue the signal to execute in the new thread, etc.

Once you have all of that working, you should add a boolean to your Worker object that you can use to tell it to stop processing. i.e. You have started doing a long running task in the new thread and the user wants to cancel the task or close the program, etc. Your long running task would then check the boolean to determine if stop has been requested, etc. To make it all thread safe, you should protect the boolean member variable with a QMutex and QMutexLocker whenever you read or change the boolean value.

Hope that helps.

Cupidvogel
18th April 2015, 22:40
I don't understand, why do we need a timer here? I though that was an example just for explaining how multi threading works in Qt?

jefftee
18th April 2015, 22:43
Use a QTimer or don't use a QTimer, doesn't matter to me... :)

Cupidvogel
18th April 2015, 23:05
Okay. So here is my revised code. Is this okay?

QueryView.hpp:



QThread *t;


QueryView.cpp:



QueryView::QueryView()
{
connect(SignalClass.get(),SIGNAL(updateViewSignal( QueryTerm&)), this, SLOT(updateView(QueryTerm&)));
connect(queryButton, SIGNAL(clicked()),this, SLOT(doQuery()));
}

void QueryView::doQuery()
{
t = new QThread()
emit fSignalClass->stopExistingQuery();
std::shared_ptr<QueryAction> action = std::make_shared<QueryAction>(queryTerm);
action->moveToThread(t);
t->start();
emit fSignalClass->excuteSignal();

}

QueryView::updateView(QueryTerm& term)
{
...update view with query result
}


QueryAction class:



QueryAction::QueryAction(QueryTerm& queryTerm): fQueryTerm(queryTerm)
{
connect(SignalClass.get(), SIGNAL(excuteSignal()), this, SLOT(startSearch()));
connect(SignalClass.get(), SIGNAL(stopExistingQuery()), this, SLOT(stopQuery()));
continueQuery = true;
}

void QueryAction::startSearch()
{
//init cursor
while (db_cursor != empty)
{
...get query result
if (continueQuery)
emit fSignalClass->updateViewSignal(aResult);
}
}

void QueryAction::stopQuery()
{
continueQuery = false;
}

jefftee
19th April 2015, 00:00
You should protect continueQuery with a QMutex and you should also return from startSearch (or break out of while loop) where you check whether continueQuery is true on line 14.

Other than that, does it work?

Cupidvogel
19th April 2015, 00:31
Okay, so here is my revised QueryAction class:



//QueryAction.hpp:

class QueryAction

{
QMutex m_mutex;
}

//QueryAction.cpp

QueryAction::QueryAction(QueryTerm& queryTerm): fQueryTerm(queryTerm)
{
connect(SignalClass.get(), SIGNAL(excuteSignal()), this, SLOT(startSearch()));
connect(SignalClass.get(), SIGNAL(stopExistingQuery()), this, SLOT(stopQuery()));
continueQuery = true;
}

void QueryAction::startSearch()
{
//init cursor
while (db_cursor != empty)
{
...get query result
QMutexLocker locker(&m_mutex);
if (continueQuery)
emit fSignalClass->updateViewSignal(aResult);
else
break;
}
}

void QueryAction::stopQuery()
{
QMutexLocker locker(&m_mutex);
continueQuery = false;
}


Is this correct logic? I haven't tried it yet, for one, this will need a large amount of code change and I am not ready for it now, and secondly, I want to understand it fully before I get my hands dirty, so trying to get my logic right..

jefftee
19th April 2015, 00:50
Is this correct logic? I haven't tried it yet, for one, this will need a large amount of code change and I am not ready for it now, and secondly, I want to understand it fully before I get my hands dirty, so trying to get my logic right..
It looks correct to me, but as much as I enjoy being your compiler and debugger, the only thing that matters is that the code executes correctly, so you need to set breakpoints, step through your code, etc. to ensure it's working the way you want. :)

Cupidvogel
19th April 2015, 00:51
Yep yep, of course. Will do the entire code and get back to you.. :)

anda_skoa
19th April 2015, 07:17
Okay. So here is my revised code. Is this okay?


connect(SignalClass.get(),SIGNAL(updateViewSignal( QueryTerm&)), this, SLOT(updateView(QueryTerm&)));


If this is the cross-thread connection then a reference is not correct (and it would quite uncommon for any type of connection).


connect(queryAction, SIGNAL(updateViewSignal(QueryTerm)), this, SLOT(updateView(QueryTerm)));






emit fSignalClass->stopExistingQuery();


Don't emit another object's signals. This would not even compile in Qt4.





void QueryView::updateView(QueryTerm& term)


If this is the slot invoked through the cross-thread signal/slot connection then it won't get a reference


void QueryView::updateView(const QueryTerm& term)


Cheers,
_

Cupidvogel
19th April 2015, 09:57
Well, emitting other object's signal does work for me. I ma using Qt 5.3.1, and it works fine like this:



class SignalClass : public QObject
{
Q_OBJECT
public:
SignalClass(QObject *parent = 0) : QObject(parent)
{
}
signals:
void excuteSignal();
void randomSignal(QString);
}


And using it like this:



//somewhere we emit it
std::shared_ptr< SignalClass > fSignalClass = std::make_shared< SignalClass >();
emit fSignalClass->randomSignal(fString);

//and somewhere else connect it to individual slot
connect(SignalClass.get(), SIGNAL(randomSignal(QString)), this, SLOT(captureString()));


I will remove the references from cross thread signal-slot connections. Cheers!

anda_skoa
19th April 2015, 11:36
Well, emitting other object's signal does work for me. I ma using Qt 5.3.1

Yes, in Qt5 the access level for signals had to be made "public" (was "proteced" before) in order to make the compile time connect() work.
Emitting another object's signals is of course still a very bad idea, they used to be protected for a reason.

Especially in the presence of threads, when QObject thread-affinity matters.

Cheers,
_

Cupidvogel
19th April 2015, 11:37
Okay, so what other way can I make it work?

anda_skoa
19th April 2015, 12:39
Okay, so what other way can I make it work?
I am afraid I don't understand the question.

What "other way" do you need for which problem?

Cheers,
_

Cupidvogel
19th April 2015, 12:43
Sorry. What I meant is that what other way can I connect a signal from one object to a slot in another? I mean, I can define a signal in the class that raises it and raise it, and I can define a slot in the receiver object, but how do I connect the two in the receiver object?

anda_skoa
19th April 2015, 13:49
connect() always works the same way:


connect(sender, SIGNAL(...), receiver, SLOT(...));

I.e. the code that runs the connect() needs access to both objects.
Whether none, one or both of these objects is "this" doesn't matter to connect().

Cheers,
_

Cupidvogel
19th April 2015, 14:17
Yeah, that is definitely one way of doing it, but I did not do it because it would introduce cyclic dependency, that is A will include B and B will include A, etc. But I will make it work anyhow. In the mean time, I tried out the entire revised code.

The query results are coming out correctly, I am applying a qDebug() on them when they are generated and they are printed correctly. But the view is not being updated through the signal/slot. I am getting this debugging message:



QObject::connect: Cannot queue arguments of type 'std::shared_ptr<QueryResult>'
(Make sure 'std::shared_ptr< QueryResult >' is registered using qRegisterMetaType().)

//this starts coming from the qDebug() statement...
Query result: context of a process and investigates the
Query result: this proved it as because


How can I make it work?

Cupidvogel
19th April 2015, 16:52
Bad news. To temporarily check whether it actually works, instead of passing QueryResult to the slot in the view, I just passed QString and made the view display it as a QPushButton. It is hanging even more than before I multithreaded it. Previously the app was stalling and freezing, but now it is completely hanging, and not being restored until the querying is complete. I verified, the thread ids for the Action thread and main thread are different.

anda_skoa
19th April 2015, 17:09
Yeah, that is definitely one way of doing it, but I did not do it because it would introduce cyclic dependency, that is A will include B and B will include A, etc.

How does that introduce a dependency of any sort, much less a cyclic one?
Neither receiver nor sender need to know about each other!




QObject::connect: Cannot queue arguments of type 'std::shared_ptr<QueryResult>'
(Make sure 'std::shared_ptr< QueryResult >' is registered using qRegisterMetaType().)


Seems like you have an argument type that you did not register with the Qt type system, as explained earlier by jefftee.
Especially the shared_ptr looks strange given that you should have a "const QueryResult&" as the argument type for both signal and slot.

So make sure that

1) your QueryResult class has been marked with Q_DECLARE_METATYPE
2) your QueryResult class has been registered with qRegisterMetaType()
3) your signal has a signature with either "QueryResult" or "const QueryResult&" as its argument type
4) your slot has a matching signature.

Cheers,
_

Cupidvogel
19th April 2015, 17:22
How does that introduce a dependency of any sort, much less a cyclic one?
Neither receiver nor sender need to know about each other!
_

I have worked around it.


How does that introduce a dependency of any sort, much less a cyclic one?
1) your QueryResult class has been marked with Q_DECLARE_METATYPE
2) your QueryResult class has been registered with qRegisterMetaType()
_

How do I do it? I found an example here: http://stackoverflow.com/questions/14216821/register-a-meta-type-in-qt, is this how it will work?



class QueryResult
{

QString queryString;
QString metadata;
int offset;
}
Q_DECLARE_METATYPE( QueryResult* )


Where do I call the qRegisterMetaType() method on QueryTerm then? And okay, I will change the signature to const QueryAction& instead of std::shared_ptr<QueryAction>.

jefftee
19th April 2015, 17:24
Where do I call the qRegisterMetaType() method on QueryTerm then? And okay, I will change the signature to const QueryAction& instead of std::shared_ptr<QueryAction>.
You can register the meta type at any time, but obviously must be done before you attempt to use it... I typically do that early in the constructor for the main widget.

Cupidvogel
19th April 2015, 17:27
Okay, I wil try to do it. But in the mean time, performance has not only not improved, it has badly worsened. :( I am just passing in a dummy QString to the view instead of the QueryAction object to check whether it actually works..

jefftee
19th April 2015, 17:40
Post your code again please. I have lost track of where your overall code base has morphed since the many posts to this thread have been made. Also, please elaborate on what performance badly worsened means.

Cupidvogel
19th April 2015, 17:50
Sure. It has become slow in the sense that previously, when there was a query term which resulted in a voluminous number of results, say 600~700, the UI used to kind of freeze, scrolling through the list of results (stored as QPushButtons in a QTableWidget) would become slow and dragging. But it worked correctly and more or less well. The loading size also stalled. But now the loading style is freezing completely, the cursor changes to the wheel icon (suggesting that the app has hung), and the results start updating, but soon stop, and finally, after quite some time, the view is updated with the remaining results, and the process completes.

QueryView.hpp:



QThread *t;


QueryView.cpp:



QueryView::QueryView()
{
connect(SignalClass.get(),SIGNAL(updateViewSignal( QueryTerm&)), this, SLOT(updateView(QueryTerm&)));
connect(queryButton, SIGNAL(clicked()),this, SLOT(doQuery()));
}

void QueryView::doQuery()
{
t = new QThread()
emit fSignalClass->stopExistingQuery();
std::shared_ptr<QueryAction> action = std::make_shared<QueryAction>(queryTerm);
action->moveToThread(t);
t->start();
emit fSignalClass->excuteSignal();

}

QueryView::updateView(QString& term)
{
...update view with query result, currently a string for testing purpose
}


QueryAction.hpp:



class QueryAction

{
QMutex m_mutex;
}


QueryAction.cpp:



QueryAction::QueryAction(QueryTerm& queryTerm): fQueryTerm(queryTerm)
{
connect(SignalClass.get(), SIGNAL(excuteSignal()), this, SLOT(startSearch()));
connect(SignalClass.get(), SIGNAL(stopExistingQuery()), this, SLOT(stopQuery()));
continueQuery = true;
}

void QueryAction::startSearch()
{
//init cursor
while (db_cursor != empty)
{
...get query result
QMutexLocker locker(&m_mutex);
if (continueQuery)
emit fSignalClass->updateViewSignal(aResult.asQString());
else
break;
}
}

void QueryAction::stopQuery()
{
QMutexLocker locker(&m_mutex);
continueQuery = false;
}

jefftee
19th April 2015, 18:12
Your use of pointers in the emit'ing of signals for a different class/object seem unnatural to me. Not sure if that's a performance problem or not, but IMHO you should rework your signal/slot usage so that each class emits a signal when it has something that others may be interested in.

So in your example above, updateViewSignal should be a signal defined by QueryAction. All it needs to do when it has new data that anyone may be interested in is:



emit updateViewSignal(aResult.asQString());


The way you have your signals setup now, the object that wants to emit the signal has to have a pointer to the object you want to receive the signal, which totally undoes the abstraction that signals/slots provides.

Cupidvogel
19th April 2015, 18:17
Okay. So suppose the updateViewSignal signal belongs to QueryAction and it emits it like you said. How do the view class listen to it?

jefftee
19th April 2015, 18:20
Same way any other signal/slot connection works. To make any connect you need the address of the sender and receiver objects and the name/signature of the signal and the name/signature of the receiving slot.

Cupidvogel
19th April 2015, 18:30
Okay. Will try this.

Added after 7 minutes:

Will this work?



void QueryView::doQuery()
{
t = new QThread();
std::shared_ptr<QueryAction> action = std::make_shared<QueryAction>(queryTerm);
connect(this, SIGNAL(stopExistingQuery()), action, SLOT(stopQuery());
connect(this, SIGNAL(excuteSignal()), action, SLOT(startSearch());
emit stopExistingQuery();
action->moveToThread(t);
t->start();
emit excuteSignal();
}


Essentially I have moved both signals from SignalClass to the view, and for each instance of the QueryAction class being generated, I am connecting the signal/slots for start and stop querying. Will this be better, even if it works?

anda_skoa
19th April 2015, 22:27
How do I do it? I found an example here: http://stackoverflow.com/questions/14216821/register-a-meta-type-in-qt, is this how it will work?



class QueryResult
{

QString queryString;
QString metadata;
int offset;
}
Q_DECLARE_METATYPE( QueryResult* )


No, the type itself:


Q_DECLARE_METATYPE(QueryResult)




And okay, I will change the signature to const QueryAction& instead of std::shared_ptr<QueryAction>.
You mean QueryResult, right?


Your use of pointers in the emit'ing of signals for a different class/object seem unnatural to me.

Indeed, already pointed that out, only to be told that my advice was worked around :-/

Cheers,
_

Cupidvogel
20th April 2015, 21:46
Sorry guys for the late reply. I did as you said, moved all signals and slots to respective classes instead of relying on Signal Class as I was doing. Still the same performance, app hangs badly, the loading symbol completely freezes, but after some time all query results do show up on the UI. Here is my code:

QueryView.hpp:



class QueryView
{
QThread *t;
QueryAction *action;
signals:
void excuteSignal();
void stopExistingQuery();
}


QueryView.cpp:



QueryView::QueryView()
{
connect(SignalClass.get(),SIGNAL(updateViewSignal( QueryTerm&)), this, SLOT(updateView(QueryTerm&)));
connect(queryButton, SIGNAL(clicked()),this, SLOT(doQuery()));
}

void QueryView::doQuery()
{
t = new QThread()
emit stopExistingQuery();
action = new QueryAction(queryTerm);

connect(this,SIGNAL(excuteSignal()), fSearchAction, SLOT(startQuery()));
connect(this,SIGNAL(stopExistingQuery()), fSearchAction, SLOT(stopQuery()));
connect(fSearchAction, SIGNAL(updateViewSignal(QString)),this, SLOT(updateView(QString)));

action->moveToThread(t);
t->start();
emit excuteSignal();

}

QueryView::updateView(QString& term)
{
...update view with query result, currently a string for testing purpose, as a QPushButton, forming a series of buttons in a QTableWidget
}


QueryAction.hpp:



class QueryAction

{
QMutex m_mutex;

signals:
void updateSearchView2(QString);
}


QueryAction.cpp:



QueryAction::QueryAction(QueryTerm& queryTerm): fQueryTerm(queryTerm)
{
QMutexLocker locker(&m_mutex);
continueQuery = true;
}

void QueryAction::startSearch()
{
//init cursor
while (db_cursor != empty)
{
...get query result
QMutexLocker locker(&m_mutex);
if (continueQuery)
emit updateViewSignal(aResult.asQString());
else
break;
}
}

void QueryAction::stopQuery()
{
QMutexLocker locker(&m_mutex);
continueQuery = false;
}

jefftee
21st April 2015, 00:15
Post a minimal compilable example please so that I can recreate and/or step through your code and see what's going on.

anda_skoa
21st April 2015, 08:06
Here is my code:

No, it is not.
That is the same code snippets with the same problems pointed out several times.

Cheers,
_

Cupidvogel
21st April 2015, 08:17
Umm, like what? You mentioned that the signals and slots should not be from a separate class. I did that. What else needs to be done? I am currently passing a simple QString to display in the UI

anda_skoa
21st April 2015, 11:26
Umm, like what?

You posted this:


QueryView::QueryView()
{
connect(SignalClass.get(),SIGNAL(updateViewSignal( QueryTerm&)), this, SLOT(updateView(QueryTerm&)));
connect(queryButton, SIGNAL(clicked()),this, SLOT(doQuery()));
}

void QueryView::doQuery()
{
t = new QThread()
emit stopExistingQuery();
action = new QueryAction(queryTerm);

connect(this,SIGNAL(excuteSignal()), fSearchAction, SLOT(startQuery()));
connect(this,SIGNAL(stopExistingQuery()), fSearchAction, SLOT(stopQuery()));
connect(fSearchAction, SIGNAL(updateViewSignal(QString)),this, SLOT(updateView(QString)));

action->moveToThread(t);
t->start();
emit excuteSignal();

}

QueryView::updateView(QString& term)
{
...update view with query result, currently a string for testing purpose, as a QPushButton, forming a series of buttons in a QTableWidget
}

And you would like us to believe that is the actual code you are using?
With SignalClass.get(), which you claim have removed, with reference as signal argument types which you have been told won't work, with signal signatures not matching slot signatures, with variable names not matching (action vs. fSearchAction)?

How stupid do you think we are?

Cheers,
_

Cupidvogel
21st April 2015, 12:12
Umm, wait, why would I ask you for help if I considered you stupid? I forgot to remove the SignalClass from that part of the code because I was editing my copy paste from my previous post and overlooked that part. Obviously this is not the exact code I have, there are lots of variable definitions, array populating, my project specific code (which I am forbidden to post for confidentiality reasons) etc which don't belong to the ambit of this problem and thus I did not include here (for starters, my QueryAction header class is 8 lines long, in reality it is around 35~40 lines.) Fo example after the querying is complete I have the action class emit a signal which feeds in to a slot in the view that shows a message like 'Total number of query results is <some_number>', but I did not include it here because it doesn't add anything to the problem scope. I have changed some of my variable names here so that I had to do less typing (for example I renamed fSearchAction to action). It's only after you said that the Signal Class mapper might be causing the problem did I go ahead and change it (while editing I forgot to edit one part). In essence this is the summary of the entire process I am currently running. Why on earth would I not try out what you are suggesting and yet ask you for further suggestions?

If it helps, here is my latest code (statutory disclaimer: this code cannot be and is not considered completely representative of my entire project, not does it purport to be. It's aim is to provide a high level overview of my problem, and is open to request [please] for more information, that will be provided subject to the terms and conditions of the company I am working for in capacity of a software engineer to make a product, with an ardent expectation that it will be sufficient to experienced Qt programmers in this forum to be able to offer advice and suggestion to improve it. Viewers' discretion is advised.)

QueryView.hpp:



class QueryView
{
QThread *t;
QueryAction *action;
signals:
void excuteSignal();
void stopExistingQuery();
}


QueryView.cpp:



QueryView::QueryView()
{
connect(queryButton, SIGNAL(clicked()),this, SLOT(doQuery()));
}

void QueryView::doQuery()
{
t = new QThread()
emit stopExistingQuery();
action = new QueryAction(queryTerm);

connect(this,SIGNAL(excuteSignal()), action, SLOT(startQuery()));
connect(this,SIGNAL(stopExistingQuery()), action, SLOT(stopQuery()));
connect(action, SIGNAL(updateViewSignal(QString)),this, SLOT(updateView(QString)));

action->moveToThread(t);
t->start();
emit excuteSignal();

}

QueryView::updateView(QString term)
{
...update view with query result, currently a string for testing purpose, as a QPushButton, forming a series of buttons in a QTableWidget
}


QueryAction.hpp:



class QueryAction

{
QMutex m_mutex;

signals:
void updateViewSignal(QString);
}


QueryAction.cpp:



QueryAction::QueryAction(QueryTerm& queryTerm): fQueryTerm(queryTerm)
{
QMutexLocker locker(&m_mutex);
continueQuery = true;
}

void QueryAction::startQuery()
{
//init cursor
while (db_cursor != empty)
{
...get query result
QMutexLocker locker(&m_mutex);
if (continueQuery)
emit updateViewSignal(aResult.asQString());
else
break;
}
}

void QueryAction::stopQuery()
{
QMutexLocker locker(&m_mutex);
continueQuery = false;
}