CODEWITHSUNDEEP
c++c++14
user4650542
user4650542

Reputation:

Relying on order of initialisation

According to the C++ 14 standard, non-static member variables are initialised in the order they are declared in a class. The cut down code below relies on this rule to control a thread function.

class foo
{
     foo(): 
          keep_going{true},
          my_thread(&foo::go,this)
     {}

      void go()
      {
          while(keep_going)
             check a std::condition_variable and do some work;
      }
      bool keep_going;
      std::thread my_thread;
}

Note that keep_going is declared before the thread object and should be set to true by the time the thread enters the go function. This is fine and seems to work OK.

However, this is multithreaded code and it pays to be paranoid so I have two questions:

1 Is it safe to rely on the order of initialisation like this? My real object doesn't make sense without the processing thread so I want to set it going in the constructor.

2 Is it unsafe to give code to others when it relies on relatively obscure things like the order of initialisation?

Upvotes: 7

Views: 156

Answers (3)

David Haim
David Haim

Reputation: 26476

Although by the standard it is safe, I wouldn't go with it.

Anecdote: I wrote a customized ThreadPool on Windows OS using visual studio 2013. I declared the thread pool global. Of course by the standard, global objects are destructed after main has returned. the thread pool destructor tried to join each thread, but alas! a dead-lock. (you can read about this problem here: std::thread::join() hangs if called after main() exits when using VS2012 RC). The standard states very clearly that if a thread is joinable, there is no problem joining it, but as you can see, this wasn't perfectly implemented.

Why am I telling you this unrelated issue? Because even compilers and platforms have some bugs. Subtle things may not get implemented 100% correctly at the first few relevant-supporting compiler versions.

This is why I wouldn't go with this idea. As a work-around, I would declare the thread wrapped in std::unique_ptr and initialize it in the constructor body. That way there is no chance it will be initialized before keep_going.

foo(): 
  keep_going{true}
   { my_thread = std::make_unique<std::thread>(&foo::go,this); }

Upvotes: 4

Andrey Nasonov
Andrey Nasonov

Reputation: 2629

I'd like to rewrite the code to make the code obvious even to a monkey.

Another potential problem here may occur when the class foo is the base class. The thread will start on non fully constructed object. What will happen if the constructor of the derived class fails? In this case it is better move thread execution out of the constructor to start() method.

Upvotes: 2

Bathsheba
Bathsheba

Reputation: 234655

  1. Is is safe according to the standard.

  2. Extremely unsafe. Few folk are aware of this, and someone maintaining your header file might reorder the members with disastrous consequences.

I wouldn't rely on it.

Upvotes: 5

Related Questions