8

recently in a job interview I was asked about the problem of leaking memory in derived classes when the base class's destructor is not declared virtual.

I wrote a small test to confirm my answer, but I found something interesting. Obviously, if you create a Derived object via new but store its pointer as a Base*, the derived object's destructor won't be called, if the pointer is deleted (so much for my answer to the question).

I thought whether the derived class's destructor is virtual or not is irelevant in that case, but on my system the following code shows otherwise:

#include <iostream>
#include <string>

// just a helper class, printing its name out when it is destructed
class PrintOnDestruct
{
    public:
        PrintOnDestruct( const std::string& name )
        : name_( name )
        {}

        ~PrintOnDestruct()
        {
            std::cout << "Destructing: " << name_ << std::endl;
        }

    protected:

        std::string name_;
};

// the Base class
class Base
{
    public:
        Base()
        {
            print_on_destruct_ = new PrintOnDestruct( "Base" );
        }

        // the destructor is NOT virtual!
        ~Base()
        {
            delete print_on_destruct_;
        }

    protected:

        PrintOnDestruct* print_on_destruct_;

};

// the NonVirtualDerived class, doesn't have a virtual destructor either
class NonVirtualDerived : public Base
{
    public:
        NonVirtualDerived()
        : Base()
        {
            print_on_destruct_child_ = new PrintOnDestruct( "NonVirtualDerived" );
        }

        // the destructor is NOT virtual!
        ~NonVirtualDerived()
        {
            delete print_on_destruct_child_;
        }

    protected:

        PrintOnDestruct* print_on_destruct_child_;

};

// the VirtualDerived class does have a virtual destructor 
class VirtualDerived : public Base
{
    public:
        VirtualDerived()
        : Base()
        {
            print_on_destruct_child_ = new PrintOnDestruct( "VirtualDerived" );
        }

        // the destructor is virtual!
        virtual ~VirtualDerived()
        {
            delete print_on_destruct_child_;
        }

    protected:

        PrintOnDestruct* print_on_destruct_child_;

};

int main()
{
    // create the two child classes
    Base* non_virtual_derived = new NonVirtualDerived;
    Base* virtual_derived = new VirtualDerived;

    // delete the two objects
    delete non_virtual_derived; // works as expected (only calls Base's destructor, the memory of NonVirtualDerived will be leaked)
    delete virtual_derived; // segfault, after calling Base's destructor

    return 0;
}

I would have expected the program to output the following two lines and quit normally:

Destructing: Base
Destructing: Base

I get that output, but immediately after the second line the program quits with a segmentation fault. And the message:

*** Error in `...': free(): invalid pointer: 0x00000000006020e8 ***

I've changed the order of the two calls to delete, but the programm would always segfault in the call to delete virtual_derived;. Can anybody tell me why this is so?

  • Try changing the Base* assignments to use *static_cast* instead of implicit cast, and see if anything changes. Probably not, but Still better to be explicit in case like this. – hyde Oct 23 '13 at 12:58
  • 2
    It's just undefined behaviour to delete through a pointer to base class wit non-virtual destructor. – jrok Oct 23 '13 at 12:59
  • 1
    @hyde Using _static_cast_ doesn't change anything. – Sebastian Schneider Oct 23 '13 at 13:46
  • @SebastianSchneider Yeah, the implicit cast should be equal to explicit `static_cast`. Though in hindsight, as is explained in the current top answer, needing that cast (implicit or explicit) with virtual methods (or multiple inheritance) indicates that pointer values may be different depending on type. – hyde Oct 23 '13 at 13:55

2 Answers2

8

The answer really lies in the statement:

    Base* virtual_derived = new VirtualDerived;

You are trying to 'free' an address that was not returned by 'malloc'. To understand why, replace this line with

    VirtualDerived* x = new VirtualDerived;
    Base* virtual_derived = x;

If you print these two addresses, you will notice that 'x' and 'virtual_derived' have different values. The address that 'malloc' returned (via 'new') is 'x' and the address that was passed to 'free' (via 'delete') is 'virtual_derived'.

satyag
  • 96
  • 2
  • You are right, the addresses are in fact different. I tried the same for NonVirtualDerived, but in that case the addresses stay the same. Where can I find information about why the pointer changes in the first case? – Sebastian Schneider Oct 23 '13 at 13:37
  • Could you explain why the addresses are different? I'm curious. Thanks – Manu343726 Oct 23 '13 at 13:39
  • 1
    it changes because of vtable added to `VirtualDerived` at the beginning of allocated memory. So `Base*` should point after vtable. – Andriy Tylychko Oct 23 '13 at 13:44
  • @AndyT Thanks for the explanaition. I can confirm that the `Base*` is pointing further. In one test I had 0xad30e0 (VirtualDerived*) vs. 0xad30e8 (Base*) – Sebastian Schneider Oct 23 '13 at 13:53
  • 1
    @SebastianSchneider: now we all know you're on x64 system :) – Andriy Tylychko Oct 23 '13 at 13:54
  • But if we try to call some function from the base class (after defining it) via the virtual_derived pointer, there is no crash: virtual_derived->some_base_func(); // OK -- why? What's the difference between the destructor call and general function call in this case? – trig-ger Mar 17 '16 at 15:18
2

You have to declare the destructor in the base class as virtual, which you do not do in this example. Declaring it as virtual in the derived class only is not sufficient.

Crowman
  • 25,242
  • 5
  • 48
  • 56
  • 1
    This does not explain segfault. – hyde Oct 23 '13 at 12:59
  • 1
    @hyde: There's no point talking about the segfault until the undefined behavior is rectified. – Crowman Oct 23 '13 at 13:00
  • 1
    The answer to the question seems to be just that "Undefined Behaviour". – hyde Oct 23 '13 at 13:01
  • The blanket statement that a virtual dtor in the base is required is false. This is completely acceptable as long as objects are destroyed through a pointer to either their dynamic type, or to a base type with a virtual dtor. The UB is invoked only in the deletion. – Simon Richter Oct 23 '13 at 13:27
  • 2
    I know, that the whole problem is fixed, if the base class destructor is made virtual, but I'm explicitly exploring the problem here what happens if it's not. If the answer is, that making the destructor not virtual in a base class, leads to undefined behavior, that's okay. But then I'd like to know where I can find that definition. – Sebastian Schneider Oct 23 '13 at 13:39