I am analysing the following code snippet and am trying to understand it in detail:
template<typename FUNCTION, typename... ARGUMENTS>
auto ThreadPool::add( FUNCTION&& Function, ARGUMENTS&&... Arguments ) -> std::future<typename std::result_of<FUNCTION(ARGUMENTS...)>::type>
{
using PackedTask = std::packaged_task<typename std::result_of<FUNCTION(ARGUMENTS...)>::type()>;
auto task = std::make_shared<PackedTask>(std::bind(std::forward<FUNCTION>(Function), std::forward<ARGUMENTS>(Arguments)...));
// get the future to return later
auto ret = task->get_future();
{
std::lock_guard<std::mutex> lock{jobsMutex};
jobs.emplace([task]() { (*task)(); });
}
// let a waiting thread know there is an available job
jobsAvailable.notify_one();
return ret;
}
I have few questions regarding the std::packaged_task
.
As you can see in the add(...)
method body, the instance of std::packaged_task
- task
is local variable which scope ends with the end of the method execution. The return value ret
of std::future
type is returned by copy. The value of ret
is given out of the task
(which is local). So once the method execution is finished, the task
goes out of scope and so I expect the connected std::future instance being returned becomes invalid, is my understanding correct?
During the execution of the method the task method to be executed in a thread is emplaced into std::queue<Job> jobs
. Why is only the pointer to operator()
of std::packaged_task
which holds the reference to the Function
given as method argument held in the std::queue
? I would expect to store directly the std::packaged_task
in order to hold the reference to the instance being created...?
Anyway, the source code snippet comes from the ThreadPool implementation which can be found here https://github.com/dabbertorres/ThreadPool and seems to be fully working. So I assume it is correct but unfortunately not completely clear to me... I would be very glad if someone could explain how the stuff works...
Many thanks in advance to anyone willing to help. Cheers Martin
As you can see in the add(...) method body, the instance of std::packaged_task - task is local variable which scope ends with the end of the method execution.
yes, it's a local variable of std::shared_ptr<PackedTask>
. when it gets out of scope, it decrements the reference count by 1. if the new reference count is 0, it deletes the object it points to. luckily, jobs
holds a copy of that shared pointer, so that pointed-object stays alive.
The return value ret of std::future type is returned by copy.
Not exactly. it's more possible that ret
is returned by move rather than by copy.
the task goes out of scope and so I expect the connected std::future instance being returned becomes invalid, is my understanding correct?
Again, task
is a shared pointer. the jobs
queue keeps it alive and probably ret
holds another copy of that shared pointer (to actually pull the result of task
). so nothing becomes invalid as long as the task is in the queue and someone holds the future to that result.
I would expect to store directly the std::packaged_task in order to hold the reference to the instance being created...?
True, it could have stored std::packaged_task
directly. my guess is that the writer of this library didn't want to mess with uncopiable/unmovable functors. if the result of std::bind
is uncopiable and unmovable, you can't really move the std::packaged_task
into the queue. using a shared pointer solves this problem as you don't copy/move the packaged_task itself, only the pointer. you can however build the the task object directly into the queue, but doing it under a held lock is not such a good practice.
I do agree however, that maybe moving task
into the lambda is way more efficient than copying it.