5
votes

I would like to use my own Thread implementation by wrapping the std::thread class from C++11 so I will be able handle exceptions like I want.

Here is my wrap class:

#include <Types.hpp>
#include <thread>
#include <exception>
#include <functional>

class Thread
{
    private:

        std::exception_ptr exceptionPtr;
        std::thread thread;

    public:

        using Id = std::thread::id;

        using NativeHandleType = std::thread::native_handle_type;

        Thread() noexcept = default;
        Thread(Thread &&t) noexcept :
            exceptionPtr(std::move(t.exceptionPtr)),
            thread(std::move(t.thread))
        {
        }

        Thread &operator =(Thread &&t) noexcept
        {
            exceptionPtr = std::move(t.exceptionPtr);
            thread = std::move(t.thread);
            return *this;
        }

        template<typename Callable, typename... Args>
        Thread(Callable &&f, Args &&... args) :
            exceptionPtr(nullptr),
            thread([&](Callable &&f, Args &&... args)
            {
                try
                {
                    std::once_flag flag;
                    std::call_once(flag, f, args...);
                }
                catch (...)
                {
                    exceptionPtr = std::current_exception();
                }

            }, f, args...)
        {
            if (exceptionPtr != nullptr)
            {
                 std::rethrow_exception(exceptionPtr);
            }
        }

        bool joinable() const noexcept
        {
            return thread.joinable();
        }

        void join()
        {
            thread.join();
        }

        void detach()
        {
            thread.detach();
        }

        Id getId() const noexcept
        {
            return thread.get_id();
        }

        NativeHandleType nativeHandle()
        {
            return thread.native_handle();
        }

        static uint32_t hardwareConcurrency() noexcept
        {
            return std::thread::hardware_concurrency();
        }

        static void wait(Time t)
        {
            std::this_thread::sleep_for(t);
        }
};

It works pretty well if there is no argument:

Thread([&]() {  /* do something */ }).detach();

... but if I try to pass variadic arguments:

Thread(&GUI::refreshTask, this, refreshDelay).detach();

... I get an error at compile time:

buildroot-2014.02/output/host/usr/i586-buildroot-linux-uclibc/include/c++/4.8.2/functional: In instantiation of 'struct std::_Bind_simple)(std::chrono::duration >); Args = {CRH::GUI const, std::chrono::duration >&}]::__lambda1(void (CRH::GUI::)(std::chrono::duration >), CRH::GUI, std::chrono::duration >)>': buildroot-2014.02/output/host/usr/i586-buildroot-linux-uclibc/include/c++/4.8.2/thread:137:47: required from 'std::thread::thread(_Callable&&, _Args&& ...) [with _Callable = CRH::Thread::Thread(Callable&&, Args&& ...) [with Callable = void (CRH::GUI::)(std::chrono::duration >); Args = {CRH::GUI const, std::chrono::duration >&}]::__lambda1; _Args = {void (CRH::GUI::&)(std::chrono::duration >), CRH::GUI const&, std::chrono::duration >&}]' /home/cyril/Documents/crh-2016/src/robot2/../core/Thread.hpp:72:30: required from 'CRH::Thread::Thread(Callable&&, Args&& ...) [with Callable = void (CRH::GUI::)(std::chrono::duration >); Args = {CRH::GUI const, std::chrono::duration >&}]' src/core/GUI.cpp:90:57: required from here buildroot-2014.02/output/host/usr/i586-buildroot-linux-uclibc/include/c++/4.8.2/functional:1697:61: error: no type named 'type' in 'class std::result_of)(std::chrono::duration >); Args = {CRH::GUI const, std::chrono::duration >&}]::__lambda1(void (CRH::GUI::)(std::chrono::duration >), CRH::GUI, std::chrono::duration >)>' typedef typename result_of<_Callable(_Args...)>::type result_type; ^ buildroot-2014.02/output/host/usr/i586-buildroot-linux-uclibc/include/c++/4.8.2/functional:1727:9: error: no type named 'type' in 'class std::result_of)(std::chrono::duration >); Args = {CRH::GUI const, std::chrono::duration >&}]::__lambda1(void (CRH::GUI::)(std::chrono::duration >), CRH::GUI, std::chrono::duration >)>' _M_invoke(_Index_tuple<_Indices...>)

