2

I'm looking to implement an "interruptible" thread in C++11 along the lines of this answer.

I've parameterized my interruptible class so that it can wrap any class of thread that has a similar constructor (intended for std::thread, but should also work for something like boost::thread (nevermind boost::thread already has this functionality)). That part shouldn't matter.

Despite the linked answer having a few issues I had to correct, it makes good sense to me, so I'd like to figure out where I'm going wrong.

I've included the relevant source and the results.

interruptible.hpp

#include <atomic>
#include <exception>
#include <thread>

class interrupted_exception : public virtual std::exception
{
public:
    char const *what() const noexcept { return "interrupted"; }
};


template <typename T>
class interruptible
{
public:
    template <typename F, typename... A>
    interruptible(F&& Function, A&&... Arguments) :
        Interrupted(false),
        Thread(
            [](std::atomic_bool *Interrupted, F&& Function, A&&... Arguments)
            {
                LocalInterrupted = Interrupted;
                Function(std::forward<A>(Arguments)...);
            },
            &this->Interrupted,
            std::forward<F>(Function),
            std::forward<A>(Arguments)...
        )
    { }

    void interrupt()            { this->Interrupted = true; }
    bool interrupted() const    { return this->Interrupted; }

    T *operator->()             { return &this->Thread; }

    static inline void check() noexcept(false)
    {
        if (!interruptible::LocalInterrupted)
            return;

        if (!interruptible::LocalInterrupted->load())
            return;

        throw interrupted_exception();
    }

private:
    static thread_local std::atomic_bool *LocalInterrupted;

    std::atomic_bool    Interrupted;
    T                   Thread;
};

template <typename T>
thread_local std::atomic_bool *interruptible<T>::LocalInterrupted = nullptr;

main.cpp

#include <iostream>
#include <unistd.h>
#include <thread>

#include "interruptible.hpp"

void DoStuff()
{
    try
    {
        while (true)
        {
            std::cout << "Loop" << std::endl;

            sleep(1);

            interruptible<std::thread>::check();
        }
    }
    catch (interrupted_exception const &e)
    {
        std::cout << "Interrupted!" << std::endl;
    }
}


int main()
{
    interruptible<std::thread> a(DoStuff);

    sleep(2);

    std::cout << "Interrupting..." << std::endl;
    a.interrupt();

    sleep(2);

    a->join();

    return 0;
}

When I compile this with g++ -std=c++11 main.cpp (gcc 4.9.2), I get:

/usr/include/c++/4.9.2/functional:1665:61: error: no type named ‘type’ in ‘class std::result_of<interruptible<T>::interruptible(F&&, A&& ...) [with F = void (&)(); A = {}; T = std::thread]::<lambda(std::atomic_bool*, void (&)())>(std::atomic_bool*, void (*)())>’
       typedef typename result_of<_Callable(_Args...)>::type result_type;
                                                             ^
/usr/include/c++/4.9.2/functional:1695:9: error: no type named ‘type’ in ‘class std::result_of<interruptible<T>::interruptible(F&&, A&& ...) [with F = void (&)(); A = {}; T = std::thread]::<lambda(std::atomic_bool*, void (&)())>(std::atomic_bool*, void (*)())>’
         _M_invoke(_Index_tuple<_Indices...>)
         ^

Any light that anyone can shed on this problem would be very appreciated!

Community
  • 1
  • 1
cdbfoster
  • 339
  • 4
  • 13
  • 1
    Clean up your code: that error can be generated from 90% less code. Minimal, complete examples, not "paste my code and ask how to fix it". – Yakk - Adam Nevraumont Jan 03 '15 at 05:38
  • I will try to condense it to just the error. My first attempt was unsuccessful, though; if you know exactly how to generate this error, any help would be appreciated. – cdbfoster Jan 03 '15 at 06:09
  • Minimized: http://coliru.stacked-crooked.com/a/9794a4bf605b4b21 – T.C. Jan 03 '15 at 07:21
  • If you agree that @t.c. simplification reflects the problem well, modify the question to match it. Remember, we care about the error more than the system. – Yakk - Adam Nevraumont Jan 03 '15 at 13:58

1 Answers1

2

std::thread stores decayed copies of the arguments passed in to its constructor. A reference to function undergoes a function-to-pointer decay, which implies that for the DoStuff argument the actual type passed to the lambda is void(*)(), while its operator() still expects the argument of type before the decay happened, which is void(&)() (deduced F). The same problem will take place for any other argument from the A parameter pack as soon as you will try to use them.

Passing objects wrapped in std::ref is not an option in this context, since the reference wrapper may outlive the lifetime of the instance stored it in (you don't actually know what are Arguments and Function, these might be prvalue temporaries whose lifetime ends after calling the constructor of interruptible). It might be reasonable for someone to decide to wrap some instances in std::ref while calling the constructor of interruptible, when one knows the lifetime of certain object, but this is not this case here.

If you want to minimize the number of copies and moves necessary to transfer the arguments from interruptible to a lambda expression and then further - to the Function object - an option is to rewirte your constructor as follows:

#include <type_traits>
// ...
template <typename F, typename... A>
interruptible(F&& Function, A&&... Arguments) :
    Interrupted(false),
    Thread(
        [](std::atomic_bool *Interrupted,
           typename std::decay<F>::type&& Function,
           typename std::decay<A>::type&&... Arguments)
        {
            LocalInterrupted = Interrupted;
            Function(std::move(Arguments)...);
        },
        &this->Interrupted,
        std::forward<F>(Function),
        std::forward<A>(Arguments)...
    )
{ }
Piotr Skotnicki
  • 46,953
  • 7
  • 118
  • 160
  • Now the thread function only takes in rvalues, making the use of `std::forward` unnecessary. Is this your intention? – David G Jan 03 '15 at 16:09
  • @0x499602D2 `std::forward` allows the thread's constructor to copy/move-construct the arguments utilizing perfect-forwarding by restoring the value category of expressions used in ctor call. then it `std::move()s` those copies to the function object (lambda in this case), so they can be bound by rvalue references which are then `std::move` to `Function`. that is intentional, compare [forward-version](http://coliru.stacked-crooked.com/a/599983bc2db3eae6) and [non-forward-version](http://coliru.stacked-crooked.com/a/19187538d4018342) – Piotr Skotnicki Jan 03 '15 at 16:18
  • So internally they are moved into the thread function's arguments? – David G Jan 03 '15 at 23:40
  • @0x499602D2 thread object calls the functor *as-if* it was `INVOKE(DECAY_COPY(std::forward(f)), DECAY_COPY(std::forward(args))...)`, which implies that: 1. these are copies that utilize perfect-forwarding, 2. functor's arguments are always rvalues – Piotr Skotnicki Jan 04 '15 at 11:19
  • If the functor's arguments are always rvalues how can they bind to lvalue-references? – David G Jan 06 '15 at 01:21
  • @0x499602D2 they can't be bound by non-const lvalue references, that's the point (unless one used `std::ref`, but the reference_wrapper itself is an rvalue that extracts a non-const lvalue reference) – Piotr Skotnicki Jan 06 '15 at 07:55