Solved

kill thread

Posted on 2011-03-14
35
638 Views
Last Modified: 2012-05-11
Is it a good practise to kill a thread in a big running application. I spawn a thread withing a class and run it in while(1) loop. In the destructor of the class instead of join() I need to kill the thread so that all the resources are released. is that bad to kill thread?
0
Comment
Question by:learningunix
  • 20
  • 13
  • 2
35 Comments
 
LVL 33

Assisted Solution

by:sarabande
sarabande earned 75 total points
ID: 35128349
no you always should terminate a thread gently by notifying it that it should terminate in the main thread and then wait until the thread has recognized notification and has terminated itself by returning from thread function.

the simplest kind of notification is setting a shared bool flag to true which was checked as often as possible in the thread.

Sara
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128362
>> is setting a shared bool flag to true

make sure you declare it volatile otherwise it may be optimised and cached so the changes are not see x-thread.
http://drdobbs.com/184403766
0
 

Author Comment

by:learningunix
ID: 35128367
The problem is the thread sleeps every 60 seconds and then wakes up. In that case, I'll have to wait 60 secs which is not good.

Why can't I directly kill the thread.

If it is good, what is the best way to kill the thread created  by

create(&myThread, &myAttr, (void* (*)(void*)) &myLoop, (void*) this);
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128418
>> Why can't I directly kill the thread.

If you kill it whilst it's doing something important (like writing to a file) it'll cause inconsitancies.

>> The problem is the thread sleeps every 60 seconds
Why not use a timed event? A kernel primitive that blocks for a time but can be unblocked on demand. How you do this depends on your OS but boost has a condition variable you could use.

http://www.boost.org/doc/libs/1_46_1/doc/html/thread/synchronization.html#thread.synchronization.condvar_ref.condition_variable
0
 

Author Comment

by:learningunix
ID: 35128423
I can definitely terminate in the main thread inside a function instead of destructor but is that recommended
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128433
There is never a good/safe way to "kill" a thread. If your code demands this you need to review your code as the design is wrong.
0
 

Author Comment

by:learningunix
ID: 35128473
Currently the thread does sleep(60); not sure how can I signal this thread while its sleeping.
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128495
The idea is you remove the sleep and wait on the condition variable instead using a timed wait. If you wish to exit early you signal the condition variable. If the timed wait returns true it was due to the condition being set (ie you want it to exit) if it's return value is false it means the timed wait expired so you just do what you would have done when your sleep expired.
0
 

Author Comment

by:learningunix
ID: 35128496
void  myLoop()
{
    while (1)
    {
       .....
       ....
       sleep(60);
   }
}
 
is there a way to do conditional sleep where I can wake up this thread if some condition occurs?
 
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128508
See my comment above.
0
 

Author Comment

by:learningunix
ID: 35128509
@evilrix:  but I need to re-execute the loop every 60 secs. so sleep is must.
Its just at the end I need to stop this thread
0
 
LVL 33

Expert Comment

by:sarabande
ID: 35128511
you could make a loop and sleep 1 second each circle. there you can check the stop flag.

Sara

0
 
LVL 40

Accepted Solution

by:
evilrix earned 175 total points
ID: 35128549
>> but I need to re-execute the loop every 60 secs. so sleep is must.
That is why you use a timed wait. When the timed wait ends you test the return value. If it's true you exit the thread if it's not you act as though the sleep had finished and then do whatever you need to do then go back to the timed wait.

void  myLoop()
{
    while (1)
    {
       .....
       ....
       if(condition.timed_wait())
       {
             break;
       }
   }
}

>> you could make a loop and sleep 1 second each circle. there you can check the stop flag.
You could but it's a sub-optimal way to do this.
0
 

Author Comment

by:learningunix
ID: 35128579
Will this work?

while (1)
  {
      .....
      cond_timedwait(&myWait, &myMutex, &myTime);  //myTime will be current time + 60 secs
      //If the return value is true means, it timed out and I continue the execution
       {
             //  continue the execution
        }
      // if the return value is false, it means someone else has send us the signal, I can break the while
         {
            break;
         }
}

0
 

Author Comment

by:learningunix
ID: 35128590
oops, I didn't see ur previous comments while posting.
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128595
That's the idea except (assuming you plan to use boost condition variable) your boolean logic is inverted.

"timed_wait returns false if the call is returning because the time specified by abs_time was reached, true otherwise"

>> oops, I didn't see ur previous comments while posting.

:)
0
 

Author Comment

by:learningunix
ID: 35128623
Here's the final code I am planning:


void cleanUp() {
   cond_signal(&myCond);
}


void myLoop() { 
  while (1)
  {
      .....
      cond_timedwait(&myWait, &myMutex, &myTime);  //myTime will be current time + 60 secs
      //If the return value is FALSE means, it timed out and I continue the execution
       {
             //  continue the execution
        }
      // if the return value is TRUE , it means someone else has send us the signal, I can break the while 
         {
            break;
         }
  }

}

Open in new window

0
Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

 

Author Comment

by:learningunix
ID: 35128626
THis

void cleanUp() {
   cond_signal(&myCond);
}


should be

void cleanUp() {
   cond_signal(&myWait);
}
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128653
That looks like the kind of thing you need to do. So when you call cleanup it will signal the condition, cause the timed wait to abort and your thread will clean up. Don't forget you need to wait for the thread to re-join though, otherwise you run the risk of terminating your program before the thread has completed. You might want to add that in the cleanup. For example (and I am not claiming this is syntactically correct.... it's just an example):

