Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ltwardus opened this issue Feb 16, 2013 · 3 comments
Closed

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

ltwardus opened this issue Feb 16, 2013 · 3 comments

Comments

@ltwardus
Copy link

"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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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 :)

progschj added a commit that referenced this issue Feb 17, 2013
Replaced deque by a queue (which by default uses a deque).
Simplified the task packaging as suggested by Ang3lus in issue #4.
The destructor now waits for the queue to empty but throws
a std::runtime_error if enqueue is called on the stopped pool
(see issue #3).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants