Reputation: 20898
I want to wrap a call to std::thread constructor (to keep track of all threads running so I can join them or do other things). In this example, the t1
thread gets constructed properly, but the t2
thread doesn't using gcc 4.8.1. However, on Windows (VS2012) it compiles without error and runs without error. Based on the discussion here, this may appear to be a bug in gcc, but arguably it is actually a bug in VS. What is the correct way of doing this?
#include <iostream>
#include <thread>
class A {
public:
void foo(int n ) { std::cout << n << std::endl; }
};
template<class F, class Arg>
std::thread& wrapper(F&& f, Arg&& a)
{
std::thread* t = new std::thread(f,a,100);
return *t;
}
int main()
{
A a;
std::thread t1(&A::foo, &a, 100);
t1.join();
std::thread t2 = wrapper(&A::foo, &a);
t2.join();
return 0;
}
Here is the compiler error
-bash-4.1$ make
g++ -std=c++11 main.cpp -o main
main.cpp: In function ‘int main()’:
main.cpp:23:41: error: use of deleted function ‘std::thread::thread(std::thread&)’
std::thread t2 = wrapper(&A::foo, &a);
^
In file included from main.cpp:2:0:
/opt/rh/devtoolset-2/root/usr/include/c++/4.8.1/thread:125:5: error: declared here
thread(thread&) = delete;
^
make: *** [all] Error 1
I asked the wrong question here and am about to delete it, but because the answers are helpful, I will leave it. The problem was that the Intel icpc 14.0 compiler (not gcc 4.8.1) was throwing the same error regarding bind
as discussed here. And it has nothing to do with the "wrapper" but just calling std::thread with a member function instead of static function.
The complaints about the memory leak are 100% valid, but that was me making an unfortunate simplification to the example (from my real code). The actual code saves the threads in a container and deletes the threads on destruction.
Here is a better example:
#include <iostream>
#include <thread>
class A {
public:
void foo() { }
template<class Function, class Arg>
std::thread* wrapper(Function&& f, Arg&& a)
{
auto t = new std::thread(f,a);
return t;
}
};
int main()
{
A a;
std::thread t1(&A::foo, &a);
t1.join();
std::thread* t2 = a.wrapper(&A::foo, &a);
t2->join();
delete t2;
return 0;
}
And the output for g++
(4.8.1) which works
-bash-4.1$ make CXX=g++
g++ -lpthread -std=c++11 main.cpp -o main
and the output for the Intel compiler icpc
(14.0) which doesn't work
-bash-4.1$ make CXX=icpc
icpc -lpthread -std=c++11 main.cpp -o main
/opt/rh/devtoolset-2/root/usr/include/c++/4.8.1/functional(1697): error: class "std::result_of<std::_Mem_fn<void (A::*)()> (A *)>" has no member "type"
typedef typename result_of<_Callable(_Args...)>::type result_type;
^
detected during:
instantiation of class "std::_Bind_simple<_Callable (_Args...)> [with _Callable=std::_Mem_fn<void (A::*)()>, _Args=<A *>]" at line 1753
instantiation of "std::_Bind_simple_helper<_Callable, _Args...>::__type std::__bind_simple(_Callable &&, _Args &&...) [with _Callable=void (A::*)(), _Args=<A *>]" at line 137 of "/opt/rh/devtoolset-2/root/usr/include/c++/4.8.1/thread"
instantiation of "std::thread::thread(_Callable &&, _Args &&...) [with _Callable=void (A::*)(), _Args=<A *>]" at line 20 of "main.cpp"
/opt/rh/devtoolset-2/root/usr/include/c++/4.8.1/functional(1726): error: class "std::result_of<std::_Mem_fn<void (A::*)()> (A *)>" has no member "type"
typename result_of<_Callable(_Args...)>::type
^
detected during:
instantiation of class "std::_Bind_simple<_Callable (_Args...)> [with _Callable=std::_Mem_fn<void (A::*)()>, _Args=<A *>]" at line 1753
instantiation of "std::_Bind_simple_helper<_Callable, _Args...>::__type std::__bind_simple(_Callable &&, _Args &&...) [with _Callable=void (A::*)(), _Args=<A *>]" at line 137 of "/opt/rh/devtoolset-2/root/usr/include/c++/4.8.1/thread"
instantiation of "std::thread::thread(_Callable &&, _Args &&...) [with _Callable=void (A::*)(), _Args=<A *>]" at line 20 of "main.cpp"
compilation aborted for main.cpp (code 2)
make: *** [all] Error 2
-bash-4.1$
Upvotes: 3
Views: 2208
Reputation: 477408
Your wrapper should be like this:
template<class F, class Arg>
std::thread wrapper(F&& f, Arg&& a)
{
return std::thread(std::forward<F>(f), std::forward<Arg>(a));
}
Upvotes: 5
Reputation: 234614
std::thread
is not copyable. Given that wrapper
returns a std::thread&
, wrapper(&A::foo, &a);
is an lvalue, and thus the initialisation of t2
requires a copy. That's what the compiler error is about.
Sadly, some versions of Visual Studio will happily perform a move here, even though there are no rvalues involved.
Something like std::thread* t = new std::thread(f,a,100); return *t;
is pretty much the same as return *new std::thread(f,a,100);
, and that clearly shows the memory leak operator: *new
. Don't do this.
While not copyable, std::thread
is movable. You just have to make it so that wrapper(&A::foo, &a);
is an rvalue. That can be done simply by writing the function in the most natural style, like so:
template<class F, class Arg>
std::thread wrapper(F&& f, Arg&& a)
{
return std::thread(f, a, 100);
}
This however fails to correctly forward the arguments preserving their value category (as is, it turns rvalues into lvalues), even though it's not a problem for this particular example. In order to have general applicability, the function body should forward the arguments properly, like so: return std::thread(std::forward<F>(f), std::forward<Arg>(a), 100);
.
Upvotes: 1