Possible Memory Leak Without a Virtual Destructor

Possible memory leak without a virtual destructor?

It might.

Because base does not have a virtual destructor, your code exhibits undefined behavior. Anything might happen. It might appear to work as you expect. It might leak memory. It might cause your program to crash. It might format your hard drive.

A citation was requested. C++11 §5.3.5/3 states that, for a scalar delete expression (i.e., not a delete[] expression):

if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the
static type shall have a virtual destructor or the behavior is undefined.

The static type (base) is different from the dynamic type (derv) and the static type does not have a virtual destructor, so the behavior is undefined.

Why is there no memory leak with non-virtual destructor

Because that is how undefined behaviour works. Memory isn't guaranteed to leak. But neither is it guaranteed to not leak. Any behaviour is possible as far as the language is concerned.

why this is Undefined Behaviour

Because a non-virtual destructor is called through a pointer whose dynamic type is of another (derived) type.

whether anything concretely can be said about the lifetime of the B::b

Well, it is a member of B, so it has the same lifetime as any B object. As for the lifetime of the dynamic B object, we cannot really say much due to the UB.

Would it lead to memory leak when delete base class pointer without virtual destructor?

What you are describing is Undefined Behavior, which means any nasty stuff could go wrong, not just memory leaks.

But in practice, if the inheritance is not virtual, the derived class has no other base classes, and the derived class has no members with non-trivial destructors, you'll probably get the Base::~Base() destructor invoked and then operator delete called on the pointer. The operator delete(void*) function just takes a pointer and frees up all the memory it pointed at, so no "memory leak" there.

Memory leak caused by pointer to derived class

It leaks, and it's Undefined Behaviour.

If your derived class didn't introduce extra heap-allocated variables, then it probably wouldn't but would still be Undefined Behaviour so speculation's pointless.

Unless base has a virtual destructor, deletion of derived objects using a base* will fail to call destructors for the further derived classes and their members and other bases... here name would not be deallocated.

It's said that if the base class destructor is made "virtual", the memory leak problem would be solved. Why?

