Cleaning up after a thread correcly

Page  
2

June 7, 2011

Andre Andre
Area 51 Engineer
6076 posts

Gerolf, you mean that QThread::finished() may be emitted before the thread is actually finished? That would be a bug, would it not?

 Signature 

Looking for Qt developers to join our team @ i-Optics: https://qt-project.org/forums/viewthread/25393/

June 7, 2011

Franzk Franzk
Lab Rat
830 posts

From reading the QThread sources it seems on symbian, the finished() signal is emitted from the threaded function (after run() returns). On unix, the signal is emitted in the pthread cleanup handler. Didn’t check the windows code. It seems Gerolf could have a point here. However, if that is the case, the only way to check if the thread is actually still running is by using some pthread_kill() magic.

 Signature 

“Horse sense is the thing a horse has which keeps it from betting on people.”—W.C. Fields

http://www.catb.org/~esr/faqs/smart-questions.html

June 7, 2011

Gerolf Gerolf
Area 51 Engineer
3211 posts
Andre wrote:
Gerolf, you mean that QThread::finished() may be emitted before the thread is actually finished? That would be a bug, would it not?

The point is: when is a thread finished?

To emit a signal, you need code. This code is somewhere :-) If it is inside the threads function (even it is outside run()) you might get in timing problems.

 Signature 

Nokia Certified Qt Specialist.
Programming Is Like Sex: One mistake and you have to support it for the rest of your life. (Michael Sinz)

June 7, 2011

Andre Andre
Area 51 Engineer
6076 posts

I get that, and judging by Franzks analysis, you have a point at least in the Symbian case. It seems logical to me that Qts implementation would make sure the thread really has terminated before the signal is emitted. I understand that you need code to do the emit, but that code does not belong in the thread who’s termination it is supposed to signal. That would mean that the Symbian implementation is wrong, but I have no idea if it is possible to do it the right way on that platform.

 Signature 

Looking for Qt developers to join our team @ i-Optics: https://qt-project.org/forums/viewthread/25393/

June 7, 2011

Gerolf Gerolf
Area 51 Engineer
3211 posts

I even don’t know if that is possible on windows.
But the Qt code does not guarantee that.
This is an excerpt of QThread for windows:

  1. unsigned int __stdcall QThreadPrivate::start(void *arg)
  2. {
  3.     // some initialization
  4.  
  5.     emit thr->started();
  6.  
  7.     thr->run();
  8.  
  9.     finish(arg);
  10.     return 0;
  11. }
  12.  
  13. void QThreadPrivate::finish(void *arg, bool lockAnyway)
  14. {
  15.     // some other stuff
  16.     emit thr->finished();
  17.     // some other stuff
  18. }
  19.  
  20. void QThread::start(Priority priority)
  21. {
  22.     // some other stuff
  23.     d->handle = (Qt::HANDLE) _beginthreadex(NULL, d->stackSize, QThreadPrivate::start,
  24.                                             this, CREATE_SUSPENDED, &(d->id));
  25.  
  26.     // some other stuff
  27. }

This means, the thread function QThreadPrivate::start defines the thread. When it is finished, the thread dies. Within this method, the finished signal is emitted and it is not at the very end of the thread, but guaranteed after the run method….

I’m not sure, whether there is a thread stopped signal on windows…

 Signature 

Nokia Certified Qt Specialist.
Programming Is Like Sex: One mistake and you have to support it for the rest of your life. (Michael Sinz)

June 8, 2011

Franzk Franzk
Lab Rat
830 posts

Apparently the windows thread performs exactly the same action as the symbian implementation. The linux implementation does pretty much the same when no pthread_cancel() is used. The cleanup function is called by a pthread_cleanup_pop(1); This is likely to happen within the thread though. Anyway, this being known, the solution would rather go towards

  1. connect(t, SIGNAL(finished()), this, SLOT(cleanupThread()));
  2. ...
  3. void This::cleanupThread()
  4. {
  5.     thread->wait();
  6.     thread->deleteLater();
  7. }

provided of course that wait() is guaranteed to wait until the OS thread is actually finished. Details are different, but the general idea is still the same.

 Signature 

“Horse sense is the thing a horse has which keeps it from betting on people.”—W.C. Fields

http://www.catb.org/~esr/faqs/smart-questions.html

June 8, 2011

Gerolf Gerolf
Area 51 Engineer
3211 posts

