progschj / ThreadPool

A simple C++11 Thread Pool implementation
zlib License
7.86k stars 2.24k forks source link

Workers will not finish their jobs if we want to stop the pool. #3

Closed ltwardus closed 11 years ago

ltwardus commented 11 years ago

"if(pool.stop)" - line 84: Worker will just exit even if there are some jobs to do, should be: if(pool.stop && pool.tasks.empty())

progschj commented 11 years ago

I see why one would want the behavior but the solution might be more complicated. Essentially it creates the possibility of "endless loops" if jobs themselves enqueue other jobs.

struct recursive {
    recursive(ThreadPool &pool) : pool(pool) { }

    void operator()()
    {
        pool.enqueue<void>(recursive(pool));
    }

    ThreadPool &pool;
};

int main()
{
    ThreadPool pool(1);
    pool.enqueue<void>(recursive(pool));
    return 0;
}

I guess one could say that is the responsibility of whoever sets up the jobs though to create a way to stop those if needed. On the other hand a explicit "stop_now" method in addition to the waiting destructor or the inverse (stopping destructor and "wait" method) would also be possible.

Hmm...

Or maybe just "clear_tasks()" to explicitly remove all waiting tasks?

progschj commented 11 years ago

The clear_tasks thing wouldn't work though since one of the still running tasks could still enqueue new stuff after the clear. I wonder if trying to enqueue with stop == true should maybe throw an exception?

ltwardus commented 11 years ago

In my opinion if somebody is using "recurrent" tasks then it is his problem that he could not stop the queue, this is just the facility and can be used in good or bad way (as all the other things, You have threads in the standard, You can do parallel computing or hit the deadlock if used in the wrong way :)) ... but the exception thrown when the queue should stop and somebody want to enqueue tasks is good idea, almost no change in code, and this will help the user to find potential dangerous places :)