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

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++

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.