20

My programs looks like below

#include <iostream>
#include <thread>

class A {
public:
    void foo(int n ) { std::cout << n << std::endl; }
};

int main()
{
    A a;

    std::thread t1(&A::foo, std::ref(a), 100);

    t1.join();
    return 0;
}

When I compile it using the following command I get errors

g++ -o main main.cc -lpthread -std=c++11

Error:

In file included from /usr/local/include/c++/4.8.2/thread:39:0,
                  from check.cc:2:
/usr/local/include/c++/4.8.2/functional: In instantiation of ‘struct std::_Bind_simple<std::_Mem_fn<void (A::*)(int)>(std::reference_wrapper<A>, int)>’:
/usr/local/include/c++/4.8.2/thread:137:47:   required from ‘std::thread::thread(_Callable&&, _Args&& ...) [with _Callable = void (A::*)(int); _Args = {std::reference_wrapper<A>, int}]’
check.cc:13:42:   required from here
/usr/local/include/c++/4.8.2/functional:1697:61: error:no type named ‘type’ in ‘class std::result_of<std::_Mem_fn<void (A::*)(int)>(std::reference_wrapper<A>, int)>’
        typedef typename result_of<_Callable(_Args...)>::type result_type;
                                                              ^
/usr/local/include/c++/4.8.2/functional:1727:9: error:no type named ‘type’ in ‘class std::result_of<std::_Mem_fn<void (A::*)(int)>(std::reference_wrapper<A>, int)>’
          _M_invoke(_Index_tuple<_Indices...>)
          ^
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
hysteria
  • 203
  • 1
  • 2
  • 4

6 Answers6

25

This isn't the right place for a reference wrapper. A simple pointer suffices, though, and achieves the desired result:

std::thread t1(&A::foo, &a, 100);
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • 6
    +1 For having the correct answer and not filing erroneous bugs on GCC bugzilla like some kind of idiot. – Casey Jan 11 '14 at 15:50
11

EDIT: RETRACTION

Kerrek is correct here: I erroneously assumed that the std::thread constructor and std::bind were by design identical interfaces. However, the automatic conversion of arguments from reference_wrapper<A> to A& is specified for only std::bind in [func.bind.bind]/10:

The values of the bound arguments v1, v2, ..., vN and their corresponding types V1, V2, ..., VN depend on the types TiD derived from the call to bind and the cv-qualifiers cv of the call wrapper g as follows:

  • if TiD is reference_wrapper<T>, the argument is tid.get() and its type Vi is T&;
  • ...

So this particular use of reference_wrapper<A> is not supported by std::thread, but is supported by std::bind. The fact that std::thread behaves identically to std::bind in this instance in other/older compilers is the bug, not the behavior of 4.8 line GCC releases.

I'll leave the incorrect answer here with this explanation in hopes that others won't make this same mistake in the future.

Short (but INCORRECT) answer

This is apparently a bug in the standard library included with GCC 4.8. The code is correctly compiled by:

Long (and also INCORRECT) answer:

The effects of the std::thread constructor

template <class F, class ...Args>
explicit thread(F&& f, Args&&... args);

are detailed in C++11 30.3.1.2 [thread.thread.constr]/4:

The new thread of execution executes

INVOKE(DECAY_COPY(std::forward<F>(f)),
       DECAY_COPY(std::forward<Args>(args))...)

with the calls to DECAY_COPY being evaluated in the constructing thread.

DECAY_COPY is described in 30.2.6 [thread.decaycopy]/1:

In several places in this Clause the operation DECAY_COPY(x) is used. All such uses mean call the function decay_copy(x) and use the result, where decay_copy is defined as follows:

template <class T> typename decay<T>::type decay_copy(T&& v)
{ return std::forward<T>(v); }

In the invocation in the OP std::thread t1(&A::foo, std::ref(a), 100); all three arguments are rvalues that DECAY_COPY will replicate into objects in the new thread's environment before the invocation, whose effect is described in 20.8.2 [func.require]/1:

