"Correct" Workerthread Termination

"Correct" Workerthread Termination

• DeviPriya

  My message got nuked due to Mozilla weirdness at school, so here goes again.  I've done something similar to what you have, and I can share it with you if you want.  (Send me email.)

  I would create a thread manager (TM) class, a globally-scoped singleton (single instance) that maintains a vector of thread class instances (e.g. MyThread*) .  (In my version, I have a scheduler where I control the number of threads that are simultaneously alive so I can optimize overall throughput for the processor I'm running on; i.e. n=2 for Hyperthreaded P4). 

  Methods of the TM are guarded with critical sections (CS) so that the thread existence in the list and their important states are always correct. 

  Each thread class has an id (since pointer to thread classes are not guaranteed unique) and a status field, which can be an enum like: PendingScheduling, Running, Finished, FinishedWithError, and Cancelled. 

  The thread status is written through the TM as it has the CS guarding.  Thread status is read anywhere, anytime.  To cancel a thread (i.e. signal it) using its id, call TM::CancelThread( id ).  It takes the CS, looks up pThread where (pThread->id == id), checks to see if it is not finished, sets it to Cancelled, and releases the CS.  (The Platform SDK headers contain a simple and adequate CriticalSection utility class.)

  Finally, to get to your point, threads can check their cancel status while they do their work.  This can be done w/o the TM, just read from this->status.  

  int MyThread::DoWork()
  {
       while( workremains && (this->status != Cancelled) ) ...
  }

  You may need to do this check in multiple places, depending on how responsive you want the cancellation to occur.

  You can decide if you want the thread class instance to be destroyed by the thread at the end of the thread entry point, but it needs to be a CS-guarded operation through the TM.  Alternatively, the thread needs to set the status to Finished (or FinishedByCancellation), and the TM deletes it later.

  Brian

   

  
  

• ZenX

  The only other way I can think of is to implement some kind of garbage collector for your most critical resources (such as global memory or files), And then tell all your worker threads that they MUST only use the garbage collected resources you provide.

  At thread termination, your main thread will kill all other processes, and can release the resources used by them through the global garbage collector.

  This method does have its problems. What happens when the worker thread is going to really need a resource you don't garbage collect (eg. a database connection). Will you implement a garbage collector for db connections just for that one caller

  To my knowledge, sprinkling CheckForKill calls across your code (or having some other code, like the message loop nish mentioned, sprinkle it for you) is really the only way to go.

  
  
  

• i_van

  As you probably might have read, MSDN specifically advises against calling TerminateThread.

  I personally don't think there's a magic bullet.  Terminating a thread is like the following:

  int simulate_system_termination  = 0;
  void function_pretending_to_be_a_thread()
  {
     void* buf = malloc(100);

     if( simulate_system_termination )
     {
         return;
     }
  
     free( buf ):
  }

  The problem of making sure buf is freed is the same as the problem of freeing thread resources if a thread is terminated before cleanup has had a chance to occur.  The matter is further complicated by the fact that a thread can also access memory allocated by other threads (including the main thread). 
  
  Unless all thread resources are embedded in the thread local store--not an idealistic scenario--thread code still need to be designed by the programmer where leaks are guaranteed not to occur.  So you're back to relying on a signaling mechanism of some sort, such as what OShah advises.

  Brian
  

  
  

• Jim25_SQL

  Hello Brian,

  I did get your reply in the notification, but it's not complete there, and I don't see it in this thread. Would you mind trying to get it here again, so that I can read it's full text

  The last I can read of your text says "The thread manager has a Ca..."

  Thanks!

  Uwe

  
  

• armin99

  Thanks Nishant,

  these worker threads crunch through massive amounts of data, and since that's their sole purpose in life, they don't have a message loop, thus no messaging (at least not through message loops) is available. As I see it, termination notification needs to come in through global flags of some sort, that's why I chose to go with the least labor intensive flagging through thread priority change. I elevate it. This has the added benefit of telling the operting system to focus on that particular thread, which is exactly what is needed in that circumstance. The user intends to interrupt number crunching, so hurry up and do it.

  Any eye-opening suggestions anyone, please

  Thanks!

  Uwe

  
  

• linkcd

  This is very very useful information!

  I assume the thread manager has a message loop which your app uses for tm control

  Thanks Brian!

  Uwe

  
  

• Seemit

  If your thread has a message loop, you could PostThreadMessage a custom message, and in its handler do cleanup. I know that sounds like an over-simplification, but you could mix that with an event to get a clean way to close a thread.
  
  

• ghostwrtrone

  Oh no Brian,

  of course that's not how this works. It's not that I am killing a thread in mid track, it does indeed get a chance to clean up after itself. The problem is the accurate tracking of what needs to be cleaned up.

  The worker could be interrupted in any line of it's processing, or better at any positive CheckForKill() return, and there may be a ton of different scenarios at which clean up needs to be accurately specific to this point. And it can be a daunting task to envision every possible scenario of cleanup. I've gotten good at that, but still a once in a full moon random crash is one too many, imo. Also the amount of development time put into this is over proportional with respect to much of the other code.

  I just can't imagine folks in Redmont and elsewhere having to go through each and every hop of this everytime they use a non-message-looped worker thread. Well, may be someone there has a statement about this when they get back to work tomorrow morning :-)

  Thanks Brian!

  Uwe

  
  

• UnkName

  Thanks for your comments!

  I'd really be interested to learn how the Microsoft pros do this. There has got to be the "best proven" method in the days of HT and dual core CPUs.

  Thanks again!

  Uwe

  
  

• rtmos

  Well, I guess I'm blessed to have the extra hours of work since I'm American, not a UK resident.  :) 

  If you're not so constrained, you may want to consider moving to .NET.  Background processing is managed by a new 2.0 class called BackgroundWorker, and garbage collection will take care of your resource tracking issue.

  Brian

  
  