It could be a little clearer... but it will be too demanding for GCC.

Any idea how to fix this issue?

Solution

#include <Types.hpp>
#include <thread>
#include <exception>
#include <functional>

class Thread
{
    private:

        std::exception_ptr exceptionPtr;
        std::thread thread;

    public:

        using Id = std::thread::id;

        using NativeHandleType = std::thread::native_handle_type;

        Thread() noexcept = default;
        Thread(Thread &&t) noexcept :
            exceptionPtr(std::move(t.exceptionPtr)),
            thread(std::move(t.thread))
        {
        }

        Thread &operator =(Thread &&t) noexcept
        {
            exceptionPtr = std::move(t.exceptionPtr);
            thread = std::move(t.thread);
            return *this;
        }

        template<typename Callable, typename... Args>
        Thread(Callable &&f, Args &&... args) :
            exceptionPtr(nullptr),
            thread([&](typename std::decay<Callable>::type &&f, typename std::decay<Args>::type &&... args)
            {
                try
                {
                    std::bind(f, args...)();
                }
                catch (...)
                {
                    exceptionPtr = std::current_exception();
                }

            }, std::forward<Callable>(f), std::forward<Args>(args)...)
        {
        }

        bool joinable() const noexcept
        {
            return thread.joinable();
        }

        void join()
        {
            thread.join();

            if (exceptionPtr != nullptr)
            {
                std::rethrow_exception(exceptionPtr);
            }
        }

        void detach()
        {
            thread.detach();
        }

        Id getId() const noexcept
        {
            return thread.get_id();
        }

        NativeHandleType nativeHandle()
        {
            return thread.native_handle();
        }

        static uint32_t hardwareConcurrency() noexcept
        {
            return std::thread::hardware_concurrency();
        }

        static void wait(Time t)
        {
            std::this_thread::sleep_for(t);
        }
};
1
Does it work if you replace thread([&](Callable &&f, Args &&... args) by thread([&](typename std::decay<Callable>::type&& f, typename std::decay<Args>::type&&... args) ?Piotr Skotnicki
Names that begin with an underscore followed by a capital letter or that contain two consecutive underscores (__THREAD_HPP) are reserved to the implementation. Don't use them.Pete Becker
Complete rules for what @PeteBecker pointed out.Baum mit Augen♦
It compiles using std::decay thanks. While testing I found something probably very normal: std::call_once call the Callable object only with the args passed the first time and ignore the orhers :S I've never had issue with the define.didil
Most of your error message is mangled.Lightness Races in Orbit

1 Answers

4
votes

Callable and Args are forwarding references, thus template argument deduction can make them either lvalue references or plain types, depending on the value category of argument expressions.

This means that when you reuse the deduced types in the declaration of a lambda:

thread([&](Callable&& f, Args&&... args)

reference collapsing comes into play and, for an lvalue argument refreshDelay, Args becomes an lvalue reference.

However, std::thread stores decayed-copies of the arguments it receives, and then it moves from its internal storage to an actual handler, turning the stored objects into xvalues. This is what the error tells you: the handler is not callable with the arguments the thread tries to pass in.

Instead, you can implement it as follows:

template <typename Callable, typename... Args>
Thread(Callable&& f, Args&&... args)
    : exceptionPtr(nullptr)
    , thread([] (typename std::decay<Callable>::type&& f
               , typename std::decay<Args>::type&&... args)
            {
                // (...)
            }
            , std::forward<Callable>(f), std::forward<Args>(args)...)
{
    // (...)
}