Getting use of pure virtual function error when trying to push unique_ptrs to a vector

Question

So I have an abstract class, called MyClassParent, which MyClass inherits from. I run the following code:

        for(auto e:elements){
            MyClass m = *this;
            MyClass * mpointer = &m;
            if(mpointer->xLargerthanY(x,y)){
                    rv.push_back(unique_ptr<MyClassParent>(mpointer));
                    if(!rv[0]->solved()) cout<<"true";//works as expected
            }
        }
        rv[0]->solved();//gives pure virtual function called error

What's strange is that rv[0]->solved() inside the for each loop works as expected, returns true if the object has x greater than y. But if I call the function from outside the for each loop, I get a pure virtual function called error, which should never happen since I override solved() in the child class. I suspect this has something to do with the unique_ptr function, as my solved method makes no changes to the object and only return true of false.

I have tested this with many other methods, they all work inside the for each loop, but as soon as I exit it, I get the pure virtual function called error.

Answer 1

rv[0]->solved();//gives pure virtual function called error

Of course it does. Your program has undefined behavior, so it can do anything. It's also fairly easy to distill that snippet into what's causing the problem:

MyClass *ptr;
{
  MyClass m;
  ptr = &m;
}
ptr->solved();

Once we get rid of all of those red herrings, we see that all the pointers in your rv container point to objects with automatic storage durations, that have since gone out of scope. Using them to access that object is just going to behave in some undefined manner.

If you want to have rv store owning pointers to copies of this, then create those copies with dynamic storage duration

for(auto e:elements){
    MyClass& m = *this; // Assuming we need the reference binding
    if(m.xLargerthanY(x,y)){
        rv.push_back(make_unique<MyClass>(m)); 
    }
}

And now everything points at valid objects.

Answer 2

Ok, let's start with a little introduction because you seem to not have a grasp on some things needed to understand smart pointers:

Automatic storage duration: the lifetime of an object is managed by the compiler. Its lifetime is defined by the scope of the variable associated.

E.g.:

{
    X x; // lifetime of x starts here
    // ....

} // lifetime of x ends here

Dynamic storage duration: the lifetime of an object is managed by the programmer. It starts with a call to new and ends with a call to delete (this is simplified a bit).

E.g.:

auto foo(X* x)
{
    delete x; // lifetime ends with delete
}


{
    X* x = new X{}; // lifetime starts with new

    foo(x);
}

In C++ you should never explicitly call new / delete and use smart pointers instead.

unique_ptr (unless specified otherwise) on destruction will automatically call delete on the pointer it helds. This is the reason it must be provided with a pointer to a dynamic object i.e. allocated with new. This is one of your problems.

X x;

std::unique_ptr<X> p{&x};
// p receives a pointer to an automatic storage duration
// this is 100% wrong. The destructor for x would be called twice
// once as part of the automatic lifetime of x
// and then as part of the destructor of p
// resulting in UB

this is what you do here:

MyClass m = ...;
MyClass * mpointer = &m;
unique_ptr<MyClassParent>(mpointer);
// unique_ptr receives a pointer to an automatic storage duration object

As if that weren't enough the other problem you have is that you access a dangling pointer.

The scope of m is within the for. The vector holds pointers to such objects and those objects go out of scope after each iteration. When you do rv[0] you access an object whose lifetime has ended. Undefined behavior again.

I hope you have a better understanding of what unique_ptr does and what problem it solves. The solution is - as Storry Teller showed - to use make_unique.

What make_unique does: it calls new and creates an unique_ptr from that pointer returned by new. You could do this by hand yourself but you shouldn't because of other problems: Differences between std::make_unique and std::unique_ptr

Answer 3

As pointed by @StoryTeller this is undefined behavior, but let me explain why it behaves the way it does in this case. Since it is undefined behavior, there is no guarantee it will behave this way in different compilers or systems, but I will explain why there is a good chance it will:

    for(auto e:elements){
        MyClass m = *this;
        MyClass * mpointer = &m;
        if(mpointer->xLargerthanY(x,y)){
                rv.push_back(unique_ptr<MyClassParent>(mpointer));
                if(!rv[0]->solved()) cout<<"true";//works as expected
        }
    }
    rv[0]->solved();//gives pure virtual function called error

Here

    for(auto e:elements){
        MyClass m = *this;
        ....
    }

A pointer to m is stored into the rv vector. But when m exists scope, the object is being destroyed. The code implicitly calls MyClass::~MyClass(), which in the end replaces the virtual table of the object. First the derived class is being destroyed, and at the last step of this destruction the virtual table is replaced so that the object no has the virtual table of the base. In the base, solved() is pure virtual, so calling:

rv[0]->solved();

so calling this function finds the definition of the base, only. The derived object no longer exists, so it can't be used. In this case, in the base class, resolved() is pure virtual, and has no body. That's why it crashes the way it does. If you had a non-virtual resolved() in the base, then there is a good chance that you'd have a different crash, since the object had been destroyed already.

Note that even if this code did not crash, then later on, when rv is destroyed things become messy. The pointers inside rv point to the stack, but std::unique_ptr calls delete and assumes that the object is on the heap. Either that will crash immediately, since this is an illegal pointer, the heap/stack will be trashed, or it will be simply ignored. The exact behavior is unknown, since this is also undefined behavior.

Here is a simpler case when you'd get a similar problem:

class Base
{
    public:
            virtual ~Base() { bar(); }
            virtual void foo() = 0;
            void bar() { foo(); }
};
class Derived: public Base
{
    public:
            void foo() override { };
};
int main()
{
    Derived b;
}