Define INVOKE(f, t1, t2, ..., tN) as follows:

  • (t1.*f)(t2, ..., tN) when f is a pointer to a member function of a class T and t1 is an object of type T or a reference to an object of type T or a reference to an object of a type derived from T;
  • ((*t1).*f)(t2, ..., tN) when f is a pointer to a member function of a class T and t1 is not one of the types described in the previous item;
  • ...

For the code in the OP, f is a pointer to member function of class A with value &A::foo, t1 is an lvalue reference_wrapper<A> whose stored reference refers to a, and t2 is an int with value 100. The second bullet of 20.8.2/1 applies. Since t1 is a reference_wrapper, *t1 evaluates to the stored reference (per 20.8.3.3/1) and the invocation in the new thread is effectively

(a.*&A::foo)(100);

So yes, the standard describes the behavior of the OP exactly as expected.

EDIT: Oddly, GCC 4.8 correctly compiles the very similar example:

class A {
public:
    void foo(int n) { std::cout << n << std::endl; }
};

int main()
{
    A a;
    auto foo = std::bind(&A::foo, std::ref(a), 100);
    foo();
}
Casey
  • 41,449
  • 7
  • 95
  • 125
  • Sorry, how exactly did you conclude that "`*t1` evaluates to the stored reference"? My 20.8.3.3 doesn't say that... – Kerrek SB Jan 11 '14 at 12:20
  • @KerrekSB Because I was under the erroneous assumption that `*t1` "magically" invokes `operator T&`. – Casey Jan 11 '14 at 15:39
  • Haha, +1 for the fix, and for digging through all the details of `INVOKE`. It's fairly involved, and any kind of systematic exposition is always appreciated. – Kerrek SB Jan 11 '14 at 16:37
10

Regarding your question title I would use a lambda for thread construction. With or without references, via calling member functions or binding parameters.

 std::thread t1([&] { a.foo(100); });
zahir
  • 1,298
  • 2
  • 15
  • 35
  • This seems like a much "cleaner" approach, ie don't expose the parameters or object or even the function to the thread, just give the thread all it wants, which is a `void (*)(void)` (well, that's the easiest way to describe it.) – Mark Lakata May 08 '14 at 00:38
7

GCC 4.8 is correct, std::thread and other components defined in terms of INVOKE must not be implemented in terms of std::bind. They must not invoke nested bind expressions and must use perfect forwarding for bound arguments (rather than forwarding them as lvalues as std::bind does), and additionally as you discovered they do not unwrap reference_wrapper objects. In GCC 4.8 I introduced an internal implementation detail, __bind_simple, for use by std::thread etc. that doesn't have the full std::bind behaviour.

While the other differences from std::bind are desirable, I think the INVOKE operation should still support reference_wrapper objects, so I filed a defect report, see LWG 2219.