virtual functions are the way the compiler coordinates calls to derived-class-specific functions from code that only has a pointer or reference to the base class. This is a fundamental aspect of Object Oriented Programming support in C++, and if you don't understand it you should get a good book or online tutorial, but to give a very concise introduction: a virtual function works a bit like having a function pointer in the base class that normally points to the base class's function implementation, but if a derived class constructor runs then the function pointer is overwritten with the address of its own implementation (in practice virtual dispatch tends to use class-specific tables of function pointers for better memory efficiency, but the observable functionality is similar to what I've described). So, your derived destructor won't run when you delete using a pointer with static type of base* unless it's virtual - there's no function-pointer-like indirection coordinated by the derived type that could arrange this.

Missing Virtual Destructor Memory Effects

It certainly can do. Consider:

class A
{
public:
 virtual void func() {}
};

class B : public A
{
public:
   void func() { s = "Some Long String xxxxxx"; }
private:
   std::string s;
   // destructor of B will call `std::string` destructor.
};

A* func(bool b)
{
   if (b)
       return new B;
   return new A;
}

... 
   A* a = func(true);
...
   delete a;

Now, this will create a memory leak, as std::string s in the B object is not freed by A::~A - you need to call B::~B, which will only happen if the destructor is virtual.

Note that this applies to ALL compilers and all runtime systems that I'm aware of (which is all the common ones and some not so common ones).

Edit:

Based on the updated actual question: Memory de-allocation happens based on the allocated size, so if you can GUARANTEE that there NEVER is a single allocation happening because of the construction/use of the class, then it's safe to not have a virtual destructor. However, this leads to interesting issues if a "customer" of the base-class can make his/her own extension classes. Marking derived classes as final will protect against them being further derived, but if the base class is visible in a header-file that others can include, then you run the risk of someone deriving their own class from Base that does something that allocates.

So, in other words, in something like a PImpl, where the Impl class is hidden inside a source file that nobody else derives from, it's plausible to have this. For most other cases, probably a bad idea.

Is this a memory leak of deleting base class pointer, and how to make it right

That is not at all a memory leak. When we delete a base class pointer if the base class pointer is holding the address of a derive class object it will first call derived class destructor if that is declared virtual. That is the reason why we have virtual destructor but not virtual constructor in C++. If we are using a Base class pointer to store the address of various derived classes pointers we should always declare the derived class destructor as virtual so that memory release should done in correct order.

Hope this will help you.

Is it safe to make destructor not virtual, and delete base pointer in special circumstances?

When the destructor is non virtual but there are only primitives types in derived, is it safe to call delete on base class ? (will there be no memory leaks ?)

You might not be aware of it, but these are two different questions.

The latter answer is: no, there won't be any memory leaks for this specific example, but there could be for other examples.

And the reason why is the answer to the former question: no, it is not safe to do this. This constitutes undefined behavior, even if the behavior is well-understood for nearly all compilers—and 'understood' is not synecticy for "is safe to do", just to be clear.

When you write code like delete mynode;, the compiler has to figure out which destructor to call. If the destructor for mynode is not virtual, then it will always use the base destructor, doing whatever the base destructor needs to do, but not whatever the derived destructor needs to do.

In this case, that's not such a big deal: the only thing AVL_Node adds is a locally allocated int variable, which will be cleaned up as part of the same process that cleans up the whole pointer.

But if your code were like this instead:

struct AVL_Node : public BST_Node {
    std::unique_ptr<int> height = std::make_unique<int>();
};

Then this code would definitely cause memory leaks, even though we've expressly used a smart pointer in the construction of the derived object! The smart pointer doesn't save us from the tribulations of deleteing a base pointer with a non-virtual destructor.

And in general, your code could cause any kind of leak, including but not limited to resource leaks, file handle leaks, and so on, if AVL_Node were responsible for other objects. Consider, for example, if AVL_Node had something like this, which is extremely common in certain kinds of graphics code:

struct AVL_Node : public BST_Node {
    int handle;
    AVL_Node() {
        glGenArrays(1, &handle);
    }
    /*
     * Pretend we implemented the copy/move constructors/assignment operators as needed
     */
    ~AVLNode() {
        glDeleteArrays(1, &handle);
    }
};

Your code wouldn't leak memory (in your own code), but it would leak an OpenGL object (and any memory allocated by that object).

What is the rule when declaring a destructor virtual in derived class only ?

If you never plan to store a pointer to the base class, then this is fine.

It's also unnecessary unless you plan to also create further derived instances of the derived class.

So here's the example we'll use for the sake of clarity:

struct A {
    std::unique_ptr<int> int_ptr = std::make_unique<int>();
};

struct B : A {
    std::unique_ptr<int> int_ptr_2 = std::make_unique<int>();
    virtual ~B() = default;
};

struct C : B {
    std::unique_ptr<int> int_ptr_3 = std::make_unique<int>();
    //virtual ~C() = default; // Unnecessary; implied by B having a virtual destructor
};

Now here's all the the code that's safe and unsafe to use with these three classes:

auto a1 = std::make_unique<A>(); //Safe; a1 knows its own type
std::unique_ptr<A> a2 = std::make_unique<A>(); //Safe; exactly the same as a1
auto b1 = std::make_unique<B>(); //Safe; b1 knows its own type
std::unique_ptr<B> b2 = std::make_unique<B>(); //Safe; exactly the same as b1
std::unique_ptr<A> b3 = std::make_unique<B>(); //UNSAFE: A does not have a virtual destructor!
auto c1 = std::make_unique<C>(); //Safe; c1 knows its own type
std::unique_ptr<C> c2 = std::make_unique<C>(); //Safe; exactly the same as c1
std::unique_ptr<B> c3 = std::make_unique<C>(); //Safe; B has a virtual destructor
std::unique_ptr<A> c4 = std::make_unique<C>(); //UNSAFE: A does not have a virtual destructor!

So if B (a class with a virtual destructor) inherits from A (a class without a virtual destructor), but as a programmer, you promise you will never refer to an instance of B with an A pointer, then you have nothing to worry about. So in that case, like my example tries to show, there may be valid reasons to declare the destructor of a derived class virtual while leaving the super class' destructor non-virtual.

When to use virtual destructors?

Virtual destructors are useful when you might potentially delete an instance of a derived class through a pointer to base class:

class Base 
{
    // some virtual methods
};

class Derived : public Base
{
    ~Derived()
    {
        // Do some important cleanup
    }
};

Here, you'll notice that I didn't declare Base's destructor to be virtual. Now, let's have a look at the following snippet:

Base *b = new Derived();
// use b
delete b; // Here's the problem!

Since Base's destructor is not virtual and b is a Base* pointing to a Derived object, delete b has undefined behaviour:

[In delete b], if the static type of the
object to be deleted is different from its dynamic type, the static
type shall be a base class of the dynamic type of the object to be
deleted and the static type shall have a virtual destructor or the
behavior is undefined.

In most implementations, the call to the destructor will be resolved like any non-virtual code, meaning that the destructor of the base class will be called but not the one of the derived class, resulting in a resources leak.

To sum up, always make base classes' destructors virtual when they're meant to be manipulated polymorphically.

If you want to prevent the deletion of an instance through a base class pointer, you can make the base class destructor protected and nonvirtual; by doing so, the compiler won't let you call delete on a base class pointer.

You can learn more about virtuality and virtual base class destructor in this article from Herb Sutter.

---

Related Topics