ErrWatt
ErrWatt

Reputation: 73

What's a a good way to stop a pool of threads from running?

I've been working on a project lately, and i need to manage a pair of thread pools. What the worker threads in the pools do is basically execute some kind of pop operation to each respective queue, eventually wait on a condition variable (pthread_cond_t) if there is no available value in the queue, and once they get an item, parse it and execute operations accordingly. What i'm concerned about is the fact that i want to have no memory leaks, and to achieve that i noticed that calling a pthread_cancel on each thread when the main process is exiting is definitely a bad idea, as it leaves a lot of garbage around. The point is, my first thought was to use a exit flag which i can set when the threads need to exit, so that they can easily free memory and call a pthread_exit... I guess i should set this flag, then send a broadcast signal to the threads waiting on the condition variable and check the flag right after the pop operation...

Is this really the correct way to implement a good thread pool termination? I don't feel that much confident about this... I'm writing some pseudo-code here to explain what i'm talking about

Each pool thread will run some code structured like this:

/* Worker thread (which will run on each pool thread) */
{ /* Thread initialization */ ... }
loop {
   value = pop();
   { /* Mutex lock because of the shared flag */ ... }
   if (flag) {{ /* Free memory and unlock mutex */ ... } pthread_exit(); }
   { /* Unlock the mutex */ ... }
   { /* Elaborate value */ ... }
}
return NULL;

And there will be some kind of pool_stopRunning() function which will look like:

/* pool_stopRunning() function code */
{ /* Acquire flag mutex */ ... }
setFlag(flag);
{ /* Unlock flag mutex */ ... }
{ /* Acquire queue mutex */ ... }
pthread_cond_broadcast(...);
{ /* Unlock queue mutex */ ... }

Thanks in advance, i just need to be sure that there isn't a fancy-er way to stop a thread pool... (or get to know a better way, by any chance) As always, i'm sorry if there is any typo, i'm not and english speaker and it's kind of late right now >:

Upvotes: 2

Views: 1197

Answers (2)

Nemo
Nemo

Reputation: 71565

What you are describing will work, but I would suggest a different approach...

You already have a mechanism for assigning tasks to threads, complete with all appropriate synchronization. So instead of complicating the design with some new parallel mechanism, just define a new type of task called "STOP". If there are N threads servicing a queue and you want to terminate them, push N STOP tasks onto the queue. Then just wait for all of the threads to terminate. (This last can be done via "join", so it should not require any new mechanism, either.)

No muss, no fuss.

Upvotes: 1

Jeff Holt
Jeff Holt

Reputation: 3190

With respect to symmetry with setting the flag and reducing serialization, this code:

{ /* Mutex lock because of the shared flag */ ... }
if (flag) {{ /* Free memory and unlock mutex */ ... } pthread_exit(); }
{ /* Unlock the mutex */ ... }

should look like this:

{ /* Mutex lock because of the shared flag */ ... }
flagcopy = readFlag();
{ /* Unlock the mutex */ ... }
if (flagcopy) {{ /* Free memory ... } pthread_exit(); }

Having said that, you can (should?) factor the mutex code into the setFlag and readFlag methods.

There is one more thing. If the flag is only a boolean and it is only changed once before the whole thing shuts down (i.e., it's never unset after being set), then I would argue that protecting the read with a mutex is not required.

I say this because if the above assumptions are true and if the loop's duration is very short and the loop iteration frequency is high, then you would be imposing undue serialization upon the business task and potentially increasing the response time unacceptably.

Upvotes: 0

Related Questions