bshoshany / thread-pool

BS::thread_pool: a fast, lightweight, and easy-to-use C++17 thread pool library
MIT License
2.21k stars 253 forks source link

[REQ] worker keeps its task alive for longer than required #124

Closed Iunave closed 1 year ago

Iunave commented 1 year ago

By declaring the std::function<void()> task outside the scope of the worker loop the task is kept alive until the worker acquires the next task to process. This is both unnecessary and is prone to unexpected behavior for code that expects its task to be destroyed once it has finished executing. eg: binding a shared pointer to the task object. The fix is really simple, just move the std::function<void()> task to where it is assigned.

how the new worker function would look like

    void worker()
    {
        while (true)
        {
            std::unique_lock tasks_lock(tasks_mutex);
            task_available_cv.wait(tasks_lock, [this] { return !tasks.empty() || !workers_running; });
            if (!workers_running)
                break;
            if (paused)
                continue;
            std::function<void()> task = std::move(tasks.front());
            tasks.pop();
            ++tasks_running;
            tasks_lock.unlock();
            task();
            tasks_lock.lock();
            --tasks_running;
            if (waiting && !tasks_running && (paused || tasks.empty()))
                tasks_done_cv.notify_all();
        }
    }
bshoshany commented 1 year ago

Hi @Iunave and thanks for opening this issue! This seems like an easy enough change. I will implement it in v3.6.0, to be released within the next few weeks.