CODEWITHSUNDEEP
c++memory-leaks
niemiro
niemiro

Reputation: 1838

Why does this cause a memory leak?

Say I have an AbstractBaseClass and a ConcreteSubclass.

The following code creates the ConcreteSubclass and then disposes of it perfectly fine and without memory leaks:

ConcreteSubclass *test = new ConcreteSubclass(args...);
delete test;

However, as soon as I push this pointer into a vector, I get a memory leak:

std::vector<AbstractBaseClass*> arr;
arr.push_back(new ConcreteSubclass(args...));
delete arr[0];

I get fewer memory leaks with delete arr[0]; than with no delete at all, but still some memory gets leaked.

I did plenty of looking online and this seems to be a well understood problem - I'm deleting the pointer to the memory but not the actual memory itself. So I tried a basic dereference... delete *arr[0]; but that just gives a compiler error. I also tried a whole load of other references and dereferences but each just gives either a compiler error or program crash. I'm just not hitting on the right solution.

Now, I can use a shared_ptr to get the job done without a leak just fine (Boost as I don't have C++11 available to me):

std::vector<boost::shared_ptr<AbstractBaseClass>> arr2;
arr2.push_back(boost::shared_ptr<AbstractBaseClass>(new ConcreteSubclass(args...)));

but I can't get the manual method to work. It's not really important - it's perfectly easy to just use Boost, but I really want to know what I'm doing wrong so I can learn from it rather than just move without finding out my error.

I tried tracing through Boost's shared_ptr templates, but I keep getting lost because each function has so many overloads and I'm finding it very difficult to follow which branch to take each time.

I think it's this one:

template<class Y>
explicit shared_ptr( Y * p ): px( p ), pn() // Y must be complete
{
    boost::detail::sp_pointer_construct( this, p, pn );
}

But I keep ending up at checked_array_delete, but that can't be right as that uses delete[]. I need to get down to just

template<class T> inline void checked_delete(T * x)
{
    // intentionally complex - simplification causes regressions
    typedef char type_must_be_complete[ sizeof(T)? 1: -1 ];
    (void) sizeof(type_must_be_complete);
    delete x;
}

And that's just calling delete, nothing special. Trying to follow through all of the reference and dereference operators was a disaster from the start. So basically I'm stuck, and I couldn't figure out how Boost made it work whilst my code didn't. So back to the start, how can I re-write this code to make the delete not leak?

std::vector<AbstractBaseClass*> arr;
arr.push_back(new ConcreteSubclass(args...));
delete arr[0];

Thank you.

Upvotes: 0

Views: 180

Answers (1)

Ben Voigt
Ben Voigt

Reputation: 283624

shared_ptr stores a "deleter", a helper function1 that casts the pointer back to its original type (ConcreteSubclass*) before calling delete. You haven't done this.

If AbstractBaseClass has a virtual destructor, then calling delete arr[0]; is fine and works just as well as delete (ConcreteSubclass*)arr[0];. If it doesn't, then deletion through a base subobject is undefined behavior, and can cause far worse things to happen than memory leaks.

Rule of thumb: Every abstract base class should have a user-declared (explicitly defaulted is ok) destructor that is

  • virtual

OR

  • modified by protected: accessibility

or both.

1 You've found the implementation, it is checked_delete. But it is instantiated with the return type from new -- checked_delete<ConcreteSubclass>(ConcreteSubclass* x). So it is using ordinary delete, but on a pointer to type ConcreteSubclass, and that makes it possible for the compiler to find the right destructor even without the help of virtual dispatch.

Upvotes: 3

Related Questions