views:

721

answers:

8

Currently i am working on a desktop application which consists mathematical analysiss.I am using qt for GUI and project written in c++. When user starts an analysis, i open a worker thread and start a progress bar.Everything is ok up to now, problem starts when user cancels operation.Operation is complex, i am using several functions and objects, i allocate/deallocate memory at several times.I want to learn what should i do for recovering in cancel operation.Because there can be memory leaks.Which pattern or method i should use to be robust and safe for cancelling operation?

My idea is throwing an exception, but operation is really complex so should i put try-catch to all of my functions or is there a more generic way, pattern..

Edit: Problem is my objects are transfered between scopes, so shared_ptr or auto_ptr doesnt solve my problem, Flag idea can be, but i think it requires so much code and there should be an easy way.

+8  A: 

A pretty common way to close down worker threads, is to mark it with a flag, and let the worker thread inspect this flag at regular intervals. If marked, it should discontinue it's workflow, clean up and exit.

Is that a possibility in your situation?

Simon Jensen
A: 

You should try to hold dynamically allocated resources in automatic (locals that live on the stack) sentry objects which free those resources in their destructors when they go out of scope. This way you can know they aren't going to leak, even if a function exits because of an exception. You also might want to investigate the boost libraries shared_ptr for sharing memory between routines.

Rob K
+4  A: 

The worker thread should check for a message to stop. the message can be through a flag or an event. when stop message received the thread should exit.

USE BOOST safe pointers for all memory allocated. on exit you would have no memory leaks. ever.

Raz
A: 

First, throwing exceptions multi-threaded applications is iffy because there isn't a standard way to handle them (do they propagate to other threads? the scheduler? main()? somewhere else?). At least until you get a C++0x library which has standardized threading built in.

For the time being it makes more sense to use RAII (which will guarantee that all resources -- including memory -- is cleaned up when the scope exits, whether it exists due to success or failure) and have some sort of status code passed back to whichever thread makes the most sense (the scheduler for instance).

Also, directly canceling threads has been discouraged for more than a decade. It's much better to tell a thread to stop itself and have the thread handle the clean up, as Simon Jensen suggests.

Max Lybbert
Just to make it explicit: the *reason* directly canceling threads is discouraged is because on almost all platforms/environments, this will cause destructors never to be called, meaning RAII simply won't work.
j_random_hacker
Well, it also discouraged because partway through a computation some data has been changed but other data hasn't. If that data is _not_ _supposed_ to be cleaned up (that is, if it's visible outside the thread), you'll have a big problem if the thread is killed without a chance to make things right.
Max Lybbert
And on some platforms locks aren't necessarily dropped when a thread is terminated.
Max Lybbert
A: 
A: 

The answer is that it depends on the complexity of your operation.

There's a few approaches here. 1) as was mentioned, put a 'cancel' flag in the operation, and have that operation poll the cancel flag at regular (close) intervals, probably at least as often as you update your progress bar. When the user hits cancel, then hit the cancel routine.

Now, as for memory handling in this scenario, I've done it a couple of ways. My preference is to use smart pointers or STL objects that will clean themselves up when you go out of scope. Basically, declare your objects inside of an object that has a destructor that will handle memory cleanup for you; as you create these objects, memory is created for you, and as the object goes out of scope, the memory is removed automatically. You can also add something like a 'dispose' method to handle the memory. It could look like this:

class MySmartPointer { 
     Object* MyObject;
     MySmartPointer() { MyObject = new Object(); }
     ~MySmartPointer() { if (MyObject != null) { delete MyObject; MyObject = null; }}
     void Dispose() { if (MyObject != null) { delete MyObject; MyObject = null; } }
     Object* Access() { return MyObject; }
 }

If you want to get really clever, you can template that class to be generic for any of your objects, or even to have arrays and the suchlike. Of course, you might have to check to see if an object has been disposed prior to accessing it, but thems the breaks when you use pointers directly. You might also be able to inline the Access method, so that it doesn't cost you a function call during execution.

2) A goto method. Declare your memory at the front, delete at the end, and when you hit the cancel method, call goto to go to the end of the method. I think certain programmers might lynch you for this, as goto is considered extremely bad style. Since I learned on basic and 'goto 10' as a way for looping, it doesn't freak me out as much, but you might have to answer to a pedant during a code review, so you'd better have a really good explanation for why you went with this one and not option 1.

