June 5, 2011

TheDestroyer TheDestroyer
Lab Rat
150 posts

Cleaning up after a thread correcly

Page  
1

Hello guys, :)

I’m tired of trying to learn how to start and delete a thread correctly without residuals or problems. I posted other problems with this program before, and to make the thing easier to solve, please download a copy of the source from the following link.

The program is a down-sampler. It simply reads a few blocks of a file, calculates the average, and writes the result. The purpose is simply making files smaller.

http://www.4shared.com/file/Lh2KqF0Q/DownSampler.html

The problem is, that whenever the start button and stop are clicked a few times, the program freezes. This happens now in Linux, but I don’t know if this happens in windows too. I think it does.

The program is compilable under windows and linux.

Please guide me to the method to fix this problem. I tried calling QThread::quit(), exit(), terminate() with a deleteLater()… but all cause problems… with no exception! please tell me how to do this correctly.

Thank you :-)

41 replies

June 5, 2011

romankr romankr
Lab Rat
16 posts

first of all, in my opinion, using of quit() [doc.qt.nokia.com], exit() [doc.qt.nokia.com], terminate() [doc.qt.nokia.com] for threads is a bad style.
You should design interaction with/within thread to avoid their use by any cost. Terminate() allowed to be only in destructors or functions with the same aim as destructor, where the immediate termination of thread is really necessary and reasonable.
The preferable way – to foresee all quit point for thread and put there only return; statement.

The second part is a code snippet overview

  1. downsamplingthread.cpp
  2.  
  3. this->terminate();
  4. this->exit();
  5. this->quit();
  6. return;

why is there so much thread-stopping functions? It’s quite enough to use one of them. I advice to read manual about each of them.

  1. if(outputTypeString == "int16")
  2.         {
  3.             ...
  4.         }
  5.         else if(outputTypeString == "int32")
  6.         {
  7.             ...
  8.         }
  9.         else if(outputTypeString == "int64")
  10.         {
  11.             ...
  12.         }

use switch ()

TheDestroyer wrote:

The problem is, that whenever the start button and stop are clicked a few times, the program freezes.

you are using new to create new [cplusplus.com] thread and forgot about delete [cplusplus.com] delete/ -> they block each other.

June 5, 2011

TheDestroyer TheDestroyer
Lab Rat
150 posts

Thank you for your reply.

You’re right about the switch thingy. I’ll use it! and about exit and quit, what should I use instead????

but concerning the new and delete commands… whenever I try to delete the thread, the program freezes! have you succeeded with that yourself? because I tried that like 10000 times with different methods with no success, and that’s why I’m asking about a plan to clean up the thread professionally.

So how can I use delete without quit()? please try it and you’ll see how the program freezes!!!!

If you know the way to go around this and use the thread cleanly, please tell me!

June 6, 2011

Franzk Franzk
Lab Rat
834 posts

romankr wrote:
  1. if(outputTypeString == "int16")
  2.         {
  3.             ...
  4.         }
  5.         else if(outputTypeString == "int32")
  6.         {
  7.             ...
  8.         }
  9.         else if(outputTypeString == "int64")
  10.         {
  11.             ...
  12.         }

use switch ()

The switch statement only works on integer constants, not on (c-)strings.

If you know the way to go around this and use the thread cleanly, please tell me!

Make sure you react properly to exiting. Use exit() only if you properly handle that. Don’t use terminate() unless you have a good reason to terminate it.

I would always go with a

  1. connect(myThread, SIGNAL(finished()), myThread, SLOT(deleteLater()));
  2. myThread->stopWorking();

This will delete your thread after it has finished its work. Only thing to do now is to make sure your main thread stays alive long enough to delete the thread object.

 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

TheDestroyer TheDestroyer
Lab Rat
150 posts

Thank you for the answers guys. Sorry for the late reply, but my laptop got a problem and I had to format it!

What do you mean with properly handle exiting? In the program, when the user clicks start, the thread is created, and the error could be issued in the thread immediately if the files don’t exist, or if the user clicks the Stop button! So how could I handle exit() correctly here? is it just by issuing exit() on error? cuz this is what I’m doing. If the user clicks start again, a new thread is created and the old one is “lost” in memory… The problem is, as far as I see it, is that deleting threads take relatively long time. Isn’t that true?

so I can’t tell the user “please wait till the thread is deleted”.

I’m thinking that since deleting threads takes long time, would it be a feasible solution to use a thread pool, in a way that whenever a thread fails, it’s ignored by the program and handled by the pool for deletion, so that I could take another new thread from the pool when the user clicks Start again?

