shared_ptr polymorphic magic pitfall
Posted: April 24, 2014 Filed under: C++, Pitfall, Software engineering | Tags: C++, Pitfall, Software engineering Leave a comment
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:
#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 2nd and 3rd cases, Derived::~Derived (and thus A::~A) gets called even thou we have a shared_ptr<Base>. The 4th 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 desctructor 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).”
- [Sutter] Herb Sutter/Andrei Alexandrescu. C++ Coding Standards. Ch 50. ISBN-13 978-0-321-11358-0
- [ACCU] Enforcing the Rule of Zero, Listing 4, http://accu.org/var/uploads/journals/Overload120.pdf
- [Arena] Ponder the use of unique_ptr to enforce the Rule of Zero, http://marcoarena.wordpress.com/2014/04/12/ponder-the-use-of-unique_ptr-to-enforce-the-rule-of-zero/
- [Flaming] Rule of Zero, http://flamingdangerzone.com/cxx11/2012/08/15/rule-of-zero.html
END OF POST