Multiple inheritance and unique_ptr destruction

Question

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;
};

Answer 1

You should declare your destructors as virtual. Otherwise, if your class D is deleted using a pointer to C, then the ~C() destructor will be called and essential parts of the cleanup will be missed.

Also note that in your second part (not using the unique_ptr) you do some object slicing. It means you are creating a copy of s1 of type D into a new object of class C, hence you can lose the extra information specific to D.

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;
}

---

See also

• When to use virtual destructors?
• What is object slicing?

---

Note

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

Answer 2

Your "this is fine" example does slicing, the vector only contains instances of C, that's why it 'works' but does not do what you expected. The solution is as dkg points out is to use virtual dtors.