June 7, 2011

Franzk Franzk
Lab Rat
834 posts

You could try and see if QtConcurrent helps you out.

 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
Robot Herder
3253 posts
TheDestroyer wrote:
If the user clicks start again, a new thread is created and the old one is “lost” in memory… The problem is, as far as I see it, is that deleting threads take relatively long time. Isn’t that true?

please define relatively long? The user recognizes it? Then it’s a bug in your code. You could have something like:

  1. MyThread::run()
  2. {
  3.     // do something
  4.     if(m_bStopThread)
  5.         return;
  6.     // do something
  7.     if(m_bStopThread)
  8.         return;
  9.     // do something
  10.     if(m_bStopThread)
  11.         return;
  12.     // do something
  13.     if(m_bStopThread)
  14.         return;
  15. }
  16.  
  17. MyThreda::StopThread()
  18. {
  19.     m_bStopThread = true;
  20. }

 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

TheDestroyer TheDestroyer
Lab Rat
150 posts

Thank you for your answers, guys!

@Franzk: QtConcurrent is very limited. It doesn’t help my general purposes. I’m more interested in learning QThread.

@Gerolf: So interestingly, Gerolf, I have the same idea in the code of my first post. Please check the program of my first post. It’s a very small program. You can see that I try to use this “stop” idea exactly the same way you mentioned it, but I can’t find the place where I have to place the delete thread statement. Whenever I introduce a thread deletion, the program freezes!!!! please take a look at the program code in my first post!

June 7, 2011

Gerolf Gerolf
Robot Herder
3253 posts

You can use it like this:

  1. MyMainWnd::beginWork()
  2. {
  3.     pThread = new MyThread();
  4.     pThread->start();
  5. }
  6.  
  7. MyMainWnd::stop()
  8. {
  9.     pThread->StopThread()
  10.     pThread->wait();
  11.     delete pThread;
  12.     pThread = 0;
  13. }

To use this, you have to ensure, that the stopped checks, will not have a too long time in between…

 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

TheDestroyer TheDestroyer
Lab Rat
150 posts

Thank you for the reply!

I’ll try this and tell you how it goes! I tried the command QThread::wait() before, but caused freezing as well. So I’ll try it again and come with detailed description of what I get :)

June 7, 2011

Andre Andre
Robot Herder
6395 posts

It will cause freezing, if the thread doesn’t respond quickly to being told to stop.

June 7, 2011

Franzk Franzk
Lab Rat
834 posts

Use the deleteLater() approach. That won’t cause you to wait for the thread to be finished. On the other hand, maybe you can come up with a way that will allow you to keep the thread around and not recreate it every time.

 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

Andre Andre
Robot Herder
6395 posts

I would not call deleteLater on a QThread that is still running. Sounds like a recipe for all kinds of funny things to happen when you don’t expect it.

Edit:
What might be a good approach, is something like this:

  1. connect(myThread, SIGNAL(finished()), myThread, SLOT(deleteLater()));
  2. myThread->stopThisThread();
  3. myThread = 0;

June 7, 2011

Franzk Franzk
Lab Rat
834 posts
Andre wrote:
I would not call deleteLater on a QThread that is still running. Sounds like a recipe for all kinds of funny things to happen when you don’t expect it.

Neither would I. I was referring to what I’ve stated earlier:

  1. connect(myThread, SIGNAL(finished()), myThread, SLOT(deleteLater()));

Edit: My point exactly :P

 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

Andre Andre
Robot Herder
6395 posts

Ah, sorry, missed your previous suggestion for the same idea. I would put the connect call before the stop call though. Otherwise, you might get into a race condition where the thread is already stopped before the connection has been made. Then again, you should probably make the connection at creation time for the thread anyway, not when you want to stop it.

June 7, 2011

Gerolf Gerolf
Robot Herder
3253 posts

Andre wrote:
I would not call deleteLater on a QThread that is still running. Sounds like a recipe for all kinds of funny things to happen when you don’t expect it.

Edit:
What might be a good approach, is something like this:

  1. connect(myThread, SIGNAL(finished()), myThread, SLOT(deleteLater()));
  2. myThread->stopThisThread();
  3. myThread = 0;

This can also lead to unexpected behavior. You call delete in the main thread when the event loop comes to this point. While it reaches this, the thread could be suspended (depends on timing) or be finished, even if the emit is the last command in the run method.

EDIT:

So you would need a wait in the destructor of the thread….

 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)

Page  
1

  ‹‹ 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!