QueuedConnection's and threads

[gri]

Hello there!

In my code I'm using non-threadsafe classes from our company. For that I've splitted my application into two threads. The main thread runs the GUI, the other one the complete non-threadsafe process. Both communicate only via QueuedConnection's. The non-gui thread has a private class (created in thread::run()) which does all the work for it and notifies the thread object about everything that happens. To forward these notifies, the private and the working-thread class have the same signals and slots. These are simply connected to each other which may cause my problem: Deadlock :(


void thread::run()
{
// Delete the old object on restart
if ( m_private )
delete m_private;

// Create our private object
m_private = new thread_private;

// And connect its signals to ours
connect( m_private, SIGNAL(databaseOpened(bool)), this, SIGNAL(databaseOpened(bool)) );
connect( m_private, SIGNAL(databaseOpening()) , this, SIGNAL(databaseOpening()) );
}

The main window is connected on databaseOpened() etc. on the thread object.

Everything works except in one case: I'm starting a search from the database and get 200 results returned. For each result an event is fired (private (queued)-> thread (direct)-> mainwindow). There happens the deadlock and I can't explain why :(

[jpn]

What kind of class is "thread_private"? Are you not running an event loop?

[gri]

What kind of class is "thread_private"? Are you not running an event loop?

thread_private is derived from QObject. The thread::run() function starts its own event loop with QThread::exec(). The working thread deadlocks in QCoreApplication::postEvent() while the other one (main) hangs in QWaitConditionPrivate::wait() coming from some scary stack (__fstat()) - maybe cause the debugger telled that it can not pause while deadlocking.

[jpn]

The thread::run() function starts its own event loop with QThread::exec().
Hmm, where is it actually called because I don't see it there?

[gri]

Hmm, where is it actually called because I don't see it there?

I've just shortened down the code for the post. Actually there are more signals. The one causing the deadlock works with own created types which are registered via Q_DECLARE_METATYPE() and qRegisterMetaType<x>().


void thread::safeStart()
{
// Invoke the "thread" method only
this->run();
}

void thread::run()
{
// Delete the old object on restart
if ( m_private )
delete m_private;

// Create our private object
m_private = new thread_private;

// And connect its signals to ours
connect( m_private, SIGNAL(databaseOpened(bool)), this, SIGNAL(databaseOpened(bool)) );
connect( m_private, SIGNAL(databaseOpening()) , this, SIGNAL(databaseOpening()) );
connect( m_private, SIGNAL(databaseClosed()) , this, SIGNAL(databaseClosed()) );

connect( m_private, SIGNAL(searchPreparing()) , this, SIGNAL(searchPreparing()) );
connect( m_private, SIGNAL(searchPrepared()) , this, SIGNAL(searchPrepared()) );
connect( m_private, SIGNAL(searchStarted()) , this, SIGNAL(searchStarted()) );
connect( m_private, SIGNAL(searchProgress(double)) , this, SIGNAL(searchProgress(double)) );
connect( m_private, SIGNAL(searchResultsCount(int)), this, SIGNAL(searchResultsCount(int)) );
connect( m_private, SIGNAL(searchResultEntry(const SearchNode&,const QList<SearchAlgo>&,int,double)),
this, SIGNAL(searchResultEntry(const SearchNode&,const QList<SearchAlgo>&,int,double)) );
connect( m_private, SIGNAL(searchFinished()) , this, SIGNAL(searchFinished()) );

connect( m_private, SIGNAL(errorHappened(const QString&)), this, SIGNAL(errorHappened(const QString&)) );

// Start the event loop if we are our own thread
if ( this->isRunning() )
{
// Start the event loop
this->exec();
}
else // Fake the start (in safe mode without threading)
emit started();
}

[jpn]

void thread::safeStart()
{
// Invoke the "thread" method only
this->run();
}

Hmm, where does this thread::safeStart() get called from? You must start the thread by calling QThread::start(), not by QThread::run(). QThread::start() does the work under hood and actually starts a new thread and calls QThread::run() in the newly created thread. QThread::start() is safe to call many times, it does nothing if the thread is already running. You should never invoke QThread::run() by yourself..

[gri]

Hmm, where does this thread::safeStart() get called from? You must start the thread by calling QThread::start(), not by QThread::run(). QThread::start() does the work under hood and actually starts a new thread and calls QThread::run() in the newly created thread. QThread::start() is safe to call many times, it does nothing if the thread is already running.

This function is not called. It's only for a safe mode because some other people here fear that threading could crash the program ... the thread is started with QThread::start(). With the "safe" method no deadlock occurs ..

[jpn]

"thread_private* m_private" could be allocated on the stack. QThread::exec() blocks anyway. Maybe that's causing the problem if the thread is started/stopped multiple times and the thread_private has been allocated in the "previous" thread?

[gri]

"thread_private* m_private" could be allocated on the stack. QThread::exec() blocks anyway. Maybe that's causing the problem if the thread is started/stopped multiple times and the thread_private has been allocated in the "previous" thread?

Also not that :( The thread is only started once in the constructor of the main window and quit()'s and wait()'s until termination in its destructor. All signals except of searchResultEntry() work. The deadlock happens while the private object (running in working thread) sends a event to the thread object (running in main thread).

When setting a breakpoint in the receiving slot no deadlock occurs. So it seems to be that there are too much events fired I think.

[jpn]

Have you tried checking QThread::currentThread() in relevant slots to assure they actually are executed in the expected thread context?

[gri]

Have you tried checking QThread::currentThread() in relevant slots to assure they actually are executed in the expected thread context?

The slot is invoked Queued (coming from QEventLoop::processEvents()). QThread::currentThread() returns the main/GUI thread which is the one, where it should be.

Maybe it's the use of custom objects via const reference (usally gets copied), I'll try to use pointers and see, what happens.

[gri]

Ok, error found.

If I send my own class via QueuedConnection the event loop deadlocks after 2 - 3 calls. Without my own class (using Qt classes like QString etc.) the same call works!

So what am I doing wrong? Code of custom node:


class SearchNode : public QObject
{
Q_OBJECT

public:
SearchNode(QObject* parent = 0);
SearchNode(const QString& name, const QString& relativePath, QObject* parent = 0);
SearchNode(const SearchNode& cpy);
~SearchNode();

SearchNode& operator=(const SearchNode& cpy);

QString relativePath() const;
void setRelativePath(const QString& d);

QString name() const;
void setName(const QString& d);

protected:
QString m_relativePath;
QString m_name;
};

Q_DECLARE_METATYPE(SearchNode);


int main(int argc, char* argv[])
{
qRegisterMetaType<SearchNode>();
...
}

[jpn]

Oh, SearchNode is a QObject. That must be causing the problem. How did you implement the copy constructor? QObject's cannot be copied.

EDIT: Do the SearchNodes have some hierarchy or what's the reason for choosing QObject? Try looking for some alternative way of constructing the hierarchy for example by simply using Qt's containers.

[gri]

Oh, SearchNode is a QObject. That must be causing the problem. How did you implement the copy constructor? QObject's cannot be copied.

Ohhh, you're right I think. Did'nt knew that QObject can not be copied. Altough here is my copy constructor which does not copy anything of QObject.


SearchNode::SearchNode(const SearchNode& cpy)
{
*this = cpy;
}

SearchNode& SearchNode::operator=(const SearchNode& cpy)
{
this->m_relativePath = cpy.m_relativePath;
this->m_name = cpy.m_name;

return *this;
}

[wysota]

Altough here is my copy constructor which does not copy anything of QObject.


SearchNode::SearchNode(const SearchNode& cpy)
{
*this = cpy;
}
Of course it does - QObject has some internal data which can't be shared. And by the way, there is no point implementing such a copy constructor - the compiler would generate such itself. You should only implement a copy constructor if you need something different then simple substitution of all members of one objects with values from the other object.