• Ashishbuddha

  Good, feeling better now :-) The post time is UTC, that's why I was wondering...

  Yes, constrained, no .NET for this one...

  I am curious as to what signalling technique you use to interrupt your worker's processing, would you mind sharing that

  Am hard at work shoving all resource dealloc business into the destructor, not an easy feast with a full-fledged app already in place. I guess some subsequent beta testing is in order :-) That one crash a month just bugs me enough :-)

  Thanks,

  Uwe

  
  

• BradLeach

  What I tend to do is associate each thread with a class (and you can even define its entry point as a static member function).  Track all resources consumed by the thread with class data members, and let the destructor guarantee that things get cleaned up.
  
  e.g.
  
  MyThread* pThread = new MyThread();
  _beginthread( &MyThread::EntryPoint, 0, (void*)pThread ); // or CreateThread equiv.
  
  class MyThread
  {
     SomeResource* pResource1;
  
  public:
     MyThread()
     {
         pResource1 = 0;
     }

     SomeResource* GetResource1()
     {
        assert( pResource1 == 0 );
        pResource1 = new SomeResource();
     }

     void FreeResource1()
     {
        delete( pResource1 );
        pResource1 = 0;
     }

     ~MyThread()
      {
          FreeResource1(); // guarantee cleanup
      }
  
      void DoWork()
      {
         // action happens here, allocating and freeing resources as needed
      }
  
      static void EntryPoint( void* context )
      {
         MyThread* pThread = (MyThread*)context;
         pThread->DoWork();
         delete pThread;  // calls destructor to guarantee resource freeing
      }

   };

  Furthermore, you can declare a static counter scoped in MyThread that keeps track of the number of threads that are alive.  It is incremented in EntryPoint prior to DoWork, and decremented after delete pThread. At the bottom of your WinMain(), assert that MyThread::numThreadsAlive == 0.  If it fires, you have an indication of a thread that has not cleaned up after itself.

  
  

• BostonMerlin

  Nope, no message loop!  I prefer to keep all the windowing code, and hence message pumping, in the main thread.  The only Win32 footprint the TM, as I've described it, has is CreateThread and the four CriticalSection (Initalize ,Enter, Leave, and Delete) APIs.

  Instead of a thread signaling to the main thread "i'm done," I would have the main thread poll the TM for threads that are finished at certain times, such as on a WM_TIMER message or when the message queue becomes empty.  Or at the most, have the TM post a message to the main application window whenever a thread sets its status to finished (maybe just to kick the message pump out of an idle state so that the message queue can become empty again for another TM poll.)  In my experiences, mixing multithreading and message pumping code needs to be minimized so you're not creating unpredictibilities. 

   

  
  

• Bertrand Larsy

  Brian,

  I hope you are not in the UK, as your time-stamp suggests because in which case you ought to be holding a nap now :-)

  Actually all my worker threads are always class-wrapped. But placing all resource tracking the way you suggest sounds like a marvelous idea. The destructor is the one place the code will crunch through regardless of point of interrupt. Why didn't I think of that

  Hey I know what I'll be doing tomorrow!

  Thanks a mille Brian!

  Uwe

  
  
• "Correct" Workerthread Termination