When to delete QThread

rovshanb

Hi,

I have following code in tree model derived from QAbstractItemModel:

@void DbTreeModel::populateChildNodes(const QModelIndex & parent)
{
... do some prosessing here ...

nodePopulatorThread=new NodePopulatorThread(parent, this);
connect(nodePopulatorThread, SIGNAL(queryCompleted(DbTreeItemResult)), this, SLOT(nodeListAvailable(DbTreeItemResult)));
nodePopulatorThread->start();

}@

and nodeListAvailable slot looks like this:

@void DbTreeModel::nodeListAvailable(const DbTreeItemResult &result)
{
nodePopulatorThread->wait();
delete nodePopulatorThread;
nodePopulatorThread=0;

... do processing of result ...

}@

My question is: Is it right to delete thread at the beginning of method, or should I delete it after processing results (because result parameter is created in thread and passed by reference to this slot)? Currently it works OK on Linux, but I think that it can lead to random crashes because data pointed by result parameter may be deleted (when deleting thread) by the time I start to process it. Or am I talking nonsense? ))

Thanks in advance!

koahnig

There a difference in your code. I guess it should not run this way, because the parameter lists are different.

[quote author="rovshanb" date="1310553943"]
@ connect(nodePopulatorThread, SIGNAL(queryCompleted(DbTreeItemResult)), this, SLOT(nodeListAvailable(DbTreeItemResult)));
nodePopulatorThread->start();
}@
[/quote]
and here you have a const reference
[quote author="rovshanb" date="1310553943"]
@
void DbTreeModel::nodeListAvailable(const DbTreeItemResult &result) @
[/quote]

Otherwise you may adapt the parameter list of your slot method. if you are not using a const reference but a copy it should not be a problem.

If you adapt your connect statement to reflect the use as in your slot method, you are correct about worrying about the life time of your result.

changsheng230

Better not delete thread directly, it is very dangerous, use QObject::deleteLater(), which will post an event that will ultimately cause its deletion by the thread the object is living in.

There is a best practice wiki talking about details of good-to-use thread, of course deletion included:
http://developer.qt.nokia.com/wiki/Threads_Events_QObjects

rovshanb

I changed connect method to:

@connect(nodePopulatorThread, SIGNAL(queryCompleted(const DbTreeItemResult &)), this, SLOT(nodeListAvailable(const DbTreeItemResult &)));@

and moved delete statements to end of method.

But the slot was also firing with the above code (Qt Creator auto suggested it).

So when I write @SLOT(nodeListAvailable(DbTreeItemResult))@ and define slot as @DbTreeModel::nodeListAvailable(const DbTreeItemResult &result)@ does it work differently than when specifying const & both in connect and slot?

rovshanb

Ok, thanks for the link, I'll read it now

koahnig

[quote author="rovshanb" date="1310554925"]I changed connect method to:

@connect(nodePopulatorThread, SIGNAL(queryCompleted(const DbTreeItemResult &)), this, SLOT(nodeListAvailable(const DbTreeItemResult &)));@

and moved delete statements to end of method.

But the slot was also firing with the above code (Qt Creator auto suggested it).

So when I write @SLOT(nodeListAvailable(DbTreeItemResult))@ and define slot as @DbTreeModel::nodeListAvailable(const DbTreeItemResult &result)@ does it work differently than when specifying const & both in connect and slot?[/quote]

I do not know. So far I believed that this will result in an error while connecting. Actually it did always give an error message to the console then and returned a false. However, could be just a conclusion on my side. When I did not receive a signal in a slot it was quite often a mismatch of the parameter lists.

rovshanb

Ok, thank you very much for helping!

dakron

You can also make your result item class as implicitly shared object "http://doc.qt.nokia.com/4.7-snapshot/implicit-sharing.html":http://doc.qt.nokia.com/4.7-snapshot/implicit-sharing.html and then you can be sure that you don't 'lose' your results.

koahnig

In case someone has similar doubts as expressed above. There has been another thread started "Signals/Slots behavior review":http://developer.qt.nokia.com/forums/viewthread/7771/ detailing the behaviour and the reasoning behind it.

[quote author="rovshanb" date="1310554925"]
But the slot was also firing with the above code (Qt Creator auto suggested it).
[/quote]

When the auto suggestions are changing parameter lists may be consiedered as a styling issue then.

luca

Hi all,
I post my question in this thread to avoid a creation of a new topic.

I have a QObject class:
@
class GestoreComunicazioneSatellite : public QObject
{
Q_OBJECT
public:
...
...
}

GestoreComunicazioneSatellite::GestoreComunicazioneSatellite() :
QObject(0)
{
...
m_thread=new QThread();
this->moveToThread(m_thread);
...
}
@

The problem is that I get some warning when I destroy the GestoreComunicazioneSatellite object.

I tryed some solution:
@
GestoreComunicazioneSatellite::~GestoreComunicazioneSatellite()
{
m_thread->deleteLater();
}
@
I get:
Warning: --> QThread: Destroyed while thread is still running

@
GestoreComunicazioneSatellite::~GestoreComunicazioneSatellite()
{
m_thread->terminate();
m_thread->wait();
m_thread->deleteLater();
}
@
I get:
Warning: --> QThread::wait: Thread tried to wait on itself

The only way to avoid warnings is:
@
GestoreComunicazioneSatellite::~GestoreComunicazioneSatellite()
{
}
@

but I'm not sure the m_thread is destroyed this way.

What do you think about?

luca

... and what if I try:
@
GestoreComunicazioneSatellite::~GestoreComunicazioneSatellite()
{
m_thread->exit(123);
m_thread->deleteLater();
}
@

?