4

I have a rather simple thread pool, and i have a question regarding thread finalizing.

this is my worker snippet :

static void* threadpool_worker(void* pool_instance)
{
    int rc;
    struct threadpool* pool = (struct threadpool*)pool_instance;
    struct threadpool_task *task;

    for(;;)
    {
        pthread_mutex_lock( &(pool->task_queue_mutex) );

        while( pool->headp->tqh_first == NULL )
        {
            rc = pthread_cond_wait( &(pool->task_queue_cond), &(pool->task_queue_mutex) );
        }

        task = pool->headp->tqh_first;
        TAILQ_REMOVE(pool->headp, pool->headp->tqh_first, entries);

       pthread_mutex_unlock( &(pool->task_queue_mutex) );
       task->routine_cb(task->data);
    }

}

so jobs are executed at this line task->routine_cb(task->data);

and in order to finalize workers threads i'm calling threadpool_enqueue_task

in the following way :

for( i=0 ; i < pool->num_of_workers ; ++i)
{
    threadpool_enqueue_task(pool, pthread_exit, NULL);
}

expecting that pthread_exit will be called in here task->routine_cb(task->data) but it does not work this way, i don't see any explicit error, just memory leak in valgrind

but when i change the worker code like that :

    if(task->routine_cb == pthread_exit)
    {
        pthread_exit(0);
    }
    task->routine_cb(task->data);

everything ends fine. so my question is is there an option to stop the worker just making it to execute pthread_exit in some way,without changing the worker code.

Edit: Thread pool task declared as following :

struct threadpool_task
{
    void (*routine_cb)(void*);
    void *data;
    TAILQ_ENTRY(threadpool_task) entries;          /* List. */
}

As per my understanig there should be no problem to get address of pthread_exit in routine_cb which declared :

extern void pthread_exit (void *__retval) __attribute__ ((__noreturn__));
Dabo
  • 2,371
  • 2
  • 18
  • 26
  • Is this helpful ? http://stackoverflow.com/questions/2084830/kill-thread-in-pthread-library – Suvarna Pattayil Dec 16 '13 at 09:45
  • they suggest actually something that i'm trying to avoid, id on't want to use pthread_cancel from the reasons mentioned in that post: pthread_cancel(thr) However, this not a recommended programming practice! It's better to use an inter-thread communication mechanism like semaphores or messages to communicate to the thread that it should stop execution. – Dabo Dec 16 '13 at 09:58
  • What OS/compiler do you use? – Sergey L. Dec 16 '13 at 10:52
  • How is `routine_cb` declared? – alk Dec 16 '13 at 10:56
  • "*... but it does not work this way ...*", so the threads do **not** end? They are still around? Did you try enqueing a wrapper to `pthread_exit()` logging it was called? – alk Dec 16 '13 at 11:02
  • My OS is ubuntu 13.04, g++ compiler. this is my threadpool_task declaration struct threadpool_task { void (*routine_cb)(void*); void *data; TAILQ_ENTRY(threadpool_task) entries; /* List. */ } – Dabo Dec 16 '13 at 11:04
  • pthread_exit called and threads appears to be dead, but i see that memory leak, which makes me think that it fails somewhere – Dabo Dec 16 '13 at 11:11
  • 2
    Too many unknowns. Can you reproduce this behaviour in a small self-contained example? – n. m. could be an AI Dec 16 '13 at 11:25
  • Just to add - in general there's nothing wrong with using `pthread_cancel()`, as long as you make sure that your code is written with cancellation in mind - that is, make sure that cancellation points exist, and that you create cleanup handlers when necessary. – sonicwave Dec 16 '13 at 11:38
  • did you investigate what memory leak valgrind reported? – tristan Dec 16 '13 at 15:28
  • @n.m. I don't really see how it's possible to shrink this example without knowing where the problem is. – Andrey Mishchenko Dec 16 '13 at 16:11
  • You don't necessarily need to shrink the example (though it would be nice). You do need to make it self-contained, so that the behaviour could be verified. – n. m. could be an AI Dec 16 '13 at 16:41
  • Without seeing what valgrind said it's a little difficult to help. There's nothing there, at the time of this writing, that could leak. – Tommi Kyntola Dec 16 '13 at 17:30

1 Answers1

0

I found the cause of the leak. It was my fault of course. I rewrote the job invocation in the following way :

    void (*routine)(void*) = task->routine_cb;
    void* data = task->data;
    free(task);
    routine(data);

instead of : 

    task->routine_cb(task->data);
    free(task);

and there were no more leaks, and threads stopped as i expected. Thanks to everyone who tried to help.

Dabo
  • 2,371
  • 2
  • 18
  • 26