Jonathan Wakely
  • 166,810
  • 27
  • 341
  • 521
  • 1
    Would something like `std::thread t(&A::foo, &a, std::ref(b));` also be illegal (assuming a member `A::foo(Bar&)` for example. I get [something similar](http://ideone.com/x2O6eU) to compile on gcc 4.8.2, and I am wondering whether that a bug or the behaviour mandated by the standard. – juanchopanza May 18 '14 at 20:05
  • 2
    @juanchopanza, that is required to work. The defect is that reference_wrappers are not supported for the class object on which you are invoking the member, wrappers work fine for the arguments to the function. – Jonathan Wakely May 19 '14 at 12:29
0

Ok the problem is ref(obj) returns a reference (alias) to an object not a pointer(address)! to work with threads we need pointers not references! See Below a handy program to use function pointers with threads:

    #include <iostream>
    #include "vector"
    #include "string"
    #include "thread"
    #include "atomic"
    #include "functional"

    #include "stdlib.h"
    #include "stdio.h"
    #include "string.h"
    #include "assert.h"

    using namespace std;
    //__________________________Global variables_________________________________________________

    atomic<int> var(0);

    //__________________________class____________________________________________________________

    class C
    {
    public:

        C()
        {}

        static void addition (int a, int b)
        {
            for(int i= 0; i< a+b; i++)
                var++;
        }

        void subtraction (int a, int b)
        {
            for(int i= 0; i< a+b; i++)
                var--;
        }
    };

    class D : std::atomic<int>
    {
    public:
        D() : std::atomic<int>(0)
        {}

        void increase_member (int n)
        {
            for (int i=0; i<n; ++i)
                fetch_add(1);
        }

        int get_atomic_val()
        {
            return this->load();
        }
    };

    //________________________________functions________________________________________________

    void non_member_add (int a, int b)
    {
        for(int i= 0; i< a+b; i++)
            var++;
    }

    //__________________________________main____________________________________________________

    int main ()
    {
        int a=1, b=5;

    // (I)...........................................static public member function (with no inheritance).........................................

        void (* add_member_func_ptr)(int,int) = C::addition;            // pointer to a static public member function

        //defining thread pool for ststic public member_add_ptr

        vector<thread> thread_pool;

        for (int i=0; i<5; i++)
        {
            thread_pool.push_back(thread(add_member_func_ptr,a,b));
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"static public member function (with no inheritance)\t"<<var<<endl;

        //defining thread pool for ststic public member function

        var=0;

        thread_pool.clear();

        for (int i=0; i<5; i++)
        {
            thread_pool.push_back(thread(C::addition,a,b));             //void (* add_member_func_ptr)(int,int) is equal to C::addition
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"static public member function (with no inheritance)\t"<<var<<endl;

    // (II)..............................................non-static public member function (with no inheritance)...................................

        C bar;

        void (C::* sub_member_func_ptr)(int,int) = & C::subtraction;            // pointer to a non-static public member function

        var=0;

        //defining thread pool for non-ststic public member function

        thread_pool.clear();

        for (int i=0; i<5; i++)
        {
            thread_pool.push_back(thread(sub_member_func_ptr,bar,a,b));
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"non-static public member function (with no inheritance)\t"<<var<<endl;

        var=0;

        //defining thread pool for non-ststic public member function

        thread_pool.clear();

        for (int i=0; i<5; i++)
        {
            thread_pool.push_back(thread(&C::subtraction,bar,a,b));         //void (C::* sub_member_func_ptr)(int,int) equals & C::subtraction;
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"non-static public member function (with no inheritance)\t"<<var<<endl;


    // (III)................................................non-member function .................................................

        void (* non_member_add_ptr)(int,int) = non_member_add;              //pointer to a non-member function

        var=0;

        //defining thread pool for non_member_add

        thread_pool.clear();

        for (int i=0; i<5; i++)
        {
            thread_pool.push_back(thread(non_member_add,a,b));
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"non-member function\t"<<var<<endl<<endl;

    // (IV)...........................................non-static public member function (with inheritance).........................

        D foo;

        void (D::* member_func_ptr) (int) = & D::increase_member;                  //pointer to a non-static public member function of a derived class

        //defining thread pool for non-ststic public member function of a derived class

        thread_pool.clear();

        for (int i=0; i<5; i++)
        {
            thread_pool.push_back(thread(member_func_ptr,&foo,10));                 //use &foo because this is derived class!
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"non-static public member function (with inheritance)\t"<<foo.get_atomic_val()<<endl;

        //defining thread pool for non-ststic public member function

        D poo;

        thread_pool.clear();

        for (int i=0; i<5; i++)
        {
            reference_wrapper<D> poo_ref= ref(poo);

            D& poo_ref_= poo_ref.get();             //ref(obj) returns a reference (alias) to an object not a pointer(address)!

            D* d_ptr= &poo;                         //to work with thread we need pointers not references!


            thread_pool.push_back(thread(&D::increase_member, d_ptr,10));             //void (D::* member_func_ptr) (int) equals & D::increase_member;
        }

        for(thread& thr: thread_pool)
            thr.join();

        cout<<"non-static public member function (with inheritance)\t"<<poo.get_atomic_val()<<endl<<endl;


        return 0;
    }
MCH
  • 453
  • 5
  • 7
0

Just wanted to add that i got the same error just by giving incompatible arguments to std::bind/std::thread. Like giving a pointer to a base class when a more specific pointer was in the signature of the actual function.

Hugo Maxwell
  • 723
  • 5
  • 13