void cleanUp() {
   cond_signal(&myCond);
   myThread.join();
}
0
 

Author Comment

by:learningunix
ID: 35128744
I guess one problem, currently I am using the same mutexLock during signalling, will that be a problem?

bool stopthread = false;
void cleanUp() {
   mutex_lock(myMutex);
     cond_signal(&myWait);
   mutex_unlock(myMutex); 

   // Instead of join can I do this
   while (!stopthread); // this will guarantee thread finishes. 
}


void myLoop() { 
  while (1)
  {
      .....
      cond_timedwait(&myWait, &myMutex, &myTime);  //myTime will be current time + 60 secs
      //If the return value is FALSE means, it timed out and I continue the execution
       {
             //  continue the execution
        }
      // if the return value is TRUE , it means someone else has send us the signal, I can break the while 
         {
            stopthread = true;    
            break;
         }
  }

}

Open in new window

0
 

Author Comment

by:learningunix
ID: 35128771
Also what if the thread is in the middle of the execution inside the if loop and then cleanUp() invokes signal. in that case, the thread is not waiting on cond_timedwait() and it won't catch the signal at all ??
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128781
Why do you need a mutex to protect cond_signal in cleanup? You don't need to do that, it should already be thread safe if you are using a proper privative.


>> while (!stopthread);
This is really not the way to wait for a thread to end. Your thread library will have a proper way to join a thread. This will just chomp CPU and could even prevent the thread from ending!
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128785
>> Also what if the thread is in the middle of the execution inside the if loop and then cleanUp() invokes sig

The timed_wait will end immediately when you call it as the condition will be set. It is designed to handle this case.
0
 

Author Comment

by:learningunix
ID: 35128789
ohh...i see i thought I need mutex for sending signal also.

Also what if the thread is in the middle of the execution inside the if loop and then cleanUp() invokes signal. in that case, the thread is not waiting on cond_timedwait() and it won't catch the signal at all ??
0
 

Author Comment

by:learningunix
ID: 35128803
i guess this should be good
void cleanUp() {
     cond_signal(&myWait);
     join(&thread);
}


void myLoop() { 
  while (1)
  {
      .....
      cond_timedwait(&myWait, &myMutex, &myTime);  //myTime will be current time + 60 secs
      //If the return value is FALSE means, it timed out and I continue the execution
       {
             //  continue the execution
        }
      // if the return value is TRUE , it means someone else has send us the signal, I can break the while 
         {
            break;
         }
  }

}

Open in new window

0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128856
Better :)

>> Also what if the thread is in the middle of the execution inside the if loop and then cleanUp() invokes signal.
See my comment above, this is fine. It will correctly handle this.

0
 

Author Closing Comment

by:learningunix
ID: 35128873
thx a lot!
0
 
LVL 40

Expert Comment

by:evilrix
ID: 35128880
You're welcome.
0
 

Author Comment

by:learningunix
ID: 35129384
      If (status == 0)  // it timed out and I continue the execution
       {
             //  continue the execution
        } else  // , it means someone else has send us the signal, I can break the while
         {
            do I have to unlock the mutex ??
            break;
         }

0
 
LVL 40

Expert Comment

by:evilrix
ID: 35129441
>> do I have to unlock the mutex ??

You actually wait on a scoped lock not a mutex. The lock already has the mutex. When the thread ends the scoped lock releases the mutex.

There is an example here.
0
 

Author Comment

by:learningunix
ID: 35129505
Here's what I am doing. am i releasing the lock properly?
void cleanUp() {
     cond_signal(&myWait);
     join(&thread);
}


void myLoop() { 
  while (1)
  {
      .....
      mutex_lock(&myMutex);   

      cond_timedwait(&myWait, &myMutex, &myTime);  //myTime will be current time + 60 secs
If (status == 0)  // it timed out and I continue the execution
       {
             //  continue the execution
        }
       else 
         {
            break;
         }
  mutex_unlock(&myMutex )
  }
   
}

Open in new window

0
 

Author Comment

by:learningunix
ID: 35129508
int status = cond_timedwait(&myWait, &myMutex, &myTime);
0
 

Author Comment

by:learningunix
ID: 35129542
I thinks so lock is not released during break;   ??
0
 

Author Comment

by:learningunix
ID: 35129637
also I believe the condition should be inverted ??
0
 

Author Comment

by:learningunix
ID: 35129837
cond_timedwait automatically releases the mutex, and waits on the condition variable cond. When the condition is signaled or the time expires, cond_timedwait reacquires the mutex and returns to the caller. The wait can also be interrupted by a UNIX system signal, in which case mutex is reacquired, the signal handler is called, and cond_timedwait returns EINTR.


I guess I need to unlock_mutex just before the break
0

Featured Post

Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

  Included as part of the C++ Standard Template Library (STL) is a collection of generic containers. Each of these containers serves a different purpose and has different pros and cons. It is often difficult to decide which container to use and …
Basic understanding on "OO- Object Orientation" is needed for designing a logical solution to solve a problem. Basic OOAD is a prerequisite for a coder to ensure that they follow the basic design of OO. This would help developers to understand the b…
The goal of the video will be to teach the user the concept of local variables and scope. An example of a locally defined variable will be given as well as an explanation of what scope is in C++. The local variable and concept of scope will be relat…
The viewer will be introduced to the technique of using vectors in C++. The video will cover how to define a vector, store values in the vector and retrieve data from the values stored in the vector.

863 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

20 Experts available now in Live!

Get 1:1 Help Now