progschj / ThreadPool

A simple C++11 Thread Pool implementation
zlib License
7.64k stars 2.21k forks source link

avoid pending when stop worker thread #30

Closed loveyacper closed 3 years ago

loveyacper commented 8 years ago

Under given conditions, worker thread may be suspended

The main thread destruct pool instance:

    {
        std::unique_lock<std::mutex> lock(queue_mutex);
        stop = true;
    }
    condition.notify_all();

In worker thread, insert a statement to sleep make the problem happen:

      std::function<void()> task;

       {
                // sleep 1 second, let main thread destruct pool first
                std::this_thread::sleep_for(std::chrono::seconds(1));

                std::unique_lock<std::mutex> lock(this->queue_mutex);

                // WILL WAIT FOREVER!!!
                this->condition.wait(lock,
                            [this]{ return this->stop || !this->tasks.empty(); });
wilx commented 8 years ago

Have you actually seen this supposed issue happen? My reading of

                this->condition.wait(lock,
                            [this]{ return this->stop || !this->tasks.empty(); });

Is that it should first check the condition in the supplied lambda and only if that is not satisfied, loop and wait.

loveyacper commented 8 years ago

Sorry... Yes, the wait condition (return this->stop) can avoid hang

loveyacper commented 8 years ago

I add some code for expand thread pool runtime. Suppose all worker threads are executing tasks, now you enqueue new task, it won't be executed immediately, I modified it to create new worker in this case.

https://github.com/loveyacper/ThreadPool

hongliuliao commented 3 years ago

I think we need make "notify_all" in lock guard to avoid this issue

aorandexiaohai commented 3 years ago

不用整很多高大上的编译上的东西,弄好性能就行。