CODEWITHSUNDEEP
c++c++11inheritancevectorunique-ptr
Dimitris Dakopoulos
Dimitris Dakopoulos

Reputation: 713

Multiple inheritance and unique_ptr destruction

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

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

Upvotes: 8

Views: 1211

Answers (2)

dkg
dkg

Reputation: 1775

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

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.

Upvotes: 14

Alexander Balabin
Alexander Balabin

Reputation: 2085

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.

Upvotes: 5

Related Questions