Wednesday, March 4, 2015

C++ virtual destructors

Several times recently in other people's code I have run into a problem with virtual destructors - specifically the destructors are not "virtual."  Now to be fair, this is a mistake I have made myself in the past, so I am not just being overly critical of other people's code.

I think a lot of C++ developers do not understand when or why to use virtual destructors.  And admittedly virtual class functions are one of the more difficult parts of C++ to understand.  So let me simplify it and make it easy.  ALWAYS MAKE YOUR DESTRUCTORS VIRTUAL!  Unless you truly understand what you are doing it is hard to think of a scenario where you would want a non-virtual destructor.  Having virtual destructors will not hurt, but non-virtual destructors will cause problems, usually in the form of resource leaks.  Let us look at an example:
class CBase
{
public:
    CBase() {}
    virtual ~CBase() {printf("Base class");}
};

class CDerived : public CBase
{
public:
    CDerived() {}
    virtual ~CDerived() {printf("Derived class");}
};

This is the simplest possible base and derived class you can have.  Now let us look at 3 different ways of using these classes:
CBase *p1 = new CBase;
delete p1;

CBase *p2 = new CDerived;
delete p2;

CDerived *p3 = new CDerived;
delete p3;

So what happens in this code depending on if CBase and/or CDerived have virtual destructors?

Scenario 1: No virtual destructors
In the first delete the CBase destructor is called, which is correct.  In the second delete only the destructor for CBase is called, which means cleanup for CDerived did not happen (resource leaks).  In the third delete both destructors were called, which is correct.

Scenario 2: CBase has a virtual destructor but not CDerived
In the first delete the CBase destructor is called, which is correct.  In the second and third deletes both destructors were called, which is correct.

Scenario 3: CDerived has a virtual destructor but not CBase
In the first delete the CBase destructor is called, which is correct.  In the second delete it actually results in a heap corruption.  Most of the time this heap corruption will not result in a crash, which is actually worse.  It will lead to random sporadic bugs that are hard to track down.  In the third delete both destructors were called, which is correct.

Scenario 4: Both classes have virtual destructors
In the first delete the CBase destructor is called, which is correct.  In the second and third deletes both destructors were called, which is correct.


Both scenario 2 and 4 work without issue, but 2 is potentially dangerous because another class may derive from CDerived in which case problems will not occur.



Similarly, say you are writing code that extends the functionality of another class outside of your control.  For example, you wish to extend a class from an SDK.  Only derive from a class that has virtual destructors.  I often times forget to check this, which is where I got bit in the past.  Older versions of Microsoft's STL implementation had non-virtual destructors.  But what do you do if you have an SDK class that does not have a virtual destructor?  Simple, instead of deriving your class from that class, make what would be the base class into a class member variable.  For example, instead of doing this:

class CDerived : public CBase
{
public:
    CDerived();
    virtual ~CDerived();

protected:
};

You would do this:
class CDerived
{
public:
    CDerived();
    virtual ~CDerived();

protected: 
    CBase m_base;
};

This is effectively the same thing, but avoids the problems caused by non-virtual destructors.




So to sum up, always make your destructors virtual!  Also, always make sure the classes you derive from have virtual destructors and if not then do not derive from them.  Some compilers will warn you about these conditions, but not all.  I wish more compilers warned about this condition.

Hopefully this helps C++ developers of all skill levels out there.