Pass parameters to std::thread wrapper

Question

I want to implement a small thread wrapper that provides the information if a thread is still active, or if the thread has finished its work. For this I need to pass the function and its parameters to be executed by the thread class to another function. I have a simple implementation that should work but cannot get it to compile, and I can't figure out what to do to make it work.

Here is my code:

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

class ManagedThread
{
public:
   template< class Function, class... Args> explicit ManagedThread( Function&& f, Args&&... args);
   bool isActive() const { return mActive; }
private:
   volatile bool  mActive;
   std::thread    mThread;
};

template< class Function, class... Args>
   void threadFunction( volatile bool& active_flag, Function&& f, Args&&... args)
{
   active_flag = true;
   f( args...);
   active_flag = false;
}

template< class Function, class... Args>
   ManagedThread::ManagedThread( Function&& f, Args&&... args):
      mActive( false),
      mThread( threadFunction< Function, Args...>, std::ref( mActive), f, args...)
{
}

static void func() { std::cout << "thread 1" << std::endl; }

int main() {
   ManagedThread  mt1( func);
   std::cout << "thread 1 active = " << std::boolalpha << mt1.isActive() << std::endl;
   ::sleep( 1);
   std::cout << "thread 1 active = " << std::boolalpha << mt1.isActive() << std::endl;

   return 0;
}

The compiler error I get:

In file included from /usr/include/c++/5/thread:39:0,
                 from prog.cpp:4:
/usr/include/c++/5/functional: In instantiation of 'struct std::_Bind_simple<void (*(std::reference_wrapper<volatile bool>, void (*)()))(volatile bool&, void (&)())>':
/usr/include/c++/5/thread:137:59:   required from 'std::thread::thread(_Callable&&, _Args&& ...) [with _Callable = void (&)(volatile bool&, void (&)()); _Args = {std::reference_wrapper<volatile bool>, void (&)()}]'
prog.cpp:28:82:   required from 'ManagedThread::ManagedThread(Function&&, Args&& ...) [with Function = void (&)(); Args = {}]'
prog.cpp:35:28:   required from here
/usr/include/c++/5/functional:1505:61: error: no type named 'type' in 'class std::result_of<void (*(std::reference_wrapper<volatile bool>, void (*)()))(volatile bool&, void (&)())>'
       typedef typename result_of<_Callable(_Args...)>::type result_type;
                                                             ^
/usr/include/c++/5/functional:1526:9: error: no type named 'type' in 'class std::result_of<void (*(std::reference_wrapper<volatile bool>, void (*)()))(volatile bool&, void (&)())>'
         _M_invoke(_Index_tuple<_Indices...>)
         ^

Live example is available here: https://ideone.com/jhBF1q

I don't have much experience with forwarding arguments, but I just somehow get a feeling you are missing a [`std::forward`](http://en.cppreference.com/w/cpp/utility/forward) call or two... You might want to read ["When to use std::forward to forward arguments?"](http://stackoverflow.com/questions/7257144/when-to-use-stdforward-to-forward-arguments). — Some programmer dude, Jan 20 '17 at 07:22
Also note that you might not get the result you are expecting. The thread you create might start, run and finish before you call `mt1.isActive()` the first time. Consider adding a short sleep in the thread function, then a longer sleep in the `main` function. — Some programmer dude, Jan 20 '17 at 07:29
I thought so too, but each std::forward call I added actually made it worse, i.e. resulted in more compiler errors. Maybe if the current problem is solved I will need (and be able to) apply std::forward calls in my function, but first I need to get the thread constructor call to work. — Rene, Jan 20 '17 at 07:32
Regarding the timing: You are right, but that would not be a real problem. Important is to get the `isActive() == false` information once the thread finished. — Rene, Jan 20 '17 at 07:33
Lastly another little thought about your design: Why not make `threadFunction` a *member* function of your wrapper? Then you don't need to pass the `mActive` flag as an argument to the function. — Some programmer dude, Jan 20 '17 at 07:35
@Someprogrammerdude I did not do that because I did not want to use `this` in the initializer list of the constructor ... (Actually I now tried that, but basically I get the same error message in the end). — Rene, Jan 20 '17 at 07:55
I applied O'Niel's answer and it compiled, built and ran in my IDE MS Visual Studio 2017 on an Intel Quad Core Extreme running Win7 64bit compiled as x86. However after exiting the program I did get a `debug` error. I don't know if this is concern to you are not... However; I added a simple destructor to the `ManageThread` class and added this line of code `mThread.detach();` into the destructor and that fixed or took care of the debug error and I was able to successfully exit the application without any errors. — Francis Cugler, Dec 28 '17 at 02:54
When I applied DeiDei's answer I got the same results with the debug error while exiting the application. I had to once again call `mThread.detach();` in the class's destructor, even with the lambda approach. Just something to be aware of. — Francis Cugler, Dec 28 '17 at 03:00

O'Neil · Accepted Answer · 2017-12-28T00:39:52.000

8

In the error message, you can see the difference void (*)() vs void (&)(). That's because std::thread's constructor parameters are std::decayed.

