5

I am trying to create a wrapper class for std::thread. This class provides a kick method which starts the thread and calls a pure virtual function. I am using a derived class to call this kick method and derived class also has implemented the virtual function.

class Executor
{
public:
    // constructor
    Executor();

    // destructor
    ~Executor();

    // kick thread execution
    void Kick();

private:
    // thread execution function
    virtual void StartExecution() = 0;

    // thread handle
    std::thread mThreadHandle;
};

Following is the implementation of executor class

Executor::Executor()
{
    // Nothing to be done here
}

Executor::~Executor()
{
    if (mThreadHandle.joinable())
        mThreadHandle.join();
}

void Executor::Kick()
{
    // mThreadHandle = std::thread(&Executor::StartExecution, this);
    mThreadHandle = std::thread([this] {this->StartExecution();});
}

I am using a Consumer class which inherits this class and implements StartExecution method. When i use the kick method it shows pure virtual function called and program terminates.

std::unique_ptr<Consumer> consumer = std::make_unique<Consumer>();
consumer->Kick();

In the executor kick method. I added a breakpoint and started looking what is wrong. It comes to the

mThreadHandle = std::thread([this] {this->StartExecution();});

line twice. First because of the kick method, second to execute the lambda function. The first time I see that this points to Consumer class. But when it comes to the lambda function it is messed up and the vptr is pointing to the pure virtual function.

I would be interested in what is wrong in this instead of simple answers.

theadnangondal
  • 1,546
  • 3
  • 14
  • 28
  • 1
    You have an error in your `Consumer` class. You haven't shown the code, so it's impossible to say more. – Pete Becker Jun 02 '18 at 16:16
  • Can you add the Consumer class please? – Coral Kashri Jun 02 '18 at 16:36
  • Usualy you can only get a `pure virtual function called` when you call a virtual function in a constructor. Do you call the `Kick()` function in a constructor? – kiloalphaindia Jun 02 '18 at 16:41
  • the problem wasnt in the consumer class but basically the way I was using it. Sorry folks I should have added the whole implementation here. But since that was long so i thought that i am not understanding lambda function correctly and added only that part. – theadnangondal Jun 02 '18 at 16:52

2 Answers2

5

Just a guess based on what I've tried: your Consumer gets destructed before the thread executes.

I've made ~Executor virtual and added some print statements for relevant function calls.

#include <iostream>
#include <memory>
#include <thread>
#include <chrono>

class Executor
{
public:
    // constructor
    Executor();

    // destructor
    virtual ~Executor();

    // kick thread execution
    void Kick();

private:
    // thread execution function
    virtual void StartExecution() { std::cout << "Executor::Kick\n"; }

    // thread handle
    std::thread mThreadHandle;
};

Executor::Executor()
{
    // Nothing to be done here
}

Executor::~Executor()
{
    std::cout << "~Executor\n";
    if (mThreadHandle.joinable())
        mThreadHandle.join();
}

void Executor::Kick()
{
    // mThreadHandle = std::thread(&Executor::StartExecution, this);
    mThreadHandle = std::thread([this] {this->StartExecution();});
}


class Consumer: public Executor {
public:
    ~Consumer() {
        std::cout << "~Consumer\n";
    }
private:
    virtual void StartExecution() { std::cout << "Consumer::Kick\n"; }
};

int main() {
    {
        std::cout << "1:\n";
        std::unique_ptr<Consumer> consumer = std::make_unique<Consumer>();
        consumer->Kick();
    }
    {
        std::cout << "2:\n";
        std::unique_ptr<Consumer> consumer = std::make_unique<Consumer>();
        consumer->Kick();
        std::cout << "Sleeping for a bit\n";
        std::this_thread::sleep_for(std::chrono::seconds(1));
    }
    return 0;
}

Output:

1:
~Consumer
~Executor
Executor::Kick
2:
Sleeping for a bit
Consumer::Kick
~Consumer
~Executor

See it run here

Sleeping before destroying the consumer lets the thread run and call the correct function. A "real" solution would be to ensure that the consumer lives at least as long as the thread itself. Since the thread exists in the base class Executor this isn't guaranteed, as derived classes are destructed before base classes.

From cppreference (emphasis mine):

When a virtual function is called directly or indirectly from a constructor or from a destructor (including during the construction or destruction of the class’s non-static data members, e.g. in a member initializer list), and the object to which the call applies is the object under construction or destruction, the function called is the final overrider in the constructor’s or destructor’s class and not one overriding it in a more-derived class. In other words, during construction or destruction, the more-derived classes do not exist.

This appears to apply when a member function is called in a different thread during construction/destruction.

Kevin
  • 6,993
  • 1
  • 15
  • 24
  • Goes to show that it's often a bad idea to rely on a destructor for thread synchronization, because some compiler-generated code in the destructor may already produce a data race before the user code is run. – Arne Vogel Jun 05 '18 at 08:46
1

Thr program goes out of memory even before the thread gets a chance to call the function. If you change your code like this

void Executor::Kick()
{
    mThreadHandle = std::thread([this] {this->StartExecution();});
    this_thread::sleep_for(chrono::seconds(1)); // any number
}

this will work.

That's the precise reason why you can't pass the this by reference in capture list

Now about your specific question

I would be interested in what is wrong in this instead of simple answers.

The vPTR points to VTable and as the class goes out of the memory, the vPTR points to base class VTable and hence this is happening. you can check the same by printing the vTable address before calling the function

Daksh Gupta
  • 7,554
  • 2
  • 25
  • 36