[opensource-dev] Virtual Destructors

Ricky kf6kjg at gmail.com
Fri Jul 1 15:25:25 PDT 2011


Poking around in the llmanip* files working on VWR-25739, I started to
get annoyed at the coding inconsistencies between those files.  So I
started looking at what it would take to make the 3 subclasses
(translate, scale, and rotate) consistent, when I tripped across the
detail that llmaniptranslate.h has the destructor declared virtual
while llmanipscale.h has it declared plainly, and llmaniprotate.h
doesn't explicitly declare a destructor.

When I looked up some reasons why a destructor should be virtual it
seems that it should be virtual when the class is going to be used in
a polymorphic way and will have delete called on a pointer to it.  IE:
// MyClass is a ParentClass
ParentClass* p = new MyClass();
destroy p;

Apparently this is about the only case for declaring the destructor
virtual. (see http://blogs.msdn.com/b/oldnewthing/archive/2004/05/07/127826.aspx
and especially http://www.erata.net/programming/virtual-destructors/ )
 It also comes with a minor performance hit, but that's outside of
scope.

It turns out that LLManipScale _is_ being used in such a way in
LLToolComp - as are LLManipScale and LLManipRotate:
lltoolcomp.h line 92: LLManip* mManip;
lltoolcomp.cpp line 194: mManip = new LLManipTranslate(this);
lltoolcomp.cpp line 203: delete mManip;
lltoolcomp.cpp line 321: mManip = new LLManipScale(this);
lltoolcomp.cpp line 330: delete mManip;
lltoolcomp.cpp line 520: mManip = new LLManipRotate(this);
lltoolcomp.cpp line 530: delete mManip;

So it looks like to me that there might be a memory leak in the scale
and rotate classes, as their destructors might NOT be being called.
Of course, Translate's destructor has only an empty definition, and
Rotate doesn't even have one, but Scale does have a full-on
destructor.  And because it is not virtual, it might not be being
called.

Looking over the history of the files gives me the following:
The Rotate destructor was last touched by Steven Bennets on 2008-03-11
in rev 341 - when LLLinkedList was culled in favor of another
technique.
The Translate destructor was emptied by James Cook on 2009-12-10 in
rev 4496 - switched to a std::vector
The Scale destructor seems to have never existed in revision history.

Anyone with more familiarity with C++'s nuances in such cases have any
thoughts/suggestions?

Ricky
Cron Stardust


More information about the opensource-dev mailing list