This should work.
wait uses the variables that are set by the finished method before the signal finished is emitted, but there is a mutex around both functions. So wait should return when the thread is really finished.

 Signature 

Nokia Certified Qt Specialist.
Programming Is Like Sex: One mistake and you have to support it for the rest of your life. (Michael Sinz)

June 8, 2011

Andre Andre
Area 51 Engineer
6076 posts

I still think it is a bug in Qt then. Qt could use the same trick itself to make sure finished() is emitted after the thread has really terminated, could it not?

 Signature 

Looking for Qt developers to join our team @ i-Optics: https://qt-project.org/forums/viewthread/25393/

June 8, 2011

Gerolf Gerolf
Area 51 Engineer
3211 posts

I can’t. Otherwise it would need another thread to wait for this one :-)

It can’t use the main thread, or has to send an internal event to the QThread object which then would have to block the QThreads creator thread untill the real thread is destroyed and then emit the signal…. not very nice….

but just calling wait in the slot for the finished signal would do the trick.

 Signature 

Nokia Certified Qt Specialist.
Programming Is Like Sex: One mistake and you have to support it for the rest of your life. (Michael Sinz)

June 8, 2011

Andre Andre
Area 51 Engineer
6076 posts

The wait time would be very short, if there at all. After all, the thread is supposed to be done, right? Now, you have to wait in the main thread anyway. Why not give that task to QThread instead? The real thread could (in absense of a signal from the OS), as it’s last act, indeed just send an event to the QThread and then terminate. The QThread object could in its handler for that event wait for the thread to be finished, and only then emit the signal. I like it more than having to code constructions like these:

  1. connect(t, SIGNAL(finished()), this, SLOT(cleanupThread()));
  2. ...
  3. void This::cleanupThread()
  4. {
  5.     thread->wait();
  6.     thread->deleteLater();
  7. }

You have gained nothing (still waiting for the thread) and you have to write additional code.

 Signature 

Looking for Qt developers to join our team @ i-Optics: https://qt-project.org/forums/viewthread/25393/

June 8, 2011

Franzk Franzk
Lab Rat
830 posts

Andre wrote:
You have gained nothing (still waiting for the thread) and you have to write additional code.

That is not entirely true. What you have gained is a non-blocking solution to the thread cleanup, because the thread is cleaned up at a point where you know for sure that QThread::wait() isn’t going to last for more than a few milliseconds.

 Signature 

“Horse sense is the thing a horse has which keeps it from betting on people.”—W.C. Fields

http://www.catb.org/~esr/faqs/smart-questions.html

June 8, 2011

Andre Andre
Area 51 Engineer
6076 posts

Considder this class then:

  1. FixedQThread: public QThread
  2. {
  3. Q_OBJECT
  4.  
  5. public:
  6.  
  7.     FixedQThread(QObject* parent = 0)
  8.     : QThread(parent) {
  9.         connect(this, SIGNAL(finished()), SLOT(waitForReallyFinished()));
  10.     }
  11.  
  12. signals:
  13.     void reallyFinished();
  14.  
  15. private slots:
  16.     void waitForReallyFinished()
  17.     {
  18.         wait();
  19.         emit reallyFinished();
  20.     }
  21. };

After which you can do:

  1. //thread of type FixedQThread now:
  2. connect(thread, SIGNAL(reallyFinished()), thread, SLOT(deleteLater()));

Right?

 Signature 

Looking for Qt developers to join our team @ i-Optics: https://qt-project.org/forums/viewthread/25393/

June 8, 2011

Franzk Franzk
Lab Rat
830 posts

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.

 Signature 

“Horse sense is the thing a horse has which keeps it from betting on people.”—W.C. Fields

http://www.catb.org/~esr/faqs/smart-questions.html

June 8, 2011

Andre Andre
Area 51 Engineer
6076 posts

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

 Signature 

Looking for Qt developers to join our team @ i-Optics: https://qt-project.org/forums/viewthread/25393/

June 8, 2011

Franzk Franzk
Lab Rat
830 posts

That would be too obvious, wouldn’t it?

 Signature 

“Horse sense is the thing a horse has which keeps it from betting on people.”—W.C. Fields

http://www.catb.org/~esr/faqs/smart-questions.html

Page  
2

  ‹‹ QGraphicsItem - Bounding rect for item, and all children? [solved]      [SOLVED] Compiling with qmake ››

You must log in to post a reply. Not a member yet? Register here!