Add a new branch for C++ 17.

[LYP951018]

Notable changes:

1. std::result_of has been deprecated in C++ 17. Use std::invoke_result instead.
2. We should use std::invoke instead of write f(args...) directly.
3. We could use initializer in lambda's capture list in C++ 14, so instead of make_shared<std::packaged_task> and copying it into the lambda we can write [task = std::move(task)].

[jhasse]

I've managed to implement 1: https://github.com/jhasse/ThreadPool/commit/428aeaed8b20aaafafdb2d9b007a85d571de85bf

But I'm having troubles with 3. This is what I've come up with so far:

    std::packaged_task<return_type()> task(
            std::bind(std::forward<F>(f), std::forward<Args>(args)...)
        );

    std::future<return_type> res = task.get_future();
    {
        std::unique_lock<std::mutex> lock(queue_mutex);

        // don't allow enqueueing after stopping the pool
        if(stop)
            throw std::runtime_error("enqueue on stopped ThreadPool");

        tasks.push([task = std::move(task)]() mutable { std::invoke(task); });
    }

But GCC complains:

In file included from /usr/include/c++/7/future:48:0,
                 from ../ThreadPool.h:10,
                 from ../example.cpp:5:
/usr/include/c++/7/bits/std_function.h: In instantiation of ‘static void std::_Function_base::_Base_manager<_Functor>::_M_clone(std::_Any_data&, const std::_Any_data&, std::false_type) [with _Functor= ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>; std::false_type = std::integral_constant<bool, false>]’:
/usr/include/c++/7/bits/std_function.h:227:16:   required from ‘static bool std::_Function_base::_Base_manager<_Functor>::_M_manager(std::_Any_data&, const std::_Any_data&, std::_Manager_operation) [with _Functor = ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>]’
/usr/include/c++/7/bits/std_function.h:695:19:   required from ‘std::function<_Res(_ArgTypes ...)>::function(_Functor) [with _Functor = ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>; <template-parameter-2-2> = void; <template-parameter-2-3> = void; _Res = void; _ArgTypes = {}]’
../ThreadPool.h:80:9:   required from ‘std::future<typename std::invoke_result<_Functor, _ArgTypes>::type> ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]’
../example.cpp:20:14:   required from here
/usr/include/c++/7/bits/std_function.h:192:6: error: use of deleted function ‘ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>::<lambda>(const ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>&)’
      new _Functor(*__source._M_access<_Functor*>());
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../example.cpp:5:0:
../ThreadPool.h:80:43: note: ‘ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>::<lambda>(const ThreadPool::enqueue(F&&, Args&& ...) [with F = main()::<lambda()>; Args = {}; typename std::invoke_result<_Functor, _ArgTypes>::type = int]::<lambda()>&)’ is implicitly deleted because the default definition would be ill-formed:
         tasks.push([task = std::move(task)]() mutable { std::invoke(task); });
                                           ^
../ThreadPool.h:80:43: error: use of deleted function ‘std::packaged_task<_Res(_ArgTypes ...)>::packaged_task(const std::packaged_task<_Res(_ArgTypes ...)>&) [with _Res = int; _ArgTypes = {}]’
In file included from ../ThreadPool.h:10:0,
                 from ../example.cpp:5:
/usr/include/c++/7/future:1516:7: note: declared here
       packaged_task(const packaged_task&) = delete;
       ^~~~~~~~~~~~~

Why doesn't it use the move constructor of packaged_task?

[LYP951018]

@jhasse Oops! The standard requires F to be CopyConstructible...

5. Initializes the target with a copy of f. If f is a null pointer to function or null pointer to member, *this will be empty after the call. This constructor does not participate in overload resolution unless f is Callable for argument types Args... and return type R. (since C++14)

http://en.cppreference.com/w/cpp/utility/functional/function/function So maybe we have to stick with shared_ptr here.

[jhasse]

We could use unique_ptr though :)

[jhasse]

Okay I misunderstood the problem. https://stackoverflow.com/questions/32486623/moving-a-lambda-once-youve-move-captured-a-move-only-type-how-can-the-lambda

The solution is to replace std::function with an implementation which also works with move-only Callables. I've found fu2::unique_function from here: https://github.com/Naios/function2

The end result looks like this: https://github.com/jhasse/ThreadPool/commit/f9b177419463511ed3ea64deb24a00bac7cb0516

2. We should use std::invoke instead of write f(args...) directly.

Do you simply mean replacing (*task)(); with std::invoke(*task)? Or getting rid of std::bind?

[jhasse]

Or even easier: Simple move the packaged_task as suggested here: #51

[mshingote]

Can I create a branch and pull request for this issue?

[jhasse]

I don't think this repository is maintained any more, so I wouldn't bother with a PR. See my C++17 fork here: https://github.com/jhasse/ThreadPool

[fogti]

another fork: https://github.com/zserik/ThreadPool I try to merge all important changes.

[mshingote]

Great Thanks! @jhasse and @zserik 👍

[ganler]

@jhasse Oops! The standard requires F to be CopyConstructible...

1. Initializes the target with a copy of f. If f is a null pointer to function or null pointer to member, *this will be empty after the call. This constructor does not participate in overload resolution unless f is Callable for argument types Args... and return type R. (since C++14)

http://en.cppreference.com/w/cpp/utility/functional/function/function So maybe we have to stick with shared_ptr here.

@LYP951018 Actually, why not simply use raw pointers with some exception code ... Without considering exceptions, you may just:

auto task = new std::packaged_task<return_type()>(
        std::bind(std::forward<Func>(f), std::forward<Args>(args)...));
..
m_task_queue.emplace( [task](){ (*task)(); delete task; } );

I don't really know if this approach could solve ur problem...

[aphenriques]

Yet another fork: https://github.com/aphenriques/thread It slightly modifies @zserik's https://github.com/zserik/ThreadPool and adds some utilities.

[fogti]

@aphenriques Why did you change the build system back to Makefiles? I'm just curious, because I think they would be a bigger maintenance burden than a CMake or meson-based system.

[aphenriques]

@zserik All projects I work with use Make as the build system. CMake or other modern build systems could probably offer greater flexibility, however, I think I would have less trouble maintaining the workflow of my code base. Moreover, the makefiles are quite automated. By the way, thanks for your contribution. Soon, some of your code will be in production.

[fawdlstty]

yet another similar project: taskpool, it can:

• sequential executive task
• async wait

#include "taskpool_t.hpp"

// ...

taskpool_t _pool { 1 };

// example run task
auto _ret1 = _pool.run ([] (int answer) { return answer; }, 42);
std::cout << _ret1.get () << std::endl;

// eample sequential executive task
auto _f0 = _pool.wait (std::chrono::seconds (3));
auto _f1 = _pool.after_wait (std::move (_f0), std::chrono::seconds (3));
auto _f2 = _pool.after_run (std::move (_f1), [] () { std::cout << "1\n"; return 2; });
auto _f3 = _pool.after_wait (std::move (_f2), std::chrono::seconds (3));
auto _f4 = _pool.after_run (std::move (_f3), [] (int n) { return n + 10; });
auto _f5 = _pool.after_run (std::move (_f4), [] (int n) { std::cout << n << "\n"; });