C++ Inheritance : Calling virtual method when it has been overridden

Question

I am trying to build a service object which can run (i.e. execute it's run() function) in a separate thread. This is the service object

#include <boost/noncopyable.hpp>
#include <atomic>
#include <thread>
#include <iostream>

class service : public boost::noncopyable {
 public:
  service() : stop_(false), started_(false) { }

  virtual ~service() {
    stop();
    if (thread_.joinable()) {
      thread_.join();
    }
  }

  virtual void stop() { stop_ = true; }

  virtual void start() {
    if (started_.load() == false) {
      started_ = true;
      thread_ = std::thread([&] () {
        run();
      });
    }
  }

 protected:
  virtual void run() = 0;

  std::atomic<bool> stop_;

  std::atomic<bool> started_;

  std::thread thread_;
};

I am the creating a test class which inherits from this abstract class and is called in the main() function

class test : public service {
 public:
  test() : service() {
    std::cout<< "CTOR" << std::endl;
    start();
  }

  ~test() {
    std::cout<< "DTOR" << std::endl;
  }

 protected:
  void run() override {
    std::cout << "HELLO WORLD" <<std::endl;
  }
};


int main() {
  test test1;
  return 0;
}

Now when I execute this, why do I get an error saying pure virtual function called? The run() function is clearly overridden in the test class. Whats worse is that it runs correctly sometimes?

$ ./a.out
CTOR
DTOR
pure virtual method called
terminate called without an active exception

$ ./a.out
CTOR
DTOR
pure virtual method called
terminate called without an active exception

$ ./a.out
CTOR
DTOR
pure virtual method called
terminate called without an active exception

$ ./a.out
CTOR
DTOR
HELLO WORLD

$ ./a.out
CTOR
DTOR
pure virtual method called
terminate called without an active exception

What could be going wrong here?

Madgemadhouse

you are calling virtual function start(); in the constructor. That's the reason for this error. – Sarpedon

@Sarpedon That's perfectly well-defined here and is not an issue. – Guereza

No, it's not. At this point, the derived object is already constructed. – Reinaldoreinaldos

Answer 1

Follow along, step by step:

1) You construct the object.

2) You execute the following piece of code:

if (started_.load() == false) {
  started_ = true;
  thread_ = std::thread([&] () {
    run();
  });
}

The parent thread immediately returns to main() where it immediately exits and destroys your object.

Here's your bug:

• You are not guaranteed that the thread started in start() is going to reach the call to run(), above, before the parent thread terminates the process. Both the child thread, and the parent thread runs concurrently.

So, every once in a while, the parent thread will destroy the object before the child thread gets in gear, and calls run().

At this point, the object whose run() method gets invoked, is already destroyed.

Undefined behavior.

The assertion you're hitting, every once in a while, is one possible result of this undefined behavior.

Reinaldoreinaldos

Shouldnt the thread_.join() in the base class' destructor ensure that the object is not destroyed before the thread is run? – Madgemadhouse

@subzero There are three steps here: (A) Derived class destructor (B) Thread body executed (C) Base class destructor. (A) definitely happens before (C). But nothing is preventing (A) happening before (B) before (C), which means the vtable is unwound before run() got called before join() got called. – Guereza

@subzero ensure that the object is not destroyed In MT programming, it's your job to have control over the object's lifetime. Leaving it up to the compiler invariably gives problems like this. – Musgrove

That makes sense. Thanks! How do I fix this? I can put a thread_.join() in the derived class destructor. I would like to wrap all the thread handling business in the base class? Can that be done? – Madgemadhouse

@subzero: put the join call in the stop routine rather than the destructor and ensure that the test destructor calls stop. Alternately, make service::run a noop virtual function rather than pure virtual. – Cicatrize

There's not actually undefined behavior here, as the service object won't be destroyed until after join returns. But the test object might be destroyed (turning the object back into a service object) prior to run being called, so you get the "pure virtual" called. – Cicatrize

Making service::run() a noop is still undefined behavior. I'm not aware of any requirement that vtable updates are thread-safe. Besides, the child process can resolve the virtual function, followed by parent process destroying the most-derived class that implements the virtual function, followed by the child process invoking the virtual function in a class that's already destroyed. – Reinaldoreinaldos

@ChrisDodd So you think calling a pure virtual function is defined behaviour? – Imprescriptible