9

Following this excellent tutorial for futures, promises and packaged tasks I got to the the point where I wanted to prepare my own task

#include <iostream>
#include <future>
using namespace std;

int ackermann(int m, int n) {   // might take a while
    if(m==0) return n+1;
    if(n==0) return ackermann(m-1,1);
    return ackermann(m-1, ackermann(m, n-1));
}

int main () {
    packaged_task<int(int,int)> task1 { &ackermann, 3, 11 }; // <- error
    auto f1 = task1.get_future();
    thread th1 { move(task1) };                              // call
    cout << "  ack(3,11):" << f1.get() << endl;
    th1.join();
}

As far as I can decipher the gcc-4.7.0 error message it expects the arguments differently? But how? I try to shorten the error message:

error: no matching function for call to 
  'std::packaged_task<int(int, int)>::packaged_task(<brace-enclosed initializer list>)'
note: candidates are:
  std::packaged_task<_Res(_ArgTypes ...)>::---<_Res(_ArgTypes ...)>&&) ---
note:   candidate expects 1 argument, 3 provided
  ...
note:   cannot convert 'ackermann'
  (type 'int (*)(int, int)') to type 'std::allocator_arg_t'

Is my variant how I provide the parameters for ackermann wrong? Or is it the wrong template parameter? I do not give the parameters 3,11 to the creation of thread, right?

Update other unsuccessful variants:

packaged_task<int()> task1 ( []{return ackermann(3,11);} );
thread th1 { move(task1)  };

packaged_task<int()> task1 ( bind(&ackermann,3,11) );
thread th1 { move(task1)  };

packaged_task<int(int,int)> task1 ( &ackermann );
thread th1 { move(task1), 3,11  };

hmm... is it me, or is it the beta-gcc?

Nathan Kleyn
  • 5,103
  • 3
  • 32
  • 49
towi
  • 21,587
  • 28
  • 106
  • 187

2 Answers2

21

Firstly, if you declare std::packaged_task to take arguments, then you must pass them to operator(), not the constructor. In a single thread you can thus do:

std::packaged_task<int(int,int)> task(&ackermann);
auto f=task.get_future();
task(3,11);
std::cout<<f.get()<<std::endl;

To do the same with a thread, you must move the task into the thread, and pass the arguments too:

std::packaged_task<int(int,int)> task(&ackermann);
auto f=task.get_future();
std::thread t(std::move(task),3,11);
t.join();
std::cout<<f.get()<<std::endl;

Alternatively, you can bind the arguments directly before you construct the task, in which case the task itself now has a signature that takes no arguments:

std::packaged_task<int()> task(std::bind(&ackermann,3,11));
auto f=task.get_future();
task();
std::cout<<f.get()<<std::endl;

Again, you can do this and pass it to a thread:

std::packaged_task<int()> task(std::bind(&ackermann,3,11));
auto f=task.get_future();
std::thread t(std::move(task));
t.join();
std::cout<<f.get()<<std::endl;

All of these examples should work (and do, with both g++ 4.6 and MSVC2010 and my just::thread implementation of the thread library). If any do not then there is a bug in the compiler or library you are using. For example, the library shipped with g++ 4.6 cannot handle passing move-only objects such as a std::packaged_task to std::thread (and thus fails to handle the 2nd and 4th examples), since it uses std::bind as an implementation detail, and that implementation of std::bind incorrectly requires that the arguments are copyable.

Anthony Williams
  • 66,628
  • 14
  • 133
  • 155
  • 3
    Looking at the requirements for `std::bind`, it's not true that the arguments are required to be copyable. The paragraph is too long to paste here, it's 20.8.9.1.2 Function template bind [func.bind.bind] paragraph 5. The actual requirements are that the stored types be move constructible and constructible from the passed arguments. Although I do remember a faulty implementation shipping with GCC that did require CopyConstructible. (Not the problem the OP is facing however.) – Luc Danton Sep 26 '11 at 01:11
  • The code works, but I have a question in the line `std::unique_lock l(td.m);` isn't the mutex in the thread already locked when it is checking the condition variable? I think I am missing something there. I thought it locks the mutex in the thread and then waits for the stop flag or for work to be added to the queue. In this case wouldn't the main thread keep waiting for the lock to the released? Is this because of a spurious wake up that this works? – bjackfly Nov 03 '13 at 21:18
  • @bjackfly: I am not sure which code you are referring to, but a condition variable wait unlocks the mutex for the duration of the wait, then relocks it on waking, before checking the predicate. – Anthony Williams Nov 04 '13 at 09:43
  • Thanks @AnthonyWilliams I don't see the code I was referring to now, must have been looking at a couple of posts at once and posted in response to this one maybe mistakenly. This answer helps though. I love the book by the way really great stuff. – bjackfly Nov 10 '13 at 17:55
3

Since you're starting the thread with no arguments, you expect the task to be started with no arguments, as if task1() were used. Hence the signature that you want to support is not int(int, int) but int(). In turn, this means that you must pass a functor that is compatible with this signature to the constructor of std::packaged_task<int()>. Try:

packaged_task<int()> task1 { std::bind(&ackermann, 3, 11) };

Another possibility is:

packaged_task<int(int,int)> task1 { &ackermann };
auto f1 = task1.get_future();
thread th1 { move(task1), 3, 11 };

because the constructor of std::thread can accept arguments. Here, the functor you pass to it will be used as if task1(3, 11) were used.

Luc Danton
  • 34,649
  • 6
  • 70
  • 114
  • @towi 'No' to what? A compilation error again? What is the error? – Luc Danton Sep 25 '11 at 21:12
  • Yes, different errors. I think the closest I got (shortest error list) was: `packaged_task task1 ( bind(ack,3,11) ); thread th1 { move(task1) };` with error: `tuple:274:17: error: 'constexpr std::_Tuple_impl<### = std::_Tuple_impl<0ul, std::packaged_task >]' declared to take const reference, but implicit declaration would take non-const` – towi Sep 25 '11 at 21:15
  • 1
    @towi I can reproduce the error on my end with a snapshot of GCC 4.7. Since the problem appears when doing e.g. `std::make_tuple(std::packaged_task {})` I believe this is a bug in the implementation of `std::tuple` (the error is pretty explicit, too). I don't think writing a functor will help much, passing the `std::packaged_task` to either `std::bind` or `std::thread` is problematic right now. – Luc Danton Sep 25 '11 at 21:27
  • Thank you. I too have a svn snapshot from last week. I will leave it there right now and will come back to that in a month or so. Don't you think the version `pkg_task – towi Sep 25 '11 at 21:44
  • @towi I don't see an appropriate constructor to make that valid, and you'd still want the signature to be `int()`, not `int(int, int)`. Also I have to warn you that I've rolled up my own `std::packaged_task`-like class since several months ago since there were already problems back then. There's not telling when that will work. – Luc Danton Sep 25 '11 at 21:51