Dimitris Dakopoulos Dimitris Dakopoulos - 3 months ago 24
C++ Question

Multiple inheritance and unique_ptr destruction

I have the classic (possible problematic) multiple inheritance diamond scheme.


  • B inherits A

  • C inherits A

  • D inherits C and B



I want to have a
std::vector
that can contain either
C
or
D

objects so I make it as
std::vector<C>
which is
D
's dad and it
works fine.

BUT when I use:
std::vector<std::unique_ptr<C>>
then I have segmentation fault upon the destruction of the vector.

** glibc detected *** ./a.out: free(): invalid pointer: 0x0000000009948018***


Why is there a difference? To me, even the first implementation is problematic.

Code

#include <string>
#include <vector>
#include <memory>

class A
{
public:
A() = default;
};

class B : public virtual A
{
public:
B() = default;
};

class C : public virtual A
{
public:
C() = default;
};

class D : public B, public C
{
public:
D() = default;
};


int main()
{
{ // this crashes
std::vector<std::unique_ptr<C>> v;
std::unique_ptr<D> s1(new D());
v.push_back(std::move(s1));
std::unique_ptr<C> s2(new C());
v.push_back(std::move(s2));
}

{ // this is fine
std::vector<C> v;
D s1;
v.push_back(s1);
C s2;
v.push_back(s2);
}

return 0;
};

dkg dkg
Answer

You should declare your destructors as virtual. Otherwise, if your class D is considered as C and its destructor is called, the C destructor will be called and will operate on D thinking it is a C class.

Also note that in your second part without using the unique_ptr, you do some object slicing. It means you are creating a copy of s1 of type D into a class C, hence you can loose the extra information the class D afford.

Here is the corrected code :

#include <string>
#include <vector>
#include <memory>

class A
{
public:
    A() = default;
    virtual ~A() {};
};

class B : public virtual A
{
public:
    B() = default;
    virtual ~B() {};
};

class C : public virtual A
{
public:
    C() = default;
    virtual ~C() {};
};

class D : public B, public C
{
public:
    D() = default;
    virtual ~D() {};
};


int main()
{
    { // this does not crashe anymore
        std::vector<std::unique_ptr<C>> v;
        std::unique_ptr<D> s1(new D());
        v.push_back(std::move(s1));
        std::unique_ptr<C> s2(new C());
        v.push_back(std::move(s2));
    }

    { // this is fine because you slice D into C, still that fine ?
        std::vector<C> v;
        D s1;
        v.push_back(s1);
        C s2;
        v.push_back(s2);
    }

    return 0;
}

This answer can also fully be applied to your case : When to use virtual destructors?

And for the second part, take a look at object slicing : What is object slicing?


Update :

As @Jarod42 states in the comments, you can only mark A destructor's as virtual, hence every derived class will also have a virtual destructor. Writing it everywhere can make it more explicit. It is a matter of style.