shared_ptr Polymorphic Magic Pitfall

Gabor Fekete

Rate me:

5.00/5 (1 vote)

24 Apr 2014CPOL3 min read

9.8K

shared_ptr Polymorphic Magic Pitfall

CodeProject
Pitfall in the polymorphic “magic” of std::shared_ptr

In [Sutter] we read:

“If A is intended to be used as a base class, and if callers should be able to destroy polymorphically, then make A::~A public and virtual. Otherwise make it protected (and not-virtual).”

Then in [ACCU], the author writes that if you use shared_ptr for polymorphic destruction, then you may omit the virtual destructor. The reason for this can be read from [Arena].

In short, through it’s template code, the shared_ptr remembers the pointer type used during construction. For example, if you say “shared_ptr<Base>{new Derived{}}” then shared_ptr will internally store a Derived*. If you say “shared_ptr<Base>{new Base{}}” then it stores a Base*. Then when the shared_ptr is destructed, it calls delete on the stored pointer. Naturally, with non-virtual destructors, for Base* it will call Base::~Base and for Derived* it will call Derived::~Derived.

If you are not careful and blindly follow the example in [ACCU], there is a nasty surprise waiting for you!
Let’s take the following example:

C++

#include <iostream>
#include <memory>

struct A
{
    ~A() { std::cout << __FUNCTION__ << std::endl; }
};

struct Base
{
    // No virtual dtor!
};

struct Derived : public Base
{
    Derived() : m_pA{new A{}} {}
    A m_A;
    std::unique_ptr<A> m_pA;
};

int main()
{
    {
        std::cout << "\nDelete Derived via Base* : A::~A() NOT CALLED" << std::endl;
        Base *p = new Derived{};
        delete p;
    }
    {
        std::cout << "\nDelete Derived via shared_ptr<Base> 
        with Derived* : A::~A() called" << std::endl;
        std::shared_ptr<Base> sp{new Derived{}};
    }
    {
        std::cout << "\nDelete Derived via make_shared<Derived> 
        : A::~A() called" << std::endl;
        std::shared_ptr<Base> sp{std::make_shared<Derived>()};
    }
    {
        std::cout << "\nDelete Derived via shared_ptr<Base> with Base* 
        : A::~A() NOT CALLED" << std::endl;
        Base *p = new Derived{};
        std::shared_ptr<Base> sp{p};
    }
    return 0;
}

Base does not have a virtual destructor, so if we delete a Derived pointer via Base then A::~A will not be called. Classic memory leak candidate.
The above code gives the following output:

Delete Derived via Base* : A::~A() NOT CALLED

Delete Derived via shared_ptr<Base> with Derived* : A::~A() called
~A
~A

Delete Derived via make_shared<Derived> : A::~A() called
~A
~A

Delete Derived via shared_ptr<Base> with Base* : A::~A() NOT CALLED

The first call is as expected, since Base has non-virtual destructor, A is not destructed. On the other hand, via the “magic” of shared_ptr, in the 2^nd and 3^rd cases, Derived::~Derived (and thus A::~A) gets called even though we have a shared_ptr<Base>. The 4^th example has a surprise: although we use again a shared_ptr with a pointer to a Derived object, shared_ptr calls Base::~Base because it was initialized with a Base*. In this case, shared_ptr cannot see that the provided Base* pointer actually points to a Derived object.

shared_ptr<Base> calls Derived::~Derived only if it is constructed directly with a pointer of type Derived*. If you construct shared_ptr<Base> with a Base*, then it will not call Derived::~Derived, it will call ~Base::Base. The magic does not happen!

Also, in [Flaming] we read this:

“Classes that have custom destructors, copy/move constructors or copy/move assignment operators should deal exclusively with ownership. Other classes should not have custom destructors, copy/move constructors or copy/move assignment operators.”

But notice in our example above that neither Base nor Derived manage resources. Derived uses only self-destructing objects internally. So one might think that there is no ownership issue here, so we erroneously decide not to provide a “custom (virtual) destructor”. So the above should read like this:

“Classes that have custom destructors, copy/move constructors or copy/move assignment operators should deal exclusively with ownership. Other classes should not have custom destructors, copy/move constructors or copy/move assignment operators except for [Sutter].” :)

Conclusion

No matter how “smart” your pointer to Derived is and no matter if you use everywhere only self-destructing objects (e.g. RAII), use a virtual destructor for polymorphic deletion!

Again, as stated in [Sutter]:

“If A is intended to be used as a base class, and if callers should be able to destroy polymorphically, then make A::~A public and virtual. Otherwise make it protected (and not-virtual).”