views:

4030

answers:

5

I know it is a good practice to declare virtual destructors for base classes in C++, but is it always important to declare virtual destructors even for abstract classes that function as interfaces? Please provide some reasons and examples why.

+34  A: 

It's even more important for an interface. Any user of your class will probably hold a pointer to the interface, not a pointer to the concrete implementation. When they come to delete it, if the destructor is non-virtual, they will call the interface's destructor (or the compiler-provided default, if you didn't specify one), not the derived class's destructor. Instant memory leak.

For example

class Interface
{
   virtual void doSomething(void) = 0;
};

class Derived : public Interface
{
   Derived(void);
   ~Derived(void) 
   {
      // Do some important cleanup...
   }
};

void myFunc(void)
{
   Interface* p = new Derived();
   delete p; // calls Interface::~Interface, not Derived::~Derived
}
Airsource Ltd
+4  A: 

Yes it is always important. Derived classes may allocate memory or hold reference to other resources that will need to be cleaned up when the object is destroyed. If you do not give your interfaces/abstract classes virtual destructors, then every time you delete a derived class instance via a base class handle your derived class' destructor will not be called.

Hence, you're opening up the potential for memory leaks

class IFoo
{
  public:
    virtual void DoFoo() = 0;
};

class Bar : public IFoo
{
  char* dooby = NULL;
  public:
    virtual void DoFoo() { dooby = new char[10]; }
    void ~Bar() { delete [] dooby; }
};

IFoo* baz = new Bar();
baz->DoFoo();
delete baz; // memory leak - dooby isn't deleted
OJ
True, in fact in that example, it may not just memory leak, but possibly crash :-/
Evan Teran
Why would it crash?
OJ
+2  A: 

Kevin, first of all, please don't take my jumping on your comments in our previous discussion personally, I didn't intend to come off so harshly. Anyway, the main answer to the question.

It is not always required, but I find it to be good practice. What it does, is allows a derived object to be safely deleted through a pointer of a base type.

So for example: boost::shared_pointer<Base> p(new Derived); is illegal if Base doesn't have a virtual destructor, because it will attempt to delete the object as if it were a Base *.

Evan Teran
No offense taken. I thought you made some good points and thought a more direct question would provide the same help to someone else. Thanks.
Kevin
No problem, it's the beauty of stack overflow, everyone gets a chance to be both a student and a teacher :)
Evan Teran
you made some good points :)
Johannes Schaub - litb
don't you want to fix boost::shared_pointer p(new Derived) to look like boost::shared_pointer<Base> p(new Derived); ? maybe ppl will understand your answer then and vote
Johannes Schaub - litb
EDIT: "Codified" a couple of parts to make the angle brackets visible, as litb suggested.
j_random_hacker
+2  A: 

It's not only good practice. It is rule #1 for any class hierarchy.

  1. The base most class of a hierarchy in C++ must have a virtual destructor

Now for the Why. Take the typical animal hierarchy. Virtual destructors go through virtual dispatch just as any other method call. Take the following example.

Animal* pAnimal = GetAnimal();
delete pAnimal;

Assume that Animal is an abstract class. The only way that C++ knows the proper destructor to call is via virtual method dispatch. If the destructor is not virtual then it will simply call Animal's destructor and not destroy any objects in derived classes.

The reason for making the destructor virtual in the base class is that it simply removes the choice from derived classes. Their destructor becomes virtual by default.

JaredPar
I *mostly* agree with you, because *usually* when defining a hierarchy you want to be able to refer to a derived object using a base class pointer/reference. But that's not *always* the case, and in those other cases, it may suffice to make the base class dtor protected instead.
j_random_hacker
@j_random_hacker making it protected won't protect you from incorrect internal deletes
JaredPar
@JaredPar: That's right, but at least you can be responsible in your own code -- the hard thing is to make sure that *client code* can't cause your code to explode. (Similarly, making a data member private doesn't prevent internal code from doing something stupid with that member.)
j_random_hacker
@j_random_hacker, sorry to respond with a blog post but it really fits this scenario. http://blogs.msdn.com/jaredpar/archive/2008/03/24/part-of-being-a-good-programmer-is-learning-not-to-trust-yourself.aspx
JaredPar
@JaredPar: Excellent post, I agree with you 100%, especially about checking contracts in retail code. I just mean that there are cases when you *know* you don't need a virtual dtor. Example: tag classes for template dispatch. They have 0 size, you only use inheritance to indicate specialisations.
j_random_hacker
@j_random_hacker, I agree there are definitely cases. For example if you control an API that's internal to your program then you can eventually achieve a level of confidence it's not needed. But how can you impart this constraint onto your future coworkers? That's the question I struggle with
JaredPar
+5  A: 

The answer to your question is often, but now always. If your abstract class forbids clients to call delete on a pointer to it (or if it says so in its documentation), you are free to not declare a virtual destructor.

You can forbid clients to call delete on a pointer to it by making its destructor protected. Working like this, it is perfectly safe and reasonable to omit a virtual destructor.

You will eventually end up with no virtual method table, and end up signalling your clients your intention on making it non-deleteable through a pointer to it, so you have indeed reason not to declare it virtual in those cases.

[See item 4 in this article: http://www.gotw.ca/publications/mill18.htm]

Johannes Schaub - litb
The key to making your answer work is "on which delete is not called upon." Usually if you have an abstract base class designed to be an interface, delete is going to be called on the interface class.
John Dibling
As John above pointed out, what you're suggesting is pretty dangerous. You're relying on the assumption that clients of your interface will never destroy an object knowing only the base type. The only way you could guarantee that if it's nonvirtual is to make the abstract class's dtor protected.
Michel
Michel, i have said so :) "If you do that, you make your destructor protected. If you do so, clients will not be able to delete using a pointer to that interface." and indeed it's not relying on the clients, but it has to enforce it telling the clients "you cannot do..." . I don't see any danger
Johannes Schaub - litb
i fixed the poor wording of my answer now. it states it explicitly now that it doesn't rely on the clients. actually i thought that's obvious that relying on clients doing something is out the way anyway. thanks :)
Johannes Schaub - litb
+1 for mentioning protected destructors, which are the other "way out" of the problem of accidentally calling the wrong destructor when deleting a pointer to a base class.
j_random_hacker
+1 because this is actually the only correct answer. It's incredible how much this is misunderstood in c++ :)
Paolo Tedesco