Why is std::packaged_task<void()> not valid?

Question

Using MSVC2012,

The following code will compile and run as expected

std::packaged_task< int() > task( []()->int{ std::cout << "hello world" << std::endl; return 0; } );
std::thread t( std::move(task) );
t.join();

while the following code will fail to compile and run

std::packaged_task< void() > task( [](){ std::cout << "hello world" << std::endl; } );
std::thread t( std::move(task) );
t.join();

Why is this so?

Edit: As a workaround, it is possible to use std::promise to get a std::future on a function that returns void

std::promise<void> promise;
auto future = promise.get_future();
std::thread thread( [](std::promise<void> &p){ std::cout << "hello world" << std::endl; p.set_value(); }, std::move(promise) );
future.wait();

Note that there is a bug in the vs2012 library with std::thread that forces you to pass the promise in as a l-value reference and move the promise in, it would not compile if you pass the promise by value or by r-value reference. This is supposedly because the implementation uses std::bind() which does not behave as expected.

Devalue

Interesting...what error does the second one give when compiling? – Cyanocobalamin

This is a bug in MSVC++ probably. – Villiers

From what I traced into their implementation of <future>, it eventually comes down to their storage of function object execution state, specifically in a template class called _State_manager. There is no specialization of _State_manager for void state, which is likley a bug. I could be completely out to lunch too, but thats where it appears everything falls apart. – Exscind

Answer 1

This is a bug in MSVC2012. There are quite a few bugs in the thread library implementation that ships with MSVC2012. I posted a partial list in my blog post comparing it to my commercial Just::Thread library: http://www.justsoftwaresolutions.co.uk/news/just-thread-v1.8.0-released.html

Jeramey

"With the VS2012 library, when std::async is used with a launch policy of std::launch::async, the destructor of the returned std::future instance does not wait for the thread to complete." -- I would make that an option. The argument that the standards text has a bug in it is compelling. – Anthracene

The standard wording for std::async was carefully chosen, and Microsoft deliberately decided to go against the standard. This is not a bug in the standard, though some believe it was a bad choice. – Jeramey

This is disappointing, some of these bugs would have been caught by the simplest of test cases. I expected more from Microsoft. – Devalue

@AnthonyWilliams The committee (notably Herb Sutter, who is obviously affiliated with Microsoft), seems to be discussing whether this behavior is indeed a bug in the standard itself. Herb is pushing for Microsoft's implementation to be the new definition of the standard for C++14: open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3391.html#Herb5 – Lhasa

@BretKuhns I'm well aware of that paper. It was discussed extensively, and there was no consensus for change. I expect Herb will come back with another paper, but it's stalled for now. – Jeramey

@AnthonyWilliams I expected you would. At the least, I thought it was worth noting for others. Microsoft's implementation isn't compliant, but it's preferable IMO, so it would be nice to see the committee do something about that. – Lhasa

Answer 2

This works in gcc 4.7.2:

#include <thread>
#include <future>
#include <iostream>

int main() {
  std::packaged_task< void() > task( [](){ std::cout << "hello world" << std::endl; } );
  std::thread t( std::move(task) );
  t.join();
  std::packaged_task< int() > task2( []()->int{ std::cout << "hello world" << std::endl; return 0; } );
  std::thread t2( std::move(task2) );
  t2.join();
}  

Together with @WhozCraig 's archeology implies that this is probably a bug in MSVC2012.

For a workaround, try using struct Nothing {}; or nullptr_t as your return value?

Anthracene

Answer 3

The problem is still there in MSVS 2013RC, but I made this temporary patch while MS corrects it. It is a specialization of packaged_task for void(...), so I suggest putting this in a header file and including it after the standard headers.Note that make_ready_at_thread_exit() is not implemented and some functions have not been fully tested, use at your own risk.

namespace std {

template<class... _ArgTypes>
class packaged_task<void(_ArgTypes...)>
{
    promise<void> _my_promise;
    function<void(_ArgTypes...)> _my_func;

public:
    packaged_task() {
    }

    template<class _Fty2>
    explicit packaged_task(_Fty2&& _Fnarg)
      : _my_func(_Fnarg) {
    }

    packaged_task(packaged_task&& _Other)
      : _my_promise(move(_Other._my_promise)),
        _my_func(move(_Other._my_func)) {
    }

    packaged_task& operator=(packaged_task&& _Other) {
        _my_promise = move(_Other._my_promise);
        _my_func = move(_Other._my_func);
        return (*this);
    }

    packaged_task(const packaged_task&) = delete;
    packaged_task& operator=(const packaged_task&) = delete;

    ~packaged_task() {
    }

    void swap(packaged_task& _Other) {
        _my_promise.swap(_Other._my_promise);
        _my_func.swap(_Other._my_func);
    }

    explicit operator bool() const {
        return _my_func != false;
    }

    bool valid() const {
        return _my_func != false;
    }

    future<void> get_future() {
        return _my_promise.get_future();
    }

    void operator()(_ArgTypes... _Args) {
        _my_func(forward<_ArgTypes>(_Args)...);
        _my_promise.set_value();
    }

    void reset() {
        swap(packaged_task());
    }
};

}; // namespace std

Azores