Cleaning up after a thread correcly

[andre]

Considder this class then:

@
FixedQThread: public QThread
{
Q_OBJECT

public:

FixedQThread(QObject* parent = 0)
: QThread(parent) {
    connect(this, SIGNAL(finished()), SLOT(waitForReallyFinished()));
}

signals:
void reallyFinished();

private slots:
void waitForReallyFinished()
{
wait();
emit reallyFinished();
}
};
@

After which you can do:
@
//thread of type FixedQThread now:
connect(thread, SIGNAL(reallyFinished()), thread, SLOT(deleteLater()));
@

Right?

[Franzk]

Aye, that ought to work. It's basically the approach mentioned earlier, just wrapped in a class, which is a good thing to do anyway.

[andre]

So... Why could that same approach not be inside QThread instead?

[Franzk]

That would be too obvious, wouldn't it?

[andre]

Which I why I stated that I think that QThread should be fixed to always emit the finished signal after the thread has actually finished. It is not like it would be very hard to do, as demonstrated above...

Edit:
Actually, I guess even wait is not going to save the day. See "this bug report":http://bugreports.qt.nokia.com/browse/QTBUG-684 . I have filed a "bugreport":http://bugreports.qt.nokia.com/browse/QTBUG-19783 on the issue.

[TheDestroyer]

Thanks for the nice discussion guys. I think now I could implement the approach as best as possible. I'm gonna come back to tell you how it went :-)

[giesbert]

I think it should work, as QThread::wait has a mutex inside, same as finished. In finished it depends on lockAnyway, whether it is locked or not. During normal thread exit, lockAnyway is true...

@
bool QThread::wait(unsigned long time)
{
Q_D(QThread);
QMutexLocker locker(&d->mutex);

// do the rest

}

void QThreadPrivate::finish(void *arg, bool lockAnyway)
{
QThread *thr = reinterpret_cast<QThread *>(arg);
QThreadPrivate *d = thr->d_func();

if (lockAnyway)
    d->mutex.lock();

// do the rest

if (lockAnyway)
    d->mutex.unlock();

}

@

[TheDestroyer]

Andre's last approach with "reallyFinished()" signal and slot worked perfectly! I like that approach because it's very clear and handles the problem professionally.

Thanks to all of you guys! I suppose problem solved :D

This was tested on Windows. I hope linux won't make nasty problems :D

[DeVeL-2010]

It's always fascinating to see how complicated threads are. ;-)

But one more question:
Can or will the method FixedQThread::waitForReallyFinished() from Andre block the main thread? Because the instance of FixedQThread will live in the context of the main thread normally, just the run() method is executed in another context.

[TheDestroyer]

Actually I tried it in my down-sampler program, and the progress bar in the main window was active with no problems.

The process of being finished or not will only be emitted after run() is done. The problem was, as far as I understood it, the accurate timing in firing the right finished signal, which happens normally (when not using this fix) after run() and before the end of call start().

So the objective in all this, is to wait for start() to return.

[Franzk]

[quote author="DeVeL-2010" date="1307617297"]
Can or will the method FixedQThread::waitForReallyFinished() from Andre block the main thread?[/quote]
Absolutely. The idea here however is to try and make the wait in the main thread be as short as possible.

[andre]

Indeed, but it should only block for a very small time: the time it takes to switch to the almost finished thread to let it terminate, via perhaps some other running threads, and back to the thread where the FixedQThread object lives. Most of the time, it should not block at all, because the thread would already have terminated properly. That is, if all goes well and it works as I expect it to. Note that I wrote that example only to illustrate a point on that this should be fixed in QThread. I don't give any guarantees that it will work correctly 100% of the time in the real world :-)

[ogoffart]

Notice that from Qt 4.8 it is no longer required to wait() in the destructor of the thread if you want to connect the finished() signal to the deleteLater() slot

(this was fixed by commit 25c9b6ed)

[andre]

Ah, nice!
Thanks for reporting back in this topic. So my FixedThread hack is not needed anymore from Qt 4.8?

[Franzk]

Looks like: https://qt.gitorious.org/qt/qt/commit/25c9b6ed