Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++11: std::thread inside a class executing a function member with thread initialisation in the constructor

I'm trying to use std::thread from C++11. I couldn't find anywhere if it is possible to have a std::thread inside a class executing one of its function members. Consider the example below... In my try (below), the function is run().

I compile with gcc-4.4 with -std=c++0x flag.

#ifndef RUNNABLE_H
#define RUNNABLE_H

#include <thread>

class Runnable
{
    public:
        Runnable() : m_stop(false) {m_thread = std::thread(Runnable::run,this); }
        virtual ~Runnable() { stop(); }
        void stop() { m_stop = false; m_thread.join(); }
    protected:
        virtual void run() = 0;
        bool m_stop;
    private:
        std::thread m_thread;
};


class myThread : public Runnable{
protected:
    void run() { while(!m_stop){ /* do something... */ }; }
};

#endif // RUNNABLE_H

I'm getting this error and others: (same error with and without the $this)

Runnable.h|9|error: no matching function for call to ‘std::thread::thread(<unresolved overloaded function type>, Runnable* const)’|

When passing a pointer.

Runnable.h|9|error: ISO C++ forbids taking the address of an unqualified or parenthesized non-static member function to form a pointer to member function.  Say ‘&Runnable::run’|
like image 496
Mooks Avatar asked May 10 '11 21:05

Mooks


2 Answers

Here's some code to mull over:

#ifndef RUNNABLE_H
#define RUNNABLE_H

#include <atomic>
#include <thread>

class Runnable
{
public:
    Runnable() : m_stop(), m_thread() { }
    virtual ~Runnable() { try { stop(); } catch(...) { /*??*/ } }

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

    void stop() { m_stop = true; m_thread.join(); }
    void start() { m_thread = std::thread(&Runnable::run, this); }

protected:
    virtual void run() = 0;
    std::atomic<bool> m_stop;

private:
    std::thread m_thread;
};


class myThread : public Runnable
{
protected:
    void run() { while (!m_stop) { /* do something... */ }; }
};

#endif // RUNNABLE_H

Some notes:

  • Declaring m_stop as a simple bool as you were is horribly insufficient; read up on memory barriers
  • std::thread::join can throw so calling it without a try..catch from a destructor is reckless
  • std::thread and std::atomic<> are non-copyable, so Runnable should be marked as such, if for no other reason than to avoid C4512 warnings with VC++
like image 72
ildjarn Avatar answered Oct 31 '22 21:10

ildjarn


That approach is wrong.

The problem is that while the object is still under construction its type is still not the most derived type, but the type of the constructor that is executing. That means that when you start the thread the object is still a Runnable and the call to run() can be dispatched to Runnable::run(), which is pure virtual, and that in turn will cause undefined behavior.

Even worse, you might run into a false sense of security, as it might be the case that under some circumstances the thread that is being started might take long enough for the current thread to complete the Runnable constructor, and enter the myThread object, in which case the new thread will execute the correct method, but change the system where you execute the program (different number of cores, or the load of the system, or any other unrelated circumstance) and the program will crash in production.

like image 16
David Rodríguez - dribeas Avatar answered Oct 31 '22 19:10

David Rodríguez - dribeas