3) Put it all into a process, rather than a thread. If you can, serialize all of the information to manipulate to disk, and then run your complicated analysis in another program. If that program dies, so be it, it doesn't destroy your main application, and if your analysis is that complicated and on a 32 bit machine, you may need all of that memory space to run anyway. Rather than using shared memory for passing progress information, you just read/write progress to disk, and canceling is instant. It's a bit trickier to implement, but not impossible, and potentially far more stable.

mmr
+2  A: 

Be sure your allocated memory is owned

Be sure every allocated memory is owned by a smart pointer, either STL's auto_ptr or Boost's scoped_ptr, or even shared_ptr (which can be shared, copied and moved).

This way, RAII will protect you from any memory leak.

Use Boost.Thread 1.37

Read Interrupt Politely, an article from Herb Sutter explaining miscellaneous ways to interrupt a thread.

Today wth Boost.Thread 1.37, You can ask a thread to terminate by throwing an exception. In Boost, it's the boost::thread_interrupted exception, which will throw an exception from any interruption point.

Thus, you do not need to handle some kind of message loop, or verify some global/shared data. The main thread asks the worker thread to stop through an exception, and as soon as the worker thread reaches an interruption point, the exception is thrown. The RAII mecanism described earlier will make sure all your allocated data will be freed correctly.

Let's say you have some pseudo code that will be called in a thread. It could be something like a function that will perhaps allocated memory, and another that will do a lot of computation inside a loop:

Object * allocateSomeObject()
{
   Object * o = NULL ;

   if(/*something*/)
   {
      // Etc.
      o = new Object() ;
      // Etc.
   }

   return o ; // o can be NULL
}

void doSomethingLengthy()
{
   for(int i = 0; i < 1000; ++i)
   {
      // Etc.
      for(int j = 0; j < 1000; ++j)
      {
         // Etc.
         // transfert of ownership
         Object * o = allocateSomeObject() ;
         // Etc.
         delete o ;
      }
      // Etc.
   }
}

The code above is not safe and will leak not matter the interruption mode if steps are not taken to be sure that at all moments the memory will be owned by a C++ object (usually, a smart pointer).

It could be modified this way to have the code be both interruptible, and memory safe:

boost::shared_ptr<Object> allocateSomeObject()
{
   boost::shared_ptr<Object> o ;

   if(/*something*/)
   {
      // Etc.
      boost::this_thread::interruption_point() ;
      // Etc.
      o = new Object() ;
      // Etc.
      boost::this_thread::interruption_point() ;
      // Etc.
   }

   return o ; // o can be "NULL"
}

void doSomethingLengthy()
{
   for(int i = 0; i < 1000; ++i)
   {
      // Etc.
      for(int j = 0; j < 1000; ++j)
      {
         // Etc.
         // transfert of ownership
         boost::shared_ptr<Object> o = allocateSomeObject() ;
         // Etc.
         boost::this_thread::interruption_point() ;
         // Etc.
      }

      // Etc.
      boost::this_thread::interruption_point() ;
      // Etc.
   }
}

void mainThread(boost::thread & worker_thread)
{
   // etc.
   if(/* some condition */)
   {
      worker_thread.interrupt() ;
   }
}

Do not use Boost?

If you do not use Boost, then you can simulate this. Have some thread storage boolean-like variable set to "true" if the thread should be interrupted. Add functions checking this variable, and then throw a specific exception if true. Have the "root" of your thread catch this exception to have it end correctly.

Disclaimer

I don't have access to Boost 1.37 right now, so I'm unable to test the previous code, but the idea is there. I will test this as soon as possible, and eventually post a more complete/correct/compilable code.

paercebal
A: 

Since you are using Qt, you can take advantage of QObject's parenting memory system.

You say that you allocate and deallocate memory during the run of your worker thread. If each allocation is an instance of QObject, why don't you just parent it to the current QThread Object?

MyObject * obj = new MyObject( QThread::currentThread() );

You can delete them as you go and that's fine, but if you miss some, they will be cleaned up when the QThread is deallocated.

Note that when you tell your worker QThread to cancel, you must then wait for it to finish before you delete the QThread instance.

workerThread->cancel();  // reqfuest that it stop
workerThread->wait();    // wait for it to complete
delete workerThread;     // deletes all child objects as well

I you use Simon Jensen's answer to quit your thread and QObject as your memory strategy, I think you'll be in a good situation.

Michael Bishop