1

I saw an interview question a couple of days ago regarding memory leaks in c++. The code was like (if I remember correctly):

#include <iostream>
using namespace std;

class super {
    int value;
    int arr[1000];
public:
    super() :value(0) {}
    super(int value) :value(value) {}
    virtual int getValue() const{
        return this->value;
    }
};

class sub : public super {
    int val;
    super sup;
    vector<int> v1;
public:
    sub() :val(0), sup(0) {}
    sub(int value) :val(value), sup(value), v1(10,0) {}
    int getValue() const{
        return this->val;
    }
};

int main() {
    sub* pt1 = new(sub);
    super* pt2 = pt1;

    pt1 = new(sub);

    delete pt2;     //memory leak ??

    //more code here...
    delete pt1;
    return 0;
}

The question was how to avoid this type of memory leaks at implementation-design level. I guess the question was more than simply answering "do not use pointers like that".

Does it have to do with implementing destructor as virtual or using dynamic cast? How can we implement destructors so that delete pt2 wont create any memory leaks? Can anyone further analyze this example?

Thanks in advance.

BugShotGG
  • 5,008
  • 8
  • 47
  • 63
  • 3
    I don't see how this leaks memory. You are deleting the first `sub` object via a pointer to its base class, which should be fine because `sub` does not hold any resources. If it did, you'd have to declare a virtual destructor in `super`. – Roland W Mar 07 '16 at 12:13
  • I guess the virtual destructor is missing here see http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors – kwarnke Mar 07 '16 at 12:15
  • @RolandW Yes I do not remember exactly the code of the example. Feel free to edit the code in the question so that sub class can hold resources. – BugShotGG Mar 07 '16 at 12:17
  • Just change `ar` to a vector. When casting sub to super and delete it, super dtor won't call sub dtor which won't call ar/vector dtor which won't free dynamic memory -> memory leak. – Youka Mar 07 '16 at 12:20
  • @Youka Is it ok now? – BugShotGG Mar 07 '16 at 12:28
  • `sup` wasn't necessary. For a minimal example, you could strip even more. To present your problem, both classes could be empty except some sort of dynamic memory in `sub`. – Youka Mar 07 '16 at 12:37

4 Answers4

2

Firstly, delete pt2; isn't specifically a memory leak. I initially said that it is undefined behavior and so anything is permitted by the standard, including a memory leak. But on closer inspection of the classes, actually for the first version of your code with int arrays, sub was trivially destructible and therefore, strange as it may seem, this code was correct. Then you changed the code to make sub no longer trivially destructible (due to the vector data member), so it's undefined behavior now.

The person who set the question might have been looking for a specific answer, and if so I don't know what that is, but this error could be "designed away" in more than one way:

  1. Make the destructor of super virtual, so that deleting a sub through the parent pointer works. A common rule of thumb is to say that classes with virtual functions should always have virtual destructors, because such classes are designed to be used via pointers/references to the base class.
  2. In the user code, use the RAII technique to ensure correct destruction. In this case, smart pointers. shared_ptr lets you write shared_ptr<super> pt2(new sub()); and the object will be deleted correctly even with a non-virtual destructor, although some consider that feature obscure.
  3. It's difficult to call this an "implementation design" decision, since "don't make grievous coding errors" isn't a design rule, it's a language rule. But the calling code could be "designed" not to delete objects through pointers to their base classes except where it's valid to do so (which for this class it is not). Classes can help with this by consistent documentation.
  4. In the classes super and sub, use a vector<int> rather than a plain int array. This makes the objects themselves much smaller, storing most of the data externally, so that for uses like this example, users don't feel they have to dynamically allocate the objects but rather can put them on the stack (not that 1000 ints necessarily can't go on the stack, just that it's a size that might make users nervous). So if you make the same change to super that you did to sub, then the calling code can reduce the risk of this kind of error by making it a design principle to use automatic variables instead of dynamic allocation wherever possible.
Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
  • I think I messed up the question. The question that I saw was definitely for memory leak issue but here I wrote the code by memory so it might be a bit wrong. – BugShotGG Mar 07 '16 at 12:53
1

It leaks memory because super doesn't have a virtual destructor (virtual ~super() = default;).

Now when you call delete on the super* that points to a sub, sub's destructor is not called, leaking its resources.

Always declare base class destructors virtual if any class derived from it has any resources to deallocate.

Emil Laine
  • 41,598
  • 9
  • 101
  • 157
0

This is not a classic memory leak. A classic memory leak would have a memory allocation and no deallocation. Your new/delete pairs actually match up.

This is undefined behavior. You see a rather lengthy explanation here why having a public but non-virtual destructor creates problems.

With their specific compiler, this might leak memory because the compiler did the best it could to behave sane in the face of undefined behavior. Another compiler or another version of their compiler might blow up or create pink dancing unicorns. It's not a good example of a memory leak because a memory leak was the best the compiler could produce from a way worse situation.

Community
  • 1
  • 1
nvoigt
  • 75,013
  • 26
  • 93
  • 142
0

If you ask for a solution at design level the discussion can get very wide.
And I believe you do not really what to discuss how to implement destructor.

The design level discussion on how to avoid 'memory leaks' has already been done by the most experts in the C++ field: Stroustrup and Sutter.

I suggest to view their presentation videos and read their article.

Matteo
  • 482
  • 3
  • 11