Add also std::ref to f:

template< class Function, class... Args>
   ManagedThread::ManagedThread( Function&& f, Args&&... args):
      mActive( false),
      mThread( threadFunction< Function, Args...>, std::ref(mActive), std::ref(f), std::forward<Args>(args)...)
{
}

edited Dec 28 '17 at 00:39

answered Jan 20 '17 at 07:49

O'Neil

3,790
4
16
30

Thank you, this compiles! Great! At the moment I get 'terminate called without an active exception', but that's another problem I have to look into. I read that about the decay'ed parameter, but I did not realize in which way this affects my function. And I will have to look how `std::forward< Args>( args)...` works ... – Rene Jan 20 '17 at 08:04
See [@jotik's answer](http://stackoverflow.com/a/41758753/3893262) for terminate call explanation and correction. – O'Neil Jan 20 '17 at 08:09

jotik · Answer 2 · 2017-01-20T08:14:27.973

O'Neil and DeiDei got here first, and they're correct as far as I can tell. However, I'm still posting my solution to your problem.

Here's something that would work better:

#include <atomic>
#include <thread>
#include <utility>

class ManagedThread {

public: /* Methods: */

    template <class F, class ... Args>
    explicit ManagedThread(F && f, Args && ... args)
        : m_thread(
            [func=std::forward<F>(f), flag=&m_active](Args && ... args)
                    noexcept(noexcept(f(std::forward<Args>(args)...)))
            {
                func(std::forward<Args>(args)...);
                flag->store(false, std::memory_order_release);
            },
            std::forward<Args>(args)...)
    {}

    bool isActive() const noexcept
    { return m_active.load(std::memory_order_acquire); }

private: /* Fields: */

    std::atomic<bool> m_active{true};
    std::thread m_thread;

};

It makes use of lambdas instead, and correctly uses std::atomic<bool> instead of volatile to synchronize the state, and also includes the appropriate noexcept() specifiers.

Also note, that the underlying std::thread is not joined or detached properly before destruction, hence leading to std::terminate() being called.

I rewrote the test code as well:

#include <chrono>
#include <iostream>

int main() {
    ManagedThread mt1(
        []() noexcept
        { std::this_thread::sleep_for(std::chrono::milliseconds(500)); });
    std::cout << "thread 1 active = " << std::boolalpha << mt1.isActive()
              << std::endl;
    std::this_thread::sleep_for(std::chrono::seconds(1));
    std::cout << "thread 1 active = " << std::boolalpha << mt1.isActive()
              << std::endl;
}

Thank you too for your solution. I did not use atomics on purpose, since only 1 thread changes the value of the flag and the other just reads it. — Rene, Jan 20 '17 at 08:09
Regarding termination: Oh, right, of course, I forgot the destructor in which I call join(). — Rene, Jan 20 '17 at 08:12
@Rene you still need atomics for that. Afaik `volatile` is not enough in the general case. See [this answer](http://stackoverflow.com/a/29633332/3919155). — jotik, Jan 20 '17 at 08:12

score 5 · Answer 3 · edited May 23 '17 at 12:32

5

The answer by @O'Neil is correct, but I would like to offer a simple lambda approach, since you've tagged this as C++14.

template<class Function, class... Args>
ManagedThread::ManagedThread(Function&& f, Args&&... args):
      mActive(false),
      mThread([&] /*()*/ { // uncomment if C++11 compatibility needed
        mActive = true;
        std::forward<Function>(f)(std::forward<Args>(args)...);
        mActive = false;
      })
{}

This would eliminate the need for an external function all together.

edited May 23 '17 at 12:32

Community

1
1

answered Jan 20 '17 at 07:56

DeiDei

10,205
6
55
80

Thank you, I will try this version too! – Rene Jan 20 '17 at 08:01
1

This has a subtle error in that the constructor can return before the thread begins executing and forwards `f` and `args`, causing a use-after-free stack error. – geometrian Feb 23 '19 at 08:31

score 2 · Answer 4 · answered Feb 25 '19 at 02:08

The following is simple, elegant, and to my knowledge the most-correct of all current answers (see below):

template < class Function, class... Args >
ManagedThread::ManagedThread( Function&& fn, Args&&... args ) :
    mActive(false),
    mThread(
        [this]( auto&& fn2, auto&&... args2 ) -> void {
            mActive = true;
            fn2(std::forward<Args>(args2)...);
            mActive = false;
        },
        std::forward<Function>(fn), std::forward<Args>(args)...
    )
{}

The answer by DeiDei has a subtle but important flaw: the lambda takes the stack variables by reference, so if the thread starts, and tries to use, the stack variables after the constructor returns, you can get a stack-use-after-free error. Indeed, compiling with -fsanitize=address is typically sufficient to demonstrate the problem.

The answer by O'Neill does not work when e.g. any arguments are lvalues, and is a bit clunky.

The answer by jotik is close, but does not work when the arguments are lvalues either, nor if they are member functions.

Pass parameters to std::thread wrapper

4 